Bug 1730117 - Part 1: Make sync XHR suppress event handling for the nested in-process documents; r=smaug
authorEdgar Chen <echen@mozilla.com>
Tue, 21 Sep 2021 11:39:45 +0000
changeset 592647 27a1d92e550caaba282a450a952434005634ca78
parent 592646 07e90ae59d8c9f94f7c957e4d737874afb8c88bc
child 592648 bafc4ddc87f6369a5085b1e43867fda014dba654
push id150108
push userechen@mozilla.com
push dateTue, 21 Sep 2021 11:42:29 +0000
treeherderautoland@bafc4ddc87f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1730117
milestone94.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1730117 - Part 1: Make sync XHR suppress event handling for the nested in-process documents; r=smaug Differential Revision: https://phabricator.services.mozilla.com/D125187
dom/base/AutoSuppressEventHandlingAndSuspend.h
dom/base/nsContentUtils.cpp
dom/base/test/file_suppressed_events_inner.html
dom/base/test/file_suppressed_events_middle.html
dom/base/test/file_suppressed_events_top_xhr.html
dom/base/test/mochitest.ini
dom/base/test/test_suppressed_events_nested_iframe.html
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
--- a/dom/base/AutoSuppressEventHandlingAndSuspend.h
+++ b/dom/base/AutoSuppressEventHandlingAndSuspend.h
@@ -10,37 +10,59 @@
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/BrowsingContextGroup.h"
 #include "mozilla/dom/Document.h"
 #include "nsCOMPtr.h"
 #include "nsPIDOMWindow.h"
 #include "nsTArray.h"
 
 namespace mozilla::dom {
+
+/**
+ * Suppresses event handling and suspends for all in-process documents in a
+ * BrowsingContext subtree.
+ */
+class MOZ_RAII AutoSuppressEventHandling
+    : private AutoWalkBrowsingContextGroup {
+ public:
+  explicit AutoSuppressEventHandling(BrowsingContext* aContext) {
+    if (aContext) {
+      SuppressBrowsingContext(aContext);
+    }
+  }
+
+  explicit AutoSuppressEventHandling(BrowsingContextGroup* aGroup) {
+    if (aGroup) {
+      SuppressBrowsingContextGroup(aGroup);
+    }
+  }
+
+  ~AutoSuppressEventHandling();
+
+ protected:
+  void SuppressDocument(Document* aDocument) override;
+  void UnsuppressDocument(Document* aDocument) override;
+};
+
 /**
  * Suppresses event handling and suspends the active inner window for all
  * in-process documents in a BrowsingContextGroup. This should be used while
  * spinning the event loop for a synchronous operation (like `window.open()`)
  * which affects operations in any other window in the same BrowsingContext
  * group.
  */
-
 class MOZ_RAII AutoSuppressEventHandlingAndSuspend
-    : private AutoWalkBrowsingContextGroup {
+    : private AutoSuppressEventHandling {
  public:
-  explicit AutoSuppressEventHandlingAndSuspend(BrowsingContextGroup* aGroup) {
-    if (aGroup) {
-      SuppressBrowsingContextGroup(aGroup);
-    }
-  }
+  explicit AutoSuppressEventHandlingAndSuspend(BrowsingContextGroup* aGroup)
+      : AutoSuppressEventHandling(aGroup) {}
 
   ~AutoSuppressEventHandlingAndSuspend();
 
  protected:
   void SuppressDocument(Document* aDocument) override;
-  void UnsuppressDocument(Document* aDocument) override;
 
  private:
   AutoTArray<nsCOMPtr<nsPIDOMWindowInner>, 16> mWindows;
 };
 }  // namespace mozilla::dom
 
 #endif
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -668,40 +668,47 @@ class SameOriginCheckerImpl final : publ
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
 };
 
 }  // namespace
 
