Bug 1307122 - Introducing a timeout for sync XHR when unload events are dispatched, r=smaug a=gchang
authorAndrea Marchesini <amarchesini@mozilla.com>
Sun, 16 Oct 2016 08:46:10 +0200
changeset 875974 d33b1329a44d70645ee3bfc7b87063de4da8c54a
parent 875973 684b31fa5cb1c69bcb7c25f7d57e31057c813b71
child 875975 6e517aa653d6aa0dd846f66ae9b6826821c1bad7
push id151235
push userphilringnalda@gmail.com
push dateTue, 25 Oct 2016 02:01:17 +0000
treeherdertry@98f56a8216a9 [default view] [failures only]
reviewerssmaug, gchang
bugs1307122
milestone51.0a2
Bug 1307122 - Introducing a timeout for sync XHR when unload events are dispatched, r=smaug a=gchang
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
dom/xhr/tests/empty.html
dom/xhr/tests/iframe_sync_xhr_unload.html
dom/xhr/tests/mochitest.ini
dom/xhr/tests/sync_xhr_unload.sjs
dom/xhr/tests/test_sync_xhr_unload.html
layout/base/nsDocumentViewer.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -8835,17 +8835,20 @@ nsDocument::OnPageHide(bool aPersisted,
                           "chrome-page-hidden" :
                           "content-page-hidden",
                         nullptr);
 
     os->RemoveObserver(this, "app-theme-changed");
     mObservingAppThemeChanged = false;
   }
 
-  DispatchPageTransition(target, NS_LITERAL_STRING("pagehide"), aPersisted);
+  {
+    PageUnloadingEventTimeStamp timeStamp(this);
+    DispatchPageTransition(target, NS_LITERAL_STRING("pagehide"), aPersisted);
+  }
 
   mVisible = false;
 
   UpdateVisibilityState();
 
   EnumerateExternalResources(NotifyPageHide, &aPersisted);
   EnumerateActivityObservers(NotifyActivityChanged, nullptr);
 
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -32,16 +32,17 @@
 #include "nsExpirationTracker.h"
 #include "nsClassHashtable.h"
 #include "prclist.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/CORSMode.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/StyleBackendType.h"
 #include "mozilla/StyleSheetHandle.h"
