Bug 1545516 - Don't flush parent document layout for detached frames from EnsureSizeAndPositionUpToDate. r=dholbert,bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 20 Apr 2019 14:41:55 +0000
changeset 470294 bbdaf05c9d16b95a9bea1bf1125d69fb58aacd81
parent 470293 a092972b53f0e566a36770e7b03363036ff820ec
child 470295 581755bf88997188199042fd77a45336bda434d9
push id35893
push userdvarga@mozilla.com
push dateSat, 20 Apr 2019 21:44:54 +0000
treeherdermozilla-central@581755bf8899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, bzbarsky
bugs1545516
milestone68.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 1545516 - Don't flush parent document layout for detached frames from EnsureSizeAndPositionUpToDate. r=dholbert,bzbarsky And add a test for the same not happening already for normal flushes. Differential Revision: https://phabricator.services.mozilla.com/D28192
dom/base/nsGlobalWindowOuter.cpp
layout/style/test/mochitest.ini
layout/style/test/test_flushing_frame.html
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -7352,19 +7352,23 @@ void nsGlobalWindowOuter::FlushPendingNo
     mDoc->FlushPendingNotifications(aType);
   }
 }
 
 void nsGlobalWindowOuter::EnsureSizeAndPositionUpToDate() {
   // If we're a subframe, make sure our size is up to date.  It's OK that this
   // crosses the content/chrome boundary, since chrome can have pending reflows
   // too.
-  nsGlobalWindowOuter* parent = nsGlobalWindowOuter::Cast(GetPrivateParent());
-  if (parent) {
-    parent->FlushPendingNotifications(FlushType::Layout);
+  //
+  // Make sure to go through the document chain rather than the window chain to
+  // not flush on detached iframes, see bug 1545516.
+  if (mDoc) {
+    if (RefPtr<Document> parent = mDoc->GetParentDocument()) {
+      parent->FlushPendingNotifications(FlushType::Layout);
+    }
   }
 }
 
 already_AddRefed<nsISupports> nsGlobalWindowOuter::SaveWindowState() {
   if (!mContext || !GetWrapperPreserveColor()) {
     // The window may be getting torn down; don't bother saving state.
     return nullptr;
   }
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -220,16 +220,17 @@ skip-if = toolkit == 'android' #bug 5366
 [test_flexbox_focus_order.html]
 [test_flexbox_layout.html]
 support-files = flexbox_layout_testcases.js
 [test_flexbox_order.html]
 [test_flexbox_order_abspos.html]
 [test_flexbox_order_table.html]
 [test_flexbox_reflow_counts.html]
 skip-if = verify
+[test_flushing_frame.html]
 [test_font_face_cascade.html]
 [test_font_face_parser.html]
 [test_font_family_parsing.html]
 [test_font_family_serialization.html]
 [test_font_loading_api.html]
 support-files =
   BitPattern.woff
   file_font_loading_api_vframe.html
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_flushing_frame.html
@@ -0,0 +1,40 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>
+  Test for bug 1545516: We don't flush layout unnecessarily on the parent
+  document when the frame is already disconnected.
+</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<div id="content"></div>
+<script>
+  SimpleTest.waitForExplicitFinish();
+  const iframe = document.createElement("iframe");
+  const content = document.querySelector("#content");
+  const parentUtils = SpecialPowers.getDOMWindowUtils(window);
+  iframe.onload = function() {
+    const win = iframe.contentWindow;
+    iframe.offsetTop; // flush layout
+    content.style.display = "inline"; // Dirty style with something that will reframe.
+
+    const previousConstructCount = parentUtils.framesConstructed;
+    let pagehideRan = false;
+    win.addEventListener("pagehide", function() {
+      pagehideRan = true;
+      win.foo = win.innerWidth;
+      is(parentUtils.framesConstructed, previousConstructCount, "innerWidth shouldn't have flushed parent document layout")
+      win.bar = win.document.documentElement.offsetHeight;
+      is(parentUtils.framesConstructed, previousConstructCount, "offsetHeight shouldn't have flushed parent document layout")
+      win.baz = win.getComputedStyle(win.document.documentElement).color;
+      is(parentUtils.framesConstructed, previousConstructCount, "getComputedStyle shouldn't have flushed parent document layout")
+    });
+
+    iframe.remove(); // Remove the iframe
+    is(pagehideRan, true, "pagehide handler should've ran");
+    is(parentUtils.framesConstructed, previousConstructCount, "Nothing should've flushed the parent document layout yet");
+    content.offsetTop;
+    isnot(parentUtils.framesConstructed, previousConstructCount, "We should've flushed layout now");
+    SimpleTest.finish();
+  };
+  document.body.appendChild(iframe);
+</script>