Bug 1307122 - Introducing a timeout for sync XHR when unload events are dispatched, r=smaug

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;
 

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 <bitset>                        // 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<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.
@@ -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)

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<nsXMLHttpRequestXPCOMifier>
 XMLHttpRequestMainThread::EnsureXPCOMifier()
 {

dom/xhr/XMLHttpRequestMainThread.h

@@ -710,6 +710,18 @@ protected:
   void StartTimeoutTimer();
   void HandleTimeoutCallback();
 
+  nsCOMPtr<nsITimer> mSyncTimeoutTimer;
+
+  enum SyncTimeoutType {
+    eErrorOrExpired,
+    eTimerStarted,
+    eNoTimerNeeded
+  };
+
+  SyncTimeoutType MaybeStartSyncTimeoutTimer();
+  void HandleSyncTimeoutTimer();
+  void CancelSyncTimeoutTimer();
+
   bool mErrorLoad;
   bool mErrorParsingXML;
   bool mWaitingForOnStopRequest;

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>
+

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]

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);
+}

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>

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);
   }