+#include "mozilla/TimeStamp.h"
 #include <bitset>                        // for member
 
 #ifdef MOZILLA_INTERNAL_API
 #include "mozilla/dom/DocumentBinding.h"
 #else
 namespace mozilla {
 namespace dom {
 class ElementCreationOptionsOrString;
@@ -209,16 +210,43 @@ public:
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IDOCUMENT_IID)
   NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
 
 #ifdef MOZILLA_INTERNAL_API
   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<nsIDocument> 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.
    *                 XXXbz It's odd to have that here.
    * @param aChannel The channel the data will come from. The channel must be
    *                 able to report its Content-Type.
    * @param aLoadGroup The loadgroup this document should use from now on.
    *                   Note that the document might not be the only thing using
@@ -926,19 +954,50 @@ public:
   nsTArray<RefPtr<mozilla::dom::AnonymousContent>>& GetAnonymousContents() {
     return mAnonymousContents;
   }
 
   static nsresult GenerateDocumentId(nsAString& aId);
   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
   {
     public:
       explicit SelectorCacheKey(const nsAString& aString) : mKey(aString)
       {
         MOZ_COUNT_CTOR(SelectorCacheKey);
       }
@@ -3252,16 +3311,18 @@ protected:
   // Flags for use counters used by any child documents of this document.
   std::bitset<mozilla::eUseCounter_Count> mChildDocumentUseCounters;
   // Flags for whether we've notified our top-level "page" of a use counter
   // for this child document.
   std::bitset<mozilla::eUseCounter_Count> mNotifiedPageForUseCounter;
 
   // Whether the user has interacted with the document or not:
   bool mUserHasInteracted;
+
+  mozilla::TimeStamp mPageUnloadingEventTimeStamp;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIDocument, NS_IDOCUMENT_IID)
 
 /**
  * mozAutoSubtreeModified batches DOM mutations so that a DOMSubtreeModified
  * event is dispatched, if necessary, when the outermost mozAutoSubtreeModified
  * object is deleted.
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -118,16 +118,17 @@ namespace {
   const nsString kLiteralString_DOMContentLoaded = NS_LITERAL_STRING("DOMContentLoaded");
 }
 
 // CIDs
 #define NS_BADCERTHANDLER_CONTRACTID \
   "@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)
 
 class nsResumeTimeoutsEvent : public Runnable
 {
 public:
   explicit nsResumeTimeoutsEvent(nsPIDOMWindowInner* aWindow) : mWindow(aWindow) {}
 
@@ -2856,25 +2857,38 @@ XMLHttpRequestMainThread::SendInternal(c
           topWindow->SuspendTimeouts(1, false);
           resumeTimeoutRunnable = new nsResumeTimeoutsEvent(topInner);
         }
       }
     }
 
     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) {
         if (!NS_ProcessNextEvent(thread)) {
           rv = NS_ERROR_UNEXPECTED;
           break;
         }
       }
+
+      // Time expired... We should throw.
+      if (syncTimeoutType == eTimerStarted && !mSyncTimeoutTimer) {
+        rv = NS_ERROR_DOM_NETWORK_ERR;
+      }
+
+      CancelSyncTimeoutTimer();
     }
 
     if (suspendedDoc) {
       suspendedDoc->UnsuppressEventHandlingAndFireEvents(nsIDocument::eEvents,
                                                          true);
     }
 
     if (resumeTimeoutRunnable) {
@@ -3429,16 +3443,21 @@ XMLHttpRequestMainThread::Notify(nsITime
     return NS_OK;
   }
 
   if (mTimeoutTimer == aTimer) {
     HandleTimeoutCallback();
     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;
 }
 
 void
 XMLHttpRequestMainThread::HandleProgressTimerCallback()
 {
@@ -3481,16 +3500,62 @@ XMLHttpRequestMainThread::StartProgressE
   if (mProgressNotifier) {
     mProgressTimerIsActive = true;
     mProgressNotifier->Cancel();
     mProgressNotifier->InitWithCallback(this, NS_PROGRESS_EVENT_INTERVAL,
                                         nsITimer::TYPE_ONE_SHOT);
   }
 }
 
+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<nsXMLHttpRequestXPCOMifier>
 XMLHttpRequestMainThread::EnsureXPCOMifier()
 {
   if (!mXPCOMifier) {
     mXPCOMifier = new nsXMLHttpRequestXPCOMifier(this);
   }
   RefPtr<nsXMLHttpRequestXPCOMifier> newRef(mXPCOMifier);
   return newRef.forget();
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -711,16 +711,28 @@ protected:
 
   // Timeout support
   PRTime mRequestSentTime;
   uint32_t mTimeoutMilliseconds;
   nsCOMPtr<nsITimer> mTimeoutTimer;
   void StartTimeoutTimer();
   void HandleTimeoutCallback();
 
+  nsCOMPtr<nsITimer> mSyncTimeoutTimer;
+
+  enum SyncTimeoutType {
+    eErrorOrExpired,
+    eTimerStarted,
+    eNoTimerNeeded
+  };
+
+  SyncTimeoutType MaybeStartSyncTimeoutTimer();
+  void HandleSyncTimeoutTimer();
+  void CancelSyncTimeoutTimer();
+
   bool mErrorLoad;
   bool mErrorParsingXML;
   bool mWaitingForOnStopRequest;
   bool mProgressTimerIsActive;
   bool mIsHtml;
   bool mWarnAboutMultipartHtml;
   bool mWarnAboutSyncHtml;
   int64_t mLoadTotal; // 0 if not known.
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/dom/xhr/tests/iframe_sync_xhr_unload.html
@@ -0,0 +1,18 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+  <script type="application/javascript">
+
+function o() {
+ var xhr = new XMLHttpRequest();
+ xhr.open("GET", "sync_xhr_unload.sjs", false);
+ try { xhr.send(); } catch(e) {}
+}
+
+window.addEventListener("beforeunload", o, false);
+window.addEventListener("unload", o, false)
+
+  </script>
+</body>
+</html>
+
--- a/dom/xhr/tests/mochitest.ini
+++ b/dom/xhr/tests/mochitest.ini
@@ -52,16 +52,19 @@ support-files =
   relativeLoad_import.js
   relativeLoad_worker.js
   relativeLoad_worker2.js
   responseIdentical.sjs
   subdir/relativeLoad_sub_worker.js
   subdir/relativeLoad_sub_worker2.js
   subdir/relativeLoad_sub_import.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
 [test_html_in_xhr.html]
 [test_sync_xhr_timer.xhtml]
 skip-if = toolkit == 'android'
 [test_xhr_abort_after_load.html]
 skip-if = toolkit == 'android'
@@ -98,8 +101,9 @@ skip-if = buildapp == 'b2g'
 skip-if = buildapp == 'b2g'
 [test_worker_xhr_responseURL.html]
 [test_worker_xhr_system.html]
 [test_worker_xhr_timeout.html]
 skip-if = (os == "win") || (os == "mac") || toolkit == 'android' #bug 798220
 [test_relativeLoad.html]
 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_bug1300552.html]
+[test_sync_xhr_unload.html]
new file mode 100644
--- /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);
+}
new file mode 100644
--- /dev/null
+++ b/dom/xhr/tests/test_sync_xhr_unload.html
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1307122</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="common_temporaryFileBlob.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <script type="application/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+info("Creating the iframe...");
+var ifr = document.createElement('iframe');
+
+ifr.addEventListener("load", function ifr_load1() {
+  info("Iframe loaded");
+
+  ifr.removeEventListener("load", ifr_load1);
+  ifr.src = "empty.html";
+
+  ifr.addEventListener("load", function ifr_load2() {
+    ok(true, "Test passed");
+    SimpleTest.finish();
+  });
+
+});
+
+ifr.src = "iframe_sync_xhr_unload.html";
+document.body.appendChild(ifr);
+
+  </script>
+</body>
+</html>
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -1085,17 +1085,17 @@ nsDocumentViewer::PermitUnloadInternal(b
    || mInPermitUnloadPrompt) {
     return NS_OK;
   }
 
   static bool sIsBeforeUnloadDisabled;
   static bool sBeforeUnloadRequiresInteraction;
   static bool sBeforeUnloadPrefsCached = false;
 
-  if (!sBeforeUnloadPrefsCached ) {
+  if (!sBeforeUnloadPrefsCached) {
     sBeforeUnloadPrefsCached = true;
     Preferences::AddBoolVarCache(&sIsBeforeUnloadDisabled,
                                  BEFOREUNLOAD_DISABLED_PREFNAME);
     Preferences::AddBoolVarCache(&sBeforeUnloadRequiresInteraction,
                                  BEFOREUNLOAD_REQUIRES_INTERACTION_PREFNAME);
   }
 
   // First, get the script global object from the document...
@@ -1133,16 +1133,18 @@ nsDocumentViewer::PermitUnloadInternal(b
     // how we get here.
     nsAutoPopupStatePusher popupStatePusher(openAbused, true);
 
     // Never permit dialogs from the beforeunload handler
     nsGlobalWindow* globalWindow = nsGlobalWindow::Cast(window);
     dialogsAreEnabled = globalWindow->AreDialogsEnabled();
     nsGlobalWindow::TemporarilyDisableDialogs disableDialogs(globalWindow);
 
+    nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument);
+
     mInPermitUnload = true;
     EventDispatcher::DispatchDOMEvent(window, nullptr, event, mPresContext,
                                       nullptr);
     mInPermitUnload = false;
   }
 
   nsCOMPtr<nsIDocShell> docShell(mContainer);
   nsAutoString text;
@@ -1316,16 +1318,18 @@ nsDocumentViewer::PageHide(bool aIsUnloa
     event.mFlags.mBubbles = false;
     // XXX Dispatching to |window|, but using |document| as the target.
     event.mTarget = mDocument;
 
     // Never permit popups from the unload handler, no matter how we get
     // here.
     nsAutoPopupStatePusher popupStatePusher(openAbused, true);
 
+    nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument);
+
     EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status);
   }
 
 #ifdef MOZ_XUL
   // look for open menupopups and close them after the unload event, in case
   // the unload event listeners open any new popups
   nsContentUtils::HidePopupsInDocument(mDocument);
 #endif