-void AutoSuppressEventHandlingAndSuspend::SuppressDocument(Document* aDoc) {
+void AutoSuppressEventHandling::SuppressDocument(Document* aDoc) {
   // Note: Document::SuppressEventHandling will also automatically suppress
   // event handling for any in-process sub-documents. However, since we need
   // to deal with cases where remote BrowsingContexts may be interleaved
   // with in-process ones, we still need to walk the entire tree ourselves.
   // This may be slightly redundant in some cases, but since event handling
   // suppressions maintain a count of current blockers, it does not cause
   // any problems.
   aDoc->SuppressEventHandling();
+}
+
+void AutoSuppressEventHandling::UnsuppressDocument(Document* aDoc) {
+  aDoc->UnsuppressEventHandlingAndFireEvents(true);
+}
+
+AutoSuppressEventHandling::~AutoSuppressEventHandling() {
+  UnsuppressDocuments();
+}
+
+void AutoSuppressEventHandlingAndSuspend::SuppressDocument(Document* aDoc) {
+  AutoSuppressEventHandling::SuppressDocument(aDoc);
   if (nsCOMPtr<nsPIDOMWindowInner> win = aDoc->GetInnerWindow()) {
     win->Suspend();
     mWindows.AppendElement(win);
   }
 }
 
-void AutoSuppressEventHandlingAndSuspend::UnsuppressDocument(Document* aDoc) {
-  aDoc->UnsuppressEventHandlingAndFireEvents(true);
-}
-
 AutoSuppressEventHandlingAndSuspend::~AutoSuppressEventHandlingAndSuspend() {
   for (const auto& win : mWindows) {
     win->Resume();
   }
-  UnsuppressDocuments();
 }
 
 /**
  * This class is used to determine whether or not the user is currently
  * interacting with the browser. It listens to observer events to toggle the
  * value of the sUserActive static.
  *
  * This class is an internal implementation detail.
new file mode 100644
--- /dev/null
+++ b/dom/base/test/file_suppressed_events_inner.html
@@ -0,0 +1,16 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>Test event suppression</title>
+</head>
+<body>
+<div>Inner</div>
+<script type="application/javascript">
+
+window.onload = function() {
+  top.postMessage("ready", "*");
+};
+
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/base/test/file_suppressed_events_middle.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>Test event suppression</title>
+</head>
+<body>
+<div>Middle</div>
+<iframe src="http://mochi.test:8888/tests/dom/base/test/file_suppressed_events_inner.html"></iframe>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/base/test/file_suppressed_events_top_xhr.html
@@ -0,0 +1,82 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>Test event suppression</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/paint_listener.js"></script>
+<script src="/tests/gfx/layers/apz/test/mochitest/apz_test_utils.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<div>Top</div>
+<script type="application/javascript">
+
+function waitForMessage(aMsg, aCallback) {
+  window.addEventListener("message", function handler(e) {
+    if (e.data != aMsg) {
+      return;
+    }
+
+    info(`received: ${e.data}`);
+    window.removeEventListener("message", handler);
+    if (aCallback) {
+      aCallback(e);
+    }
+  });
+}
+
+function waitForClickEvent(aElement, aWindow) {
+  return new Promise((aResolve) => {
+    aElement.addEventListener("click", aResolve, { once: true });
+    synthesizeMouseAtCenter(aElement, { type: "mousedown" }, aWindow);
+    synthesizeMouseAtCenter(aElement, { type: "mouseup" }, aWindow);
+  });
+}
+
+waitForMessage("ready", async function(e) {
+  await waitUntilApzStable();
+
+  let innerWin = e.source;
+  let innerDiv = innerWin.document.querySelector("div");
+
+  let eventCount = 0;
+  innerDiv.addEventListener("mousemove", function() {
+    eventCount++;
+  });
+
+  // Test that event handling is suppressed.
+  let xhr = new XMLHttpRequest();
+  opener.info("xhr open");
+  xhr.open('GET', 'slow.sjs', false);
+
+  const TOTAL = 100;
+  for (let i = 0; i < TOTAL; i++) {
+    synthesizeMouseAtCenter(innerDiv, { type: "mousemove" }, innerWin);
+  }
+  xhr.send();
+  opener.info(`xhr done`);
+
+  // Wait for click event to ensure we have received all mousemove events.
+  await waitForClickEvent(innerDiv, innerWin);
+  opener.info(`eventCount: ${eventCount}`);
+  opener.ok(eventCount < TOTAL, "event should be suspressed");
+
+  // Test that event handling is not suppressed.
+  eventCount = 0;
+  for (let i = 0; i < TOTAL; i++) {
+    synthesizeMouseAtCenter(innerDiv, { type: "mousemove" }, innerWin);
+  }
+
+  // Wait for click event to ensure we have received all mousemove events.
+  await waitForClickEvent(innerDiv, innerWin);
+  opener.info(`eventCount: ${eventCount}`);
+  opener.is(eventCount, TOTAL, "event should not be suspressed");
+
+  opener.postMessage("done", "*");
+});
+
+</script>
+<iframe src="http://example.org/tests/dom/base/test/file_suppressed_events_middle.html"></iframe>
+</body>
+</html>
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@@ -769,16 +769,23 @@ skip-if = debug == false
 [test_setting_opener.html]
 [test_shared_compartment1.html]
 [test_shared_compartment2.html]
 [test_structuredclone_backref.html]
 [test_style_cssText.html]
 [test_suppressed_events_and_scrolling.html]
 support-files =
   file_suppressed_events_and_scrolling.html
+[test_suppressed_events_nested_iframe.html]
+skip-if = os == "android"
+support-files =
+  file_suppressed_events_top_xhr.html
+  file_suppressed_events_middle.html
+  file_suppressed_events_inner.html
+  !/gfx/layers/apz/test/mochitest/apz_test_utils.js
 [test_suppressed_microtasks.html]
 skip-if = debug || asan || verify || toolkit == 'android' # The test needs to run reasonably fast.
 [test_text_wholeText.html]
 [test_textnode_normalize_in_selection.html]
 [test_textnode_split_in_selection.html]
 [test_timeout_clamp.html]
 [test_timer_flood.html]
 [test_title.html]
new file mode 100644
--- /dev/null
+++ b/dom/base/test/test_suppressed_events_nested_iframe.html
@@ -0,0 +1,49 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1730117
+-->
+<head>
+  <title>Test event suppression on nested iframe</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1730117">Mozilla Bug 1730117</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+function waitForMessage(aMsg) {
+  return new Promise((aResolve) => {
+    window.addEventListener("message", function handler(e) {
+      info(`receive: ${e.data}`);
+      if (e.data != aMsg) {
+        return;
+      }
+
+      window.removeEventListener("message", handler);
+      aResolve();
+    });
+  });
+}
+
+/** Test for Bug 1730117 **/
+
+add_task(async function test_sync_xhr() {
+  await SpecialPowers.pushPrefEnv({"set": [
+    ["test.events.async.enabled", true],
+    ["dom.events.coalesce.mousemove", false],
+  ]});
+
+  let w = window.open("file_suppressed_events_top_xhr.html");
+  await waitForMessage("done");
+  w.close();
+});
+
+</script>
+</pre>
+</body>
+</html>
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -9,16 +9,17 @@
 #include <algorithm>
 #ifndef XP_WIN
 #  include <unistd.h>
 #endif
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/Components.h"
+#include "mozilla/dom/AutoSuppressEventHandlingAndSuspend.h"
 #include "mozilla/dom/BlobBinding.h"
 #include "mozilla/dom/BlobURLProtocolHandler.h"
 #include "mozilla/dom/DocGroup.h"
 #include "mozilla/dom/DOMString.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FileBinding.h"
 #include "mozilla/dom/FileCreatorHelper.h"
 #include "mozilla/dom/FetchUtil.h"
@@ -2792,25 +2793,20 @@ void XMLHttpRequestMainThread::EnsureCha
   nsAutoCString contentType;
   if (NS_FAILED(mChannel->GetContentType(contentType)) ||
       contentType.IsEmpty() ||
       contentType.EqualsLiteral(UNKNOWN_CONTENT_TYPE)) {
     mChannel->SetContentType("text/xml"_ns);
   }
 }
 
