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
2021-09-22 14:50:55 +00:00 · 2021-09-22 14:50:55 +00:00 · e016ae9d81
--- a/dom/base/AutoSuppressEventHandlingAndSuspend.h
+++ b/dom/base/AutoSuppressEventHandlingAndSuspend.h
@ -15,6 +15,28 @@
 #include "nsTArray.h"

 namespace mozilla::dom {
+
+/**
+ * Suppresses event handling and suspends for all in-process documents in a
+ * BrowsingContext subtree.
+ */
+class MOZ_RAII AutoSuppressEventHandling : public AutoWalkBrowsingContextGroup {
+ public:
+  AutoSuppressEventHandling() = default;
+
+  explicit AutoSuppressEventHandling(BrowsingContext* aContext) {
+    if (aContext) {
+      SuppressBrowsingContext(aContext);
+    }
+  }
+
+  ~AutoSuppressEventHandling();
+
+ protected:
+  virtual 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
@ -22,9 +44,8 @@ namespace mozilla::dom {
 * 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) {
@ -36,7 +57,6 @@ class MOZ_RAII AutoSuppressEventHandlingAndSuspend

 protected:
  void SuppressDocument(Document* aDocument) override;
-  void UnsuppressDocument(Document* aDocument) override;

 private:
  AutoTArray<nsCOMPtr<nsPIDOMWindowInner>, 16> mWindows;
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@ -673,7 +673,7 @@ class SameOriginCheckerImpl final : public nsIChannelEventSink,

 }  // 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
@ -682,21 +682,28 @@ void AutoSuppressEventHandlingAndSuspend::SuppressDocument(Document* aDoc) {
  // 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();
 }

 /**
--- a/dom/base/test/file_suppressed_events_inner.html
+++ 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>
--- a/dom/base/test/file_suppressed_events_middle.html
+++ 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>
--- a/dom/base/test/file_suppressed_events_top_xhr.html
+++ 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
@ -774,6 +774,13 @@ skip-if = debug == false
 [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]
--- a/dom/base/test/test_suppressed_events_nested_iframe.html
+++ 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
@ -14,6 +14,7 @@
 #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"
@ -2797,15 +2798,10 @@ void XMLHttpRequestMainThread::EnsureChannelContentType() {
  }
 }

-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;
@ -3028,7 +3024,15 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
  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()) {
@ -3036,10 +3040,8 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
              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);
        }
@ -3048,11 +3050,6 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,

    SuspendEventDispatching();
    StopProgressEventTimer();
-    auto scopeExit = MakeScopeExit([&] {
-      CancelSyncTimeoutTimer();
-      UnsuppressEventHandlingAndResume();
-      ResumeEventDispatching();
-    });

    SyncTimeoutType syncTimeoutType = MaybeStartSyncTimeoutTimer();
    if (syncTimeoutType == eErrorOrExpired) {
@ -3061,7 +3058,7 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
      return;
    }

-    nsAutoSyncOperation sync(mSuspendedDoc,
+    nsAutoSyncOperation sync(suspendedDoc,
                             SyncOperationBehavior::eSuspendInput);
    if (!SpinEventLoopUntil([&]() { return !mFlagSyncLooping; })) {
      aRv.Throw(NS_ERROR_UNEXPECTED);
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@ -304,7 +304,7 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
  bool IsCrossSiteCORSRequest() const;
  bool IsDeniedCrossSiteCORSRequest();

-  void UnsuppressEventHandlingAndResume();
+  void ResumeTimeout();

  void MaybeLowerChannelPriority();

@ -679,7 +679,6 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
  void StartTimeoutTimer();
  void HandleTimeoutCallback();

-  RefPtr<Document> mSuspendedDoc;
  nsCOMPtr<nsIRunnable> mResumeTimeoutRunnable;

  nsCOMPtr<nsITimer> mSyncTimeoutTimer;