Backed out 3 changesets (bug 1847584) for causing vsync failures. CLOSED TREE

Backed out changeset bdfc3c7388cf (bug 1847584) Backed out changeset 54b34c76db47 (bug 1847584) Backed out changeset 03bf43b4ae07 (bug 1847584)
2023-08-09 15:47:03 +03:00 · 2023-08-09 15:47:03 +03:00 · 428aa75e35
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@ -29,7 +29,6 @@ skip-if =
  debug
  os == "win" # Bug 1775626
  os == "linux" && socketprocess_networking # Bug 1382809, bug 1369959
-[browser_hidden_browser_vsync.js]
 [browser_panel_vsync.js]
 support-files =
  !/browser/components/downloads/test/browser/head.js
--- a/browser/base/content/test/performance/browser_hidden_browser_vsync.js
+++ b/browser/base/content/test/performance/browser_hidden_browser_vsync.js
@ -1,56 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-add_task(async function test_hidden_sidebar() {
-  let b = document.createXULElement("browser");
-  for (let [k, v] of Object.entries({
-    type: "content",
-    disablefullscreen: "true",
-    disablehistory: "true",
-    flex: "1",
-    style: "min-width: 300px",
-    message: "true",
-    remote: "true",
-    maychangeremoteness: "true",
-  })) {
-    b.setAttribute(k, v);
-  }
-  let mainBrowser = gBrowser.selectedBrowser;
-  let panel = gBrowser.getPanel(mainBrowser);
-  panel.append(b);
-  let loaded = BrowserTestUtils.browserLoaded(b);
-  BrowserTestUtils.loadURIString(
-    b,
-    `data:text/html,<!doctype html><style>
-  @keyframes fade-in {
-   from {
-     opacity: .25;
-   }
-   to {
-     opacity: 1;
-   }
-  </style>
-  <div style="
-    animation-name: fade-in;
-    animation-direction: alternate;
-    animation-duration: 1s;
-    animation-iteration-count: infinite;
-    animation-timing-function: ease-in-out;
-    background-color: red;
-    height: 500px;
-    width: 100%;
-  "></div>`
-  );
-  await loaded;
-  ok(b, "Browser was created.");
-  await SpecialPowers.spawn(b, [], async () => {
-    await new Promise(r =>
-      content.requestAnimationFrame(() => content.requestAnimationFrame(r))
-    );
-  });
-  b.hidden = true;
-  ok(b.hidden, "Browser should be hidden.");
-  // Now the framework will test to see vsync goes away
-});
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@ -3534,15 +3534,11 @@ void BrowsingContext::DidSet(FieldIndex<IDX_IsUnderHiddenEmbedderElement>,
  if (!shell) {
    return;
  }
+
  const bool newValue = IsUnderHiddenEmbedderElement();
  if (NS_WARN_IF(aOldValue == newValue)) {
    return;
  }
-
-  if (auto* bc = BrowserChild::GetFrom(shell)) {
-    bc->UpdateVisibility();
-  }
-
  if (PresShell* presShell = shell->GetPresShell()) {
    presShell->SetIsUnderHiddenEmbedderElement(newValue);
  }
@ -3561,7 +3557,7 @@ void BrowsingContext::DidSet(FieldIndex<IDX_IsUnderHiddenEmbedderElement>,
    }

    bool embedderFrameIsHidden = true;
-    if (auto* embedderFrame = embedderElement->GetPrimaryFrame()) {
+    if (auto embedderFrame = embedderElement->GetPrimaryFrame()) {
      embedderFrameIsHidden = !embedderFrame->StyleVisibility()->IsVisible();
    }

--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@ -7098,12 +7098,6 @@ bool Document::ShouldThrottleFrameRequests() const {
    return true;
  }

-  if (mPresShell->IsUnderHiddenEmbedderElement()) {
-    // For display: none and visibility: hidden we always throttle, for
-    // consistency with OOP iframes.
-    return true;
-  }
-
  Element* el = GetEmbedderElement();
  if (!el) {
    // If we're not in-process, our refresh driver is throttled separately (via
@ -7121,7 +7115,7 @@ bool Document::ShouldThrottleFrameRequests() const {
  // acceptable / unlikely to be human-perceivable, though we could improve on
  // it if needed by adding an intersection margin or something of that sort.
  const IntersectionInput input = DOMIntersectionObserver::ComputeInput(
-      *el->OwnerDoc(), /* aRoot = */ nullptr, /* aRootMargin = */ nullptr);
+      *el->OwnerDoc(), /* aRoot = */ nullptr, /* aMargin = */ nullptr);
  const IntersectionOutput output =
      DOMIntersectionObserver::Intersect(input, *el);
  return !output.Intersects();
--- a/dom/base/test/test_bug1639328.html
+++ b/dom/base/test/test_bug1639328.html
@ -16,9 +16,6 @@
 <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>
-<iframe id="vis-hidden-http" style="visibility: hidden" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
-<iframe id="vis-hidden-https" style="visibility: hidden" src="https://example.com/tests/dom/base/test/file_bug1639328.html"></iframe>
-<iframe id="vis-hidden-same-origin" style="visibility: hidden" src="file_bug1639328.html"></iframe>
 <script>
 SimpleTest.waitForExplicitFinish();

@ -51,30 +48,19 @@ async function checkFrame(frame, shouldThrottle) {
  is(message.throttledFrameRequests, shouldThrottle, frame.id);
 }

-function shouldThrottle(frame) {
-  return frame.style.display == "none" || frame.style.visibility == "hidden";
-}
-
 onload = async function() {
  await SimpleTest.promiseFocus(window);
  await ticks(2);
  is(SpecialPowers.DOMWindowUtils.effectivelyThrottlesFrameRequests, false, "Should not be throttling main page");
  for (let frame of document.querySelectorAll("iframe")) {
-    let originalShouldThrottle = shouldThrottle(frame);
-    await checkFrame(frame, originalShouldThrottle);
-    for (let prop of ["display", "visibility"]) {
-      info(`Switching ${prop} of ${frame.id}`);
-      let orig = frame.style[prop];
-      let throttledValue = prop == "display" ? "none" : "hidden";
-      frame.style[prop] = orig == throttledValue ? "" : throttledValue;
-      if (orig != throttledValue) {
-        is(shouldThrottle(frame), true, `Should throttle for ${prop}: ${throttledValue}`);
-      }
-      await checkFrame(frame, shouldThrottle(frame));
-      info(`Switching ${prop} back for ${frame.id}`);
-      frame.style[prop] = orig;
-      await checkFrame(frame, originalShouldThrottle);
-    }
+    let shouldThrottle = frame.style.display == "none";
+    await checkFrame(frame, shouldThrottle);
+    info("Switching display of " + frame.id);
+    frame.style.display = shouldThrottle ? "" : "none";
+    await checkFrame(frame, !shouldThrottle);
+    info("And switching display back for " + frame.id);
+    frame.style.display = shouldThrottle ? "none" : "";
+    await checkFrame(frame, shouldThrottle);
  }

  SimpleTest.finish();
--- a/dom/base/test/test_bug1730284.html
+++ b/dom/base/test/test_bug1730284.html
@ -36,7 +36,7 @@
 </style>
 <iframe class="visible"></iframe>
 <iframe class="display-none" data-throttled-expected></iframe>
-<iframe class="vis-hidden" data-throttled-expected></iframe>
+<iframe class="vis-hidden"></iframe>
 <iframe class="transparent"></iframe>
 <iframe class="zero-size"></iframe>
 <div class="scroller">
--- a/dom/ipc/BrowserChild.cpp
+++ b/dom/ipc/BrowserChild.cpp
@ -2434,7 +2434,7 @@ mozilla::ipc::IPCResult BrowserChild::RecvUpdateNativeWindowHandle(
 }

 mozilla::ipc::IPCResult BrowserChild::RecvDestroy() {
-  MOZ_ASSERT(!mDestroyed);
+  MOZ_ASSERT(mDestroyed == false);
  mDestroyed = true;

  nsTArray<PContentPermissionRequestChild*> childArray =
@ -2444,7 +2444,7 @@ mozilla::ipc::IPCResult BrowserChild::RecvDestroy() {
  // Need to close undeleted ContentPermissionRequestChilds before tab is
  // closed.
  for (auto& permissionRequestChild : childArray) {
-    auto* child = static_cast<RemotePermissionRequest*>(permissionRequestChild);
+    auto child = static_cast<RemotePermissionRequest*>(permissionRequestChild);
    child->Destroy();
  }

@ -2715,7 +2715,7 @@ void BrowserChild::InitAPZState() {
  if (!mCompositorOptions->UseAPZ()) {
    return;
  }
-  auto* cbc = CompositorBridgeChild::Get();
+  auto cbc = CompositorBridgeChild::Get();

  // Initialize the ApzcTreeManager. This takes multiple casts because of ugly
  // multiple inheritance.
@ -2747,7 +2747,7 @@ IPCResult BrowserChild::RecvUpdateEffects(const EffectsInfo& aEffects) {
  bool needInvalidate = false;
  if (mEffectsInfo.IsVisible() && aEffects.IsVisible() &&
      mEffectsInfo != aEffects) {
-    // If we are staying visible and either the visrect or scale changed we need
+    // if we are staying visible and either the visrect or scale changed we need
    // to invalidate
    needInvalidate = true;
  }
@ -2756,12 +2756,14 @@ IPCResult BrowserChild::RecvUpdateEffects(const EffectsInfo& aEffects) {
  UpdateVisibility();

  if (needInvalidate) {
-    if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation())) {
+    nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation());
+    if (docShell) {
      // We don't use BrowserChildBase::GetPresShell() here because that would
      // create a content viewer if one doesn't exist yet. Creating a content
      // viewer can cause JS to run, which we want to avoid.
      // nsIDocShell::GetPresShell returns null if no content viewer exists yet.
-      if (RefPtr<PresShell> presShell = docShell->GetPresShell()) {
+      RefPtr<PresShell> presShell = docShell->GetPresShell();
+      if (presShell) {
        if (nsIFrame* root = presShell->GetRootFrame()) {
          root->InvalidateFrame();
        }
@ -2777,38 +2779,15 @@ bool BrowserChild::IsVisible() {
 }

 void BrowserChild::UpdateVisibility() {
-  const bool shouldBeVisible = [&] {
-    // If we're known to be visibility: hidden / display: none, just return
-    // false here, we're pretty sure we don't want to be considered visible
-    // here.
-    if (mBrowsingContext && mBrowsingContext->IsUnderHiddenEmbedderElement()) {
-      return false;
-    }
-    // For OOP iframes, include viewport visibility. For top-level <browser>
-    // elements we don't use this, because the front-end relies on using
-    // `mRenderLayers` when invisible for tab warming purposes.
-    //
-    // An alternative, maybe more consistent approach would be to add an opt-in
-    // into this behavior for top-level tabs managed by the tab-switcher
-    // instead...
-    if (!mIsTopLevel && !mEffectsInfo.IsVisible()) {
-      return false;
-    }
-    // If we're explicitly told not to render layers, we're also invisible.
-    if (!mRenderLayers) {
-      return false;
-    }
-    return true;
-  }();
+  bool shouldBeVisible = mIsTopLevel ? mRenderLayers : mEffectsInfo.IsVisible();
+  bool isVisible = IsVisible();

-  const bool isVisible = IsVisible();
-  if (shouldBeVisible == isVisible) {
-    return;
-  }
-  if (shouldBeVisible) {
-    MakeVisible();
-  } else {
-    MakeHidden();
+  if (shouldBeVisible != isVisible) {
+    if (shouldBeVisible) {
+      MakeVisible();
+    } else {
+      MakeHidden();
+    }
  }
 }

--- a/dom/ipc/BrowserChild.h
+++ b/dom/ipc/BrowserChild.h
@ -190,7 +190,7 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
  // Let managees query if it is safe to send messages.
  bool IsDestroyed() const { return mDestroyed; }

-  TabId GetTabId() const {
+  const TabId GetTabId() const {
    MOZ_ASSERT(mUniqueId != 0);
    return mUniqueId;
  }
@ -456,8 +456,8 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
   * activated widget, retained layer tree, etc.  (Respectively,
   * made not visible.)
   */
-  void UpdateVisibility();
-  void MakeVisible();
+  MOZ_CAN_RUN_SCRIPT void UpdateVisibility();
+  MOZ_CAN_RUN_SCRIPT void MakeVisible();
  void MakeHidden();
  void PresShellActivenessMaybeChanged();