-void XMLHttpRequestMainThread::UnsuppressEventHandlingAndResume() {
+void XMLHttpRequestMainThread::ResumeTimeout() {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mFlagSynchronous);
 
-  if (mSuspendedDoc) {
-    mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(true);
-    mSuspendedDoc = nullptr;
-  }
-
   if (mResumeTimeoutRunnable) {
     DispatchToMainThread(mResumeTimeoutRunnable.forget());
     mResumeTimeoutRunnable = nullptr;
   }
 }
 
 void XMLHttpRequestMainThread::Send(
     const Nullable<
@@ -3023,50 +3019,51 @@ void XMLHttpRequestMainThread::SendInter
   StartTimeoutTimer();
 
   mWaitingForOnStopRequest = true;
 
   // Step 8
   mFlagSend = true;
 
   // If we're synchronous, spin an event loop here and wait
+  RefPtr<Document> suspendedDoc;
   if (mFlagSynchronous) {
+    auto scopeExit = MakeScopeExit([&] {
+      CancelSyncTimeoutTimer();
+      ResumeTimeout();
+      ResumeEventDispatching();
+    });
+    Maybe<AutoSuppressEventHandling> autoSuppress;
+
     mFlagSyncLooping = true;
 
     if (GetOwner()) {
       if (nsCOMPtr<nsPIDOMWindowOuter> topWindow =
               GetOwner()->GetOuterWindow()->GetInProcessTop()) {
         if (nsCOMPtr<nsPIDOMWindowInner> topInner =
                 topWindow->GetCurrentInnerWindow()) {
-          mSuspendedDoc = topWindow->GetExtantDoc();
-          if (mSuspendedDoc) {
-            mSuspendedDoc->SuppressEventHandling();
-          }
+          suspendedDoc = topWindow->GetExtantDoc();
+          autoSuppress.emplace(topWindow->GetBrowsingContext());
           topInner->Suspend();
           mResumeTimeoutRunnable = new nsResumeTimeoutsEvent(topInner);
         }
       }
     }
 
     SuspendEventDispatching();
     StopProgressEventTimer();
-    auto scopeExit = MakeScopeExit([&] {
-      CancelSyncTimeoutTimer();
-      UnsuppressEventHandlingAndResume();
-      ResumeEventDispatching();
-    });
 
     SyncTimeoutType syncTimeoutType = MaybeStartSyncTimeoutTimer();
     if (syncTimeoutType == eErrorOrExpired) {
       Abort();
       aRv.Throw(NS_ERROR_DOM_NETWORK_ERR);
       return;
     }
 
-    nsAutoSyncOperation sync(mSuspendedDoc,
+    nsAutoSyncOperation sync(suspendedDoc,
                              SyncOperationBehavior::eSuspendInput);
     if (!SpinEventLoopUntil([&]() { return !mFlagSyncLooping; })) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
 
     // Time expired... We should throw.
     if (syncTimeoutType == eTimerStarted && !mSyncTimeoutTimer) {
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -299,17 +299,17 @@ class XMLHttpRequestMainThread final : p
 
   nsresult MaybeSilentSendFailure(nsresult aRv);
   void SendInternal(const BodyExtractorBase* aBody,
                     bool aBodyIsDocumentOrString, ErrorResult& aRv);
 
   bool IsCrossSiteCORSRequest() const;
   bool IsDeniedCrossSiteCORSRequest();
 
-  void UnsuppressEventHandlingAndResume();
+  void ResumeTimeout();
 
   void MaybeLowerChannelPriority();
 
  public:
   virtual void Send(
       const Nullable<
           DocumentOrBlobOrArrayBufferViewOrArrayBufferOrFormDataOrURLSearchParamsOrUSVString>&
           aData,
@@ -674,17 +674,16 @@ class XMLHttpRequestMainThread final : p
 
   // Timeout support
   PRTime mRequestSentTime;
   uint32_t mTimeoutMilliseconds;
   nsCOMPtr<nsITimer> mTimeoutTimer;
   void StartTimeoutTimer();
   void HandleTimeoutCallback();
 
-  RefPtr<Document> mSuspendedDoc;
   nsCOMPtr<nsIRunnable> mResumeTimeoutRunnable;
 
   nsCOMPtr<nsITimer> mSyncTimeoutTimer;
 
   enum SyncTimeoutType { eErrorOrExpired, eTimerStarted, eNoTimerNeeded };
 
   SyncTimeoutType MaybeStartSyncTimeoutTimer();
   void HandleSyncTimeoutTimer();