From 9215b7b957129306fc3e5f36c5342f9010ce615f Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Sun, 16 Oct 2016 08:46:10 +0200 Subject: [PATCH] Bug 1307122 - Introducing a timeout for sync XHR when unload events are dispatched, r=smaug --- dom/base/nsDocument.cpp | 5 +- dom/base/nsIDocument.h | 61 +++++++++++++++++++++ dom/xhr/XMLHttpRequestMainThread.cpp | 67 ++++++++++++++++++++++- dom/xhr/XMLHttpRequestMainThread.h | 12 ++++ dom/xhr/tests/empty.html | 0 dom/xhr/tests/iframe_sync_xhr_unload.html | 18 ++++++ dom/xhr/tests/mochitest.ini | 4 ++ dom/xhr/tests/sync_xhr_unload.sjs | 15 +++++ dom/xhr/tests/test_sync_xhr_unload.html | 36 ++++++++++++ layout/base/nsDocumentViewer.cpp | 6 +- 10 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 dom/xhr/tests/empty.html create mode 100644 dom/xhr/tests/iframe_sync_xhr_unload.html create mode 100644 dom/xhr/tests/sync_xhr_unload.sjs create mode 100644 dom/xhr/tests/test_sync_xhr_unload.html diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index d92a8d32f751..f7b6fa00fae4 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -8811,7 +8811,10 @@ nsDocument::OnPageHide(bool aPersisted, nullptr); } - DispatchPageTransition(target, NS_LITERAL_STRING("pagehide"), aPersisted); + { + PageUnloadingEventTimeStamp timeStamp(this); + DispatchPageTransition(target, NS_LITERAL_STRING("pagehide"), aPersisted); + } mVisible = false; diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index cd3853bb94d6..5202f64af8d7 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -37,6 +37,7 @@ #include "mozilla/LinkedList.h" #include "mozilla/StyleBackendType.h" #include "mozilla/StyleSheet.h" +#include "mozilla/TimeStamp.h" #include // for member #ifdef MOZILLA_INTERNAL_API @@ -216,6 +217,33 @@ public: nsIDocument(); #endif + // This helper class must be set when we dispatch beforeunload and unload + // events in order to avoid unterminate sync XHRs. + class MOZ_RAII PageUnloadingEventTimeStamp + { + nsCOMPtr mDocument; + bool mSet; + + public: + explicit PageUnloadingEventTimeStamp(nsIDocument* aDocument) + : mDocument(aDocument) + , mSet(false) + { + MOZ_ASSERT(aDocument); + if (mDocument->mPageUnloadingEventTimeStamp.IsNull()) { + mDocument->SetPageUnloadingEventTimeStamp(); + mSet = true; + } + } + + ~PageUnloadingEventTimeStamp() + { + if (mSet) { + mDocument->CleanUnloadEventsTimeStamp(); + } + } + }; + /** * Let the document know that we're starting to load data into it. * @param aCommand The parser command. Must not be null. @@ -936,9 +964,40 @@ public: nsresult GetOrCreateId(nsAString& aId); void SetId(const nsAString& aId); + mozilla::TimeStamp GetPageUnloadingEventTimeStamp() const + { + if (!mParentDocument) { + return mPageUnloadingEventTimeStamp; + } + + mozilla::TimeStamp parentTimeStamp(mParentDocument->GetPageUnloadingEventTimeStamp()); + if (parentTimeStamp.IsNull()) { + return mPageUnloadingEventTimeStamp; + } + + if (!mPageUnloadingEventTimeStamp || + parentTimeStamp < mPageUnloadingEventTimeStamp) { + return parentTimeStamp; + } + + return mPageUnloadingEventTimeStamp; + } + protected: virtual Element *GetRootElementInternal() const = 0; + void SetPageUnloadingEventTimeStamp() + { + MOZ_ASSERT(!mPageUnloadingEventTimeStamp); + mPageUnloadingEventTimeStamp = mozilla::TimeStamp::NowLoRes(); + } + + void CleanUnloadEventsTimeStamp() + { + MOZ_ASSERT(mPageUnloadingEventTimeStamp); + mPageUnloadingEventTimeStamp = mozilla::TimeStamp(); + } + private: class SelectorCacheKey { @@ -3242,6 +3301,8 @@ protected: // Whether the user has interacted with the document or not: bool mUserHasInteracted; + + mozilla::TimeStamp mPageUnloadingEventTimeStamp; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsIDocument, NS_IDOCUMENT_IID) diff --git a/dom/xhr/XMLHttpRequestMainThread.cpp b/dom/xhr/XMLHttpRequestMainThread.cpp index 85331656788d..49e8680e3383 100644 --- a/dom/xhr/XMLHttpRequestMainThread.cpp +++ b/dom/xhr/XMLHttpRequestMainThread.cpp @@ -125,6 +125,7 @@ namespace { "@mozilla.org/content/xmlhttprequest-bad-cert-handler;1" #define NS_PROGRESS_EVENT_INTERVAL 50 +#define MAX_SYNC_TIMEOUT_WHEN_UNLOADING 10000 /* 10 secs */ NS_IMPL_ISUPPORTS(nsXHRParseEndListener, nsIDOMEventListener) @@ -2932,7 +2933,13 @@ XMLHttpRequestMainThread::SendInternal(const RequestBodyBase* aBody) StopProgressEventTimer(); - { + SyncTimeoutType syncTimeoutType = MaybeStartSyncTimeoutTimer(); + if (syncTimeoutType == eErrorOrExpired) { + Abort(); + rv = NS_ERROR_DOM_NETWORK_ERR; + } + + if (NS_SUCCEEDED(rv)) { nsAutoSyncOperation sync(suspendedDoc); nsIThread *thread = NS_GetCurrentThread(); while (mFlagSyncLooping) { @@ -2941,6 +2948,13 @@ XMLHttpRequestMainThread::SendInternal(const RequestBodyBase* aBody) break; } } + + // Time expired... We should throw. + if (syncTimeoutType == eTimerStarted && !mSyncTimeoutTimer) { + rv = NS_ERROR_DOM_NETWORK_ERR; + } + + CancelSyncTimeoutTimer(); } if (suspendedDoc) { @@ -3512,6 +3526,11 @@ XMLHttpRequestMainThread::Notify(nsITimer* aTimer) return NS_OK; } + if (mSyncTimeoutTimer == aTimer) { + HandleSyncTimeoutTimer(); + return NS_OK; + } + // Just in case some JS user wants to QI to nsITimerCallback and play with us... NS_WARNING("Unexpected timer!"); return NS_ERROR_INVALID_POINTER; @@ -3564,6 +3583,52 @@ XMLHttpRequestMainThread::StartProgressEventTimer() } } +XMLHttpRequestMainThread::SyncTimeoutType +XMLHttpRequestMainThread::MaybeStartSyncTimeoutTimer() +{ + MOZ_ASSERT(mFlagSynchronous); + + nsIDocument* doc = GetDocumentIfCurrent(); + if (!doc || !doc->GetPageUnloadingEventTimeStamp()) { + return eNoTimerNeeded; + } + + // If we are in a beforeunload or a unload event, we must force a timeout. + TimeDuration diff = (TimeStamp::NowLoRes() - doc->GetPageUnloadingEventTimeStamp()); + if (diff.ToMilliseconds() > MAX_SYNC_TIMEOUT_WHEN_UNLOADING) { + return eErrorOrExpired; + } + + mSyncTimeoutTimer = do_CreateInstance(NS_TIMER_CONTRACTID); + if (!mSyncTimeoutTimer) { + return eErrorOrExpired; + } + + uint32_t timeout = MAX_SYNC_TIMEOUT_WHEN_UNLOADING - diff.ToMilliseconds(); + nsresult rv = mSyncTimeoutTimer->InitWithCallback(this, timeout, + nsITimer::TYPE_ONE_SHOT); + return NS_FAILED(rv) ? eErrorOrExpired : eTimerStarted; +} + +void +XMLHttpRequestMainThread::HandleSyncTimeoutTimer() +{ + MOZ_ASSERT(mSyncTimeoutTimer); + MOZ_ASSERT(mFlagSyncLooping); + + CancelSyncTimeoutTimer(); + Abort(); +} + +void +XMLHttpRequestMainThread::CancelSyncTimeoutTimer() +{ + if (mSyncTimeoutTimer) { + mSyncTimeoutTimer->Cancel(); + mSyncTimeoutTimer = nullptr; + } +} + already_AddRefed XMLHttpRequestMainThread::EnsureXPCOMifier() { diff --git a/dom/xhr/XMLHttpRequestMainThread.h b/dom/xhr/XMLHttpRequestMainThread.h index a572aa388e34..7ea2ae72f95c 100644 --- a/dom/xhr/XMLHttpRequestMainThread.h +++ b/dom/xhr/XMLHttpRequestMainThread.h @@ -710,6 +710,18 @@ protected: void StartTimeoutTimer(); void HandleTimeoutCallback(); + nsCOMPtr mSyncTimeoutTimer; + + enum SyncTimeoutType { + eErrorOrExpired, + eTimerStarted, + eNoTimerNeeded + }; + + SyncTimeoutType MaybeStartSyncTimeoutTimer(); + void HandleSyncTimeoutTimer(); + void CancelSyncTimeoutTimer(); + bool mErrorLoad; bool mErrorParsingXML; bool mWaitingForOnStopRequest; diff --git a/dom/xhr/tests/empty.html b/dom/xhr/tests/empty.html new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/dom/xhr/tests/iframe_sync_xhr_unload.html b/dom/xhr/tests/iframe_sync_xhr_unload.html new file mode 100644 index 000000000000..d58b1994c8b1 --- /dev/null +++ b/dom/xhr/tests/iframe_sync_xhr_unload.html @@ -0,0 +1,18 @@ + + + + + + + diff --git a/dom/xhr/tests/mochitest.ini b/dom/xhr/tests/mochitest.ini index 1e3ea937f799..7e0a336e76d2 100644 --- a/dom/xhr/tests/mochitest.ini +++ b/dom/xhr/tests/mochitest.ini @@ -60,6 +60,9 @@ support-files = common_temporaryFileBlob.js worker_temporaryFileBlob.js worker_bug1300552.js + sync_xhr_unload.sjs + iframe_sync_xhr_unload.html + empty.html [test_xhr_overridemimetype_throws_on_invalid_state.html] skip-if = buildapp == 'b2g' # Requires webgl support @@ -107,3 +110,4 @@ skip-if = (os == "win") || (os == "mac") || toolkit == 'android' #bug 798220 skip-if = buildapp == 'b2g' # b2g(Failed to load script: relativeLoad_import.js) b2g-debug(Failed to load script: relativeLoad_import.js) b2g-desktop(Failed to load script: relativeLoad_import.js) [test_temporaryFileBlob.html] [test_bug1300552.html] +[test_sync_xhr_unload.html] diff --git a/dom/xhr/tests/sync_xhr_unload.sjs b/dom/xhr/tests/sync_xhr_unload.sjs new file mode 100644 index 000000000000..4185056e314d --- /dev/null +++ b/dom/xhr/tests/sync_xhr_unload.sjs @@ -0,0 +1,15 @@ +var timer = null; + +function handleRequest(request, response) +{ + response.processAsync(); + timer = Components.classes["@mozilla.org/timer;1"] + .createInstance(Components.interfaces.nsITimer); + timer.initWithCallback(function() + { + response.setStatusLine(null, 200, "OK"); + response.setHeader("Content-Type", "text/plain", false); + response.write("hello"); + response.finish(); + }, 30000 /* milliseconds */, Components.interfaces.nsITimer.TYPE_ONE_SHOT); +} diff --git a/dom/xhr/tests/test_sync_xhr_unload.html b/dom/xhr/tests/test_sync_xhr_unload.html new file mode 100644 index 000000000000..b11ab1f75aa6 --- /dev/null +++ b/dom/xhr/tests/test_sync_xhr_unload.html @@ -0,0 +1,36 @@ + + + + + Test for Bug 1307122 + + + + + + + + diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index e894eda303f2..006bfefb5a94 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -1087,7 +1087,7 @@ nsDocumentViewer::PermitUnloadInternal(bool *aShouldPrompt, static bool sBeforeUnloadRequiresInteraction; static bool sBeforeUnloadPrefsCached = false; - if (!sBeforeUnloadPrefsCached ) { + if (!sBeforeUnloadPrefsCached) { sBeforeUnloadPrefsCached = true; Preferences::AddBoolVarCache(&sIsBeforeUnloadDisabled, BEFOREUNLOAD_DISABLED_PREFNAME); @@ -1135,6 +1135,8 @@ nsDocumentViewer::PermitUnloadInternal(bool *aShouldPrompt, dialogsAreEnabled = globalWindow->AreDialogsEnabled(); nsGlobalWindow::TemporarilyDisableDialogs disableDialogs(globalWindow); + nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument); + mInPermitUnload = true; EventDispatcher::DispatchDOMEvent(window, nullptr, event, mPresContext, nullptr); @@ -1318,6 +1320,8 @@ nsDocumentViewer::PageHide(bool aIsUnload) // here. nsAutoPopupStatePusher popupStatePusher(openAbused, true); + nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument); + EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status); }