From eae8d21428acdb3fb842df091ded0eb276bcaaaa Mon Sep 17 00:00:00 2001
From: janekptacijarabaci <janekptacijarabaci@seznam.cz>
Date: Tue, 24 Apr 2018 21:39:13 +0200
Subject: moebius#138: Optimize operations on root of deeply-nested frame tree

https://github.com/MoonchildProductions/moebius/pull/138
---
 layout/style/nsStyleStruct.cpp                     |  28 ++---
 layout/style/test/mochitest.ini                    |   1 +
 .../test/test_dynamic_change_causing_reflow.html   | 107 +++++++++++++++++-
 .../test_viewport_scrollbar_causing_reflow.html    | 125 +++++++++++++++++++++
 4 files changed, 234 insertions(+), 27 deletions(-)
 create mode 100644 layout/style/test/test_viewport_scrollbar_causing_reflow.html

(limited to 'layout/style')

diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp
index eab11b80c..553239e0e 100644
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -1629,28 +1629,11 @@ nsStylePosition::CalcDifference(const nsStylePosition& aNewData,
   if (aOldStyleVisibility) {
     bool isVertical = WritingMode(aOldStyleVisibility).IsVertical();
     if (isVertical ? widthChanged : heightChanged) {
-      // Block-size changes can affect descendant intrinsic sizes due to
-      // replaced elements with percentage bsizes in descendants which
-      // also have percentage bsizes. This is handled via
-      // nsChangeHint_UpdateComputedBSize which clears intrinsic sizes
-      // for frames that have such replaced elements.
-      //
-      // We need to use nsChangeHint_ClearAncestorIntrinsics for
-      // block-size changes so we clear results of cached CSS Flex
-      // measuring reflows.
-      hint |= nsChangeHint_NeedReflow |
-              nsChangeHint_UpdateComputedBSize |
-              nsChangeHint_ReflowChangesSizeOrPosition |
-              nsChangeHint_ClearAncestorIntrinsics;
+      hint |= nsChangeHint_ReflowHintsForBSizeChange;
     }
 
     if (isVertical ? heightChanged : widthChanged) {
-      // None of our inline-size differences can affect descendant
-      // intrinsic sizes and none of them need to force children to
-      // reflow.
-      hint |= nsChangeHint_AllReflowHints &
-              ~(nsChangeHint_ClearDescendantIntrinsics |
-                nsChangeHint_NeedDirtyReflow);
+      hint |= nsChangeHint_ReflowHintsForISizeChange;
     }
   } else {
     if (widthChanged || heightChanged) {
@@ -3263,8 +3246,6 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
       || mDisplay != aNewData.mDisplay
       || mContain != aNewData.mContain
       || (mFloat == StyleFloat::None) != (aNewData.mFloat == StyleFloat::None)
-      || mOverflowX != aNewData.mOverflowX
-      || mOverflowY != aNewData.mOverflowY
       || mScrollBehavior != aNewData.mScrollBehavior
       || mScrollSnapTypeX != aNewData.mScrollSnapTypeX
       || mScrollSnapTypeY != aNewData.mScrollSnapTypeY
@@ -3276,6 +3257,11 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
     hint |= nsChangeHint_ReconstructFrame;
   }
 
+  if (mOverflowX != aNewData.mOverflowX
+      || mOverflowY != aNewData.mOverflowY) {
+    hint |= nsChangeHint_CSSOverflowChange;
+  }
+
   /* Note: When mScrollBehavior, mScrollSnapTypeX, mScrollSnapTypeY,
    * mScrollSnapPointsX, mScrollSnapPointsY, or mScrollSnapDestination are
    * changed, nsChangeHint_NeutralChange is not sufficient to enter
diff --git a/layout/style/test/mochitest.ini b/layout/style/test/mochitest.ini
index 406c6f901..8182691ca 100644
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -295,6 +295,7 @@ skip-if = toolkit == 'android'
 [test_variables.html]
 support-files = support/external-variable-url.css
 [test_video_object_fit.html]
+[test_viewport_scrollbar_causing_reflow.html]
 [test_viewport_units.html]
 [test_visited_image_loading.html]
 skip-if = toolkit == 'android' #TIMED_OUT
diff --git a/layout/style/test/test_dynamic_change_causing_reflow.html b/layout/style/test/test_dynamic_change_causing_reflow.html
index a941191f6..a5bb3045c 100644
--- a/layout/style/test/test_dynamic_change_causing_reflow.html
+++ b/layout/style/test/test_dynamic_change_causing_reflow.html
@@ -95,6 +95,90 @@ const gTestcases = [
     expectReflow: true,
   },
 
+  // * Changing 'overflow' on <body> should cause reflow,
+  // but not frame reconstruction
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: scroll",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: scroll",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: visible",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: auto",
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: visible",
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+
+  // * Changing 'overflow' on <html> should cause reflow,
+  // but not frame reconstruction
+  {
+    elem: document.documentElement,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.documentElement,
+    beforeStyle: "overflow: visible",
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+
+  // * Setting 'overflow' on arbitrary node should cause reflow as well as
+  // frame reconstruction
+  {
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: auto",
+    expectConstruction: true,
+    expectReflow: true,
+  },
+  {
+    beforeStyle: "overflow: auto",
+    afterStyle:  "overflow: visible",
+    expectConstruction: true,
+    expectReflow: true,
+  },
+
   // * Changing 'display' should cause frame construction and reflow.
   {
     beforeStyle: "display: inline",
@@ -135,23 +219,34 @@ function runOneTest(aTestcase)
     return;
   }
 
+  // Figure out which element we'll be tweaking (defaulting to gElem)
+  let elem = aTestcase.elem ?
+    aTestcase.elem : gElem;
+
+  // Verify that 'style' attribute is unset (avoid causing ourselves trouble):
+  if (elem.hasAttribute("style")) {
+    ok(false,
+       "test element has 'style' attribute already set! We're going to stomp " +
+       "on whatever's there when we clean up...");
+  }
+
   // Set the "before" style, and compose the first part of the message
   // to be used in our "is"/"isnot" invocations:
   let msgPrefix = "Changing style ";
   if (aTestcase.beforeStyle) {
-    gElem.setAttribute("style", aTestcase.beforeStyle);
+    elem.setAttribute("style", aTestcase.beforeStyle);
     msgPrefix += "from '" + aTestcase.beforeStyle + "' ";
   }
-  msgPrefix += "to '" + aTestcase.afterStyle + "' ";
+  msgPrefix += "on " + elem.nodeName + " ";
 
   // Establish initial counts:
-  let unusedVal = gElem.offsetHeight; // flush layout
+  let unusedVal = elem.offsetHeight; // flush layout
   let origFramesConstructed = gUtils.framesConstructed;
   let origFramesReflowed = gUtils.framesReflowed;
 
   // Make the change and flush:
-  gElem.setAttribute("style", aTestcase.afterStyle);
-  unusedVal = gElem.offsetHeight; // flush layout
+  elem.setAttribute("style", aTestcase.afterStyle);
+  unusedVal = elem.offsetHeight; // flush layout
 
   // Make our is/isnot assertions about whether things should have changed:
   checkFinalCount(gUtils.framesConstructed, origFramesConstructed,
@@ -162,7 +257,7 @@ function runOneTest(aTestcase)
                   "reflow");
 
   // Clean up!
-  gElem.removeAttribute("style");
+  elem.removeAttribute("style");
 }
 
 gTestcases.forEach(runOneTest);
diff --git a/layout/style/test/test_viewport_scrollbar_causing_reflow.html b/layout/style/test/test_viewport_scrollbar_causing_reflow.html
new file mode 100644
index 000000000..dfd7ec450
--- /dev/null
+++ b/layout/style/test/test_viewport_scrollbar_causing_reflow.html
@@ -0,0 +1,125 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1367568
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1367568</title>
+  <script type="application/javascript" 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=">Mozilla Bug 1367568</a>
+<div id="content">
+  <!-- Some fixed-width divs that we shouldn't have to reflow when the viewport
+       changes: -->
+  <div style="width: 100px">
+    fixed-width
+    <div>(child)</div>
+  </div>
+  <div style="position: absolute; width: 150px">
+    abs-fixed-width
+    <div>(child)</div>
+  </div>
+</div>
+<pre id="test">
+<script type="application/javascript">
+"use strict";
+
+/** Test for Bug 1367568 **/
+
+/**
+ * This test verifies that "overflow" changes on the <body> don't cause
+ * an unnecessarily large amount of reflow.
+ */
+
+// Vars used in setStyleAndMeasure that we really only have to look up once:
+const gUtils = SpecialPowers.getDOMWindowUtils(window);
+
+function setStyleAndMeasure(initialStyle, finalStyle) {
+  is(document.body.style.length, 0,
+     "Bug in test - body should start with empty style");
+  let unusedVal = document.body.offsetHeight; // flush layout
+  let constructCount = gUtils.framesConstructed;
+
+  document.body.style = initialStyle;
+  unusedVal = document.body.offsetHeight; // flush layout
+  let reflowCountBeforeTweak = gUtils.framesReflowed;
+
+  document.body.style = finalStyle;
+  unusedVal = document.body.offsetHeight; // flush layout
+  let reflowCountAfterTweak = gUtils.framesReflowed;
+
+  // Clean up:
+  document.body.style = "";
+
+  is(gUtils.framesConstructed, constructCount,
+     "Style tweak shouldn't have triggered frame construction");
+
+  // ...and return the delta:
+  return reflowCountAfterTweak - reflowCountBeforeTweak;
+}
+
+function main() {
+  // First, we sanity-check that our measurement make sense -- if we leave
+  // styles unchanged, we should measure no frames being reflowed:
+  let count = setStyleAndMeasure("width: 50px; height: 80px",
+                                 "width: 50px; height: 80px");
+  is(count, 0,
+     "Shouldn't reflow anything when we leave 'width' & 'height' unchanged");
+
+  // Now: see how many frames are reflowed when the "width" & "height" change.
+  // We'll use this as the reference when measuring reflow counts for various
+  // changes to "overflow" below.
+  count = setStyleAndMeasure("width: 50px; height: 80px",
+                             "width: 90px; height: 60px");
+  ok(count > 0,
+     "Should reflow some frames when 'width' & 'height' change");
+
+  // Expected maximum number of frames reflowed for "overflow" changes
+  // (+2 is to allow for reflowing scrollbars themselves):
+  const expectedMax = count + 2;
+
+  // Shared ending for messages in all ok() checks below:
+  const messageSuffix =
+    " shouldn't be greater than count for tweaking width/height on body (" +
+    expectedMax + ")";
+
+  // OK, here is where the relevant tests actually begin!!
+  // See how many frames we reflow for various tweaks to "overflow" on
+  // the body -- we expect the count to be no larger than |expectedMax|.
+  count = setStyleAndMeasure("", "overflow: scroll");
+  ok(count <= expectedMax,
+     "Reflow count when setting 'overflow: scroll' on body (" + count + ")" +
+     messageSuffix);
+
+  count = setStyleAndMeasure("", "overflow: hidden");
+  ok(count <= expectedMax,
+     "Reflow count when setting 'overflow: hidden' on body (" + count + ")" +
+     messageSuffix);
+
+  // Test removal of "overflow: scroll":
+  count = setStyleAndMeasure("overflow: scroll", "");
+  ok(count <= expectedMax,
+     "Reflow count when removing 'overflow: scroll' from body (" + count + ")" +
+     messageSuffix);
+
+  count = setStyleAndMeasure("overflow: hidden", "");
+  ok(count <= expectedMax,
+     "Reflow count when removing 'overflow: hidden' from body (" + count + ")" +
+     messageSuffix);
+
+  // Test change between two non-'visible' overflow values:
+  count = setStyleAndMeasure("overflow: scroll", "overflow: hidden");
+  ok(count <= expectedMax,
+     "Reflow count when changing 'overflow' on body (" + count + ")" +
+     messageSuffix);
+}
+
+main();
+
+</script>
+</pre>
+</body>
+</html>
-- 
cgit v1.2.3