Bug 1639328 - Make sure BrowserChilds for OOP iframes start in a consistent state. r=smaug

Right now they start with a FullyHidden() effect info, but with a "visible" widget, and thus active docshell and so on. That's no good :) Differential Revision: https://phabricator.services.mozilla.com/D86364
2020-08-10 14:50:27 +00:00 · 2020-08-10 14:50:27 +00:00 · 9d054b84e4
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@ -6237,11 +6237,13 @@ already_AddRefed<PresShell> Document::CreatePresShell(

 void Document::UpdateFrameRequestCallbackSchedulingState(
    PresShell* aOldPresShell) {
-  // If the condition for shouldBeScheduled changes to depend on some other
-  // variable, add UpdateFrameRequestCallbackSchedulingState() calls to the
-  // places where that variable can change.
-  bool shouldBeScheduled = mPresShell && IsEventHandlingEnabled() &&
-                           !mFrameRequestCallbacks.IsEmpty();
+  // If this condition changes to depend on some other variable, make sure to
+  // call UpdateFrameRequestCallbackSchedulingState() calls to the places where
+  // that variable can change. Also consider if you should change
+  // WouldScheduleFrameRequestCallbacks() instead of adding more stuff to this
+  // condition.
+  bool shouldBeScheduled =
+      WouldScheduleFrameRequestCallbacks() && !mFrameRequestCallbacks.IsEmpty();
  if (shouldBeScheduled == mFrameRequestCallbacksScheduled) {
    // nothing to do
    return;
@ -6269,7 +6271,7 @@ void Document::TakeFrameRequestCallbacks(nsTArray<FrameRequest>& aCallbacks) {
  mFrameRequestCallbacksScheduled = false;
 }

-bool Document::ShouldThrottleFrameRequests() {
+bool Document::ShouldThrottleFrameRequests() const {
  if (mStaticCloneCount > 0) {
    // Even if we're not visible, a static clone may be, so run at full speed.
    return false;
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@ -2682,10 +2682,17 @@ class Document : public nsINode,

  uint32_t EventHandlingSuppressed() const { return mEventsSuppressed; }

-  bool IsEventHandlingEnabled() {
+  bool IsEventHandlingEnabled() const {
    return !EventHandlingSuppressed() && mScriptGlobalObject;
  }

+  bool WouldScheduleFrameRequestCallbacks() const {
+    // If this function changes to depend on some other variable, make sure to
+    // call UpdateFrameRequestCallbackSchedulingState() calls to the places
+    // where that variable can change.
+    return mPresShell && IsEventHandlingEnabled();
+  }
+
  void DecreaseEventSuppression() {
    MOZ_ASSERT(mEventsSuppressed);
    --mEventsSuppressed;
@ -3017,7 +3024,7 @@ class Document : public nsINode,
   * throttled. We throttle requestAnimationFrame for documents which aren't
   * visible (e.g. scrolled out of the viewport).
   */
-  bool ShouldThrottleFrameRequests();
+  bool ShouldThrottleFrameRequests() const;

  // This returns true when the document tree is being teared down.
  bool InUnlinkOrDeletion() { return mInUnlinkOrDeletion; }
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@ -4401,3 +4401,14 @@ nsDOMWindowUtils::GetWebrtcRawDeviceId(nsAString& aRawDeviceId) {
  aRawDeviceId.AppendInt(rawDeviceId);
  return NS_OK;
 }
+
+NS_IMETHODIMP
+nsDOMWindowUtils::GetEffectivelyThrottlesFrameRequests(bool* aResult) {
+  Document* doc = GetDocument();
+  if (!doc) {
+    return NS_ERROR_FAILURE;
+  }
+  *aResult = !doc->WouldScheduleFrameRequestCallbacks() ||
+             doc->ShouldThrottleFrameRequests();
+  return NS_OK;
+}
--- a/dom/base/test/file_bug1639328.html
+++ b/dom/base/test/file_bug1639328.html
@ -0,0 +1,8 @@
+<!doctype html>
+<script>
+onmessage = function(e) {
+  parent.postMessage({
+    throttledFrameRequests: SpecialPowers.DOMWindowUtils.effectivelyThrottlesFrameRequests,
+  }, e.origin);
+};
+</script>
--- a/dom/base/test/mochitest.ini
+++ b/dom/base/test/mochitest.ini
@ -305,6 +305,8 @@ skip-if = (os == "android" || headless) # See
 skip-if = headless # fails in clipboard mode
 [test_bug166235.html]
 skip-if = headless # headless != clipboard
+[test_bug1639328.html]
+support-files = file_bug1639328.html
 [test_bug199959.html]
 [test_bug218236.html]
 [test_bug218277.html]
--- a/dom/base/test/test_bug1639328.html
+++ b/dom/base/test/test_bug1639328.html
@ -0,0 +1,54 @@
+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>Test for bug 1639328</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" href="/tests/SimpleTest/test.css">
+<iframe id="http" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
+<iframe id="https" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
+<iframe id="same-origin" src="file_bug1639328.html"></iframe>
+<iframe id="display-none-http" style="display: none" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
+<iframe id="display-none-https" style="display: none" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
+<iframe id="display-none-same-origin" style="display: none" src="file_bug1639328.html"></iframe>
+<script>
+SimpleTest.waitForExplicitFinish();
+
+function getOneMessage(frame) {
+  info("querying " + frame.src);
+  let resolve;
+  let promise = new Promise(r => { resolve = r; });
+  window.addEventListener("message", function(e) {
+    info("got " + JSON.stringify(e.data));
+    resolve(e.data);
+  }, { once: true });
+  frame.contentWindow.postMessage("ping", "*");
+  return promise;
+}
+
+async function ticks(n) {
+  for (let i = 0; i < n; ++i) {
+    await new Promise(resolve => requestAnimationFrame(resolve));
+  }
+}
+
+async function checkFrame(frame, shouldThrottle) {
+  let message = await getOneMessage(frame);
+  is(message.throttledFrameRequests, shouldThrottle, frame.id);
+}
+
+onload = async function() {
+  for (let frame of document.querySelectorAll("iframe")) {
+    let shouldThrottle = frame.style.display == "none";
+    await checkFrame(frame, shouldThrottle);
+    info("Switching display of " + frame.id);
+    frame.style.display = shouldThrottle ? "" : "none";
+    await ticks(2);
+    await checkFrame(frame, !shouldThrottle);
+    info("And switching display back for " + frame.id);
+    frame.style.display = shouldThrottle ? "none" : "";
+    await ticks(2);
+    await checkFrame(frame, shouldThrottle);
+  }
+
+  SimpleTest.finish();
+};
+</script>
--- a/dom/interfaces/base/nsIDOMWindowUtils.idl
+++ b/dom/interfaces/base/nsIDOMWindowUtils.idl
@ -2060,6 +2060,9 @@ interface nsIDOMWindowUtils : nsISupports {
  // Returns true if we are using overlay scrollbars.
  readonly attribute bool usesOverlayScrollbars;

+  // Returns true if we are effectively throttling frame requests.
+  readonly attribute bool effectivelyThrottlesFrameRequests;
+
  // Returns the ID for the underlying window widget, which can
  // be compared against the rawId from a nsIMediaDevice to determine
  // if the window is being shared.
--- a/dom/ipc/BrowserChild.cpp
+++ b/dom/ipc/BrowserChild.cpp
@ -524,8 +524,7 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
  // frames.
  if (mIsTopLevel) {
    nsContentUtils::SetScrollbarsVisibility(
-        window->GetDocShell(),
-        !!(mChromeFlags & nsIWebBrowserChrome::CHROME_SCROLLBARS));
+        docShell, !!(mChromeFlags & nsIWebBrowserChrome::CHROME_SCROLLBARS));
  }

  nsWeakPtr weakPtrThis = do_GetWeakReference(
@ -548,6 +547,11 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
  rv = mSessionStoreListener->Init();
  NS_ENSURE_SUCCESS(rv, rv);
 #endif
+
+  // We've all set up, make sure our visibility state is consistent. This is
+  // important for OOP iframes, which start off as hidden.
+  UpdateVisibility();
+
  return NS_OK;
 }

--- a/dom/ipc/BrowserChild.h
+++ b/dom/ipc/BrowserChild.h
@ -189,8 +189,8 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
               dom::BrowsingContext* aBrowsingContext, uint32_t aChromeFlags,
               bool aIsTopLevel);

-  nsresult Init(mozIDOMWindowProxy* aParent,
-                WindowGlobalChild* aInitialWindowChild);
+  MOZ_CAN_RUN_SCRIPT nsresult Init(mozIDOMWindowProxy* aParent,
+                                   WindowGlobalChild* aInitialWindowChild);

  /** Return a BrowserChild with the given attributes. */
  static already_AddRefed<BrowserChild> Create(
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@ -1014,7 +1014,8 @@ nsresult ContentChild::ProvideWindowCommon(

  // Now that |newChild| has had its IPC link established, call |Init| to set it
  // up.
-  nsPIDOMWindowOuter* parentWindow = parent ? parent->GetDOMWindow() : nullptr;
+  RefPtr<nsPIDOMWindowOuter> parentWindow =
+      parent ? parent->GetDOMWindow() : nullptr;
  if (NS_FAILED(newChild->Init(parentWindow, windowChild))) {
    return NS_ERROR_ABORT;
  }
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@ -104,15 +104,13 @@ class ContentChild final : public PContentChild,
    nsCString updateURL;
  };

-  nsresult ProvideWindowCommon(BrowserChild* aTabOpener,
-                               nsIOpenWindowInfo* aOpenWindowInfo,
-                               uint32_t aChromeFlags, bool aCalledFromJS,
-                               bool aWidthSpecified, nsIURI* aURI,
-                               const nsAString& aName,
-                               const nsACString& aFeatures, bool aForceNoOpener,
-                               bool aForceNoReferrer,
-                               nsDocShellLoadState* aLoadState,
-                               bool* aWindowIsNew, BrowsingContext** aReturn);
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult ProvideWindowCommon(
+      BrowserChild* aTabOpener, nsIOpenWindowInfo* aOpenWindowInfo,
+      uint32_t aChromeFlags, bool aCalledFromJS, bool aWidthSpecified,
+      nsIURI* aURI, const nsAString& aName, const nsACString& aFeatures,
+      bool aForceNoOpener, bool aForceNoReferrer,
+      nsDocShellLoadState* aLoadState, bool* aWindowIsNew,
+      BrowsingContext** aReturn);

  bool Init(MessageLoop* aIOLoop, base::ProcessId aParentPid,
            const char* aParentBuildID, UniquePtr<IPC::Channel> aChannel,
@ -493,7 +491,7 @@ class ContentChild final : public PContentChild,

  bool DeallocPFileDescriptorSetChild(PFileDescriptorSetChild*);

-  mozilla::ipc::IPCResult RecvConstructBrowser(
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY mozilla::ipc::IPCResult RecvConstructBrowser(
      ManagedEndpoint<PBrowserChild>&& aBrowserEp,
      ManagedEndpoint<PWindowGlobalChild>&& aWindowEp, const TabId& aTabId,
      const IPCTabContext& aContext, const WindowGlobalInit& aWindowInit,