Bug 1428670 - Part 2: Correctly mark all child frames as dirty when font inflation status changes. r=dbaron
☠☠ backed out by 5688a792346d ☠ ☠
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 29 Sep 2018 15:55:14 +0000
changeset 494631 faec1cb8ab5d14e4cc056d179859f6dae1606be0
parent 494630 34736c8507e625bfc1d792e14a6fbaf62b9fceb7
child 494632 1bf6b5fac1f9cc439338777531bbf9c50fc5656a
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1428670, 1308876, 707855
milestone64.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 1428670 - Part 2: Correctly mark all child frames as dirty when font inflation status changes. r=dbaron Before bug 1308876, child frames marked themselves as dirty during reflow if their parent was dirty, too. After bug 1308876, the point where dirtiness is being propagated to a frame's descendants has been shifted: Now, dirty parents are responsible for marking all their children as dirty, too, when the parent starts reflowing. This means that if a frame wants to mark a whole subtree as dirty *during its own* reflow, it's no longer sufficient to just mark the root of the subtree as dirty and then rely on all further children marking themselves as dirty as well when reflow reaches them. The font inflation code is one such case. When the font inflation data on a font inflation flow root has become dirty, or we're resizing the top-level frame (which because of the effective container width clamping from bug 707855 can affect the font inflation font size as well), we now need to explicitly mark all affected children as dirty. Differential Revision: https://phabricator.services.mozilla.com/D5577
layout/generic/ReflowInput.cpp
layout/generic/ReflowInput.h
layout/reftests/font-inflation/reftest.list
--- a/layout/generic/ReflowInput.cpp
+++ b/layout/generic/ReflowInput.cpp
@@ -343,41 +343,48 @@ ReflowInput::SetComputedHeight(nscoord a
     ComputedHeight() = aComputedHeight;
     LayoutFrameType frameType = mFrame->Type();
     if (frameType != LayoutFrameType::Viewport || !mWritingMode.IsVertical()) {
       InitResizeFlags(mFrame->PresContext(), frameType);
     }
   }
 }
 
+/* static */ void
+ReflowInput::MarkFrameChildrenDirty(nsIFrame* aFrame)
+{
+  if (aFrame->IsXULBoxFrame()) {
+    return;
+  }
+  // Mark all child frames as dirty.
+  //
+  // We don't do this for XUL boxes because they handle their child
+  // reflow separately.
+  for (nsIFrame::ChildListIterator childLists(aFrame); !childLists.IsDone();
+       childLists.Next()) {
+    for (nsIFrame* childFrame : childLists.CurrentList()) {
+      if (!childFrame->IsTableColGroupFrame()) {
+        childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+      }
+    }
+  }
+}
+
 void
 ReflowInput::Init(nsPresContext*     aPresContext,
                         const LogicalSize* aContainingBlockSize,
                         const nsMargin*    aBorder,
                         const nsMargin*    aPadding)
 {
-  if ((mFrame->GetStateBits() & NS_FRAME_IS_DIRTY) &&
-      !mFrame->IsXULBoxFrame()) {
-    // Mark all child frames as dirty.
-    //
-    // We don't do this for XUL boxes because they handle their child
-    // reflow separately.
-    //
+  if ((mFrame->GetStateBits() & NS_FRAME_IS_DIRTY)) {
     // FIXME (bug 1376530): It would be better for memory locality if we
     // did this as we went.  However, we need to be careful not to do
     // this twice for any particular child if we reflow it twice.  The
     // easiest way to accomplish that is to do it at the start.
-    for (nsIFrame::ChildListIterator childLists(mFrame);
-         !childLists.IsDone(); childLists.Next()) {
-      for (nsIFrame* childFrame : childLists.CurrentList()) {
-        if (!childFrame->IsTableColGroupFrame()) {
-          childFrame->AddStateBits(NS_FRAME_IS_DIRTY);
-        }
-      }
-    }
+    MarkFrameChildrenDirty(mFrame);
   }
 
   if (AvailableISize() == NS_UNCONSTRAINEDSIZE) {
     // Look up the parent chain for an orthogonal inline limit,
     // and reset AvailableISize() if found.
     for (const ReflowInput *parent = mParentReflowInput;
          parent != nullptr; parent = parent->mParentReflowInput) {
       if (parent->GetWritingMode().IsOrthogonalTo(mWritingMode) &&
@@ -632,19 +639,21 @@ ReflowInput::InitResizeFlags(nsPresConte
       // the second time we'd set it to false even without the
       // NS_FRAME_IS_DIRTY bit already set.
       if (mFrame->IsSVGForeignObjectFrame()) {
         // Foreign object frames use dirty bits in a special way.
         mFrame->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
         nsIFrame *kid = mFrame->PrincipalChildList().FirstChild();
         if (kid) {
           kid->AddStateBits(NS_FRAME_IS_DIRTY);
+          MarkFrameChildrenDirty(kid);
         }
       } else {
         mFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+        MarkFrameChildrenDirty(mFrame);
       }
 
       // Mark intrinsic widths on all descendants dirty.  We need to do
       // this (1) since we're changing the size of text and need to
       // clear text runs on text frames and (2) since we actually are
       // changing some intrinsic widths, but only those that live inside
       // of containers.
 
--- a/layout/generic/ReflowInput.h
+++ b/layout/generic/ReflowInput.h
@@ -1004,13 +1004,19 @@ protected:
   // in the aAxis dimension that goes inside the edge given by box-sizing;
   // aOutsideBoxSizing returns the rest.
   void CalculateBorderPaddingMargin(mozilla::LogicalAxis aAxis,
                                     nscoord aContainingBlockSize,
                                     nscoord* aInsideBoxSizing,
                                     nscoord* aOutsideBoxSizing) const;
 
   void CalculateBlockSideMargins(LayoutFrameType aFrameType);
+
+  /**
+   * Make all descendants of this frame dirty.
+   * Exceptions: XULBoxFrame and TabeColGroupFrame children.
+   */
+  static void MarkFrameChildrenDirty(nsIFrame* aFrame);
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ReflowInput_h
--- a/layout/reftests/font-inflation/reftest.list
+++ b/layout/reftests/font-inflation/reftest.list
@@ -67,17 +67,17 @@ test-pref(font.size.inflation.emPerLine,
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,0) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == fixed-height-body.html fixed-height-body-ref.html # Bug 1392106
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,0) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == fixed-height-body-child.html fixed-height-body-child-ref.html # Bug 1392106
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,0) == consecutive-inline.html consecutive-inline-ref.html
 
 # Ordinarily, reftests use a browser.viewport.desktopWidth of 800px, same as the
 # size of the reftest document.  The failure condition of the test below however
 # depends on the initial window size being smaller than the viewport the
 # MobileViewportManager eventually calculates, so we use a bigger value here.
-fails test-pref(font.size.inflation.emPerLine,20) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,0) test-pref(browser.viewport.desktopWidth,1000) == fixed-width-body-viewport.html fixed-width-body-viewport-ref.html
+test-pref(font.size.inflation.emPerLine,20) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,0) test-pref(browser.viewport.desktopWidth,1000) == fixed-width-body-viewport.html fixed-width-body-viewport-ref.html
 
 # The tests below use nonzero values of the lineThreshold preference.
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) == text-1.html text-1.html
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == list-1.html list-1-ref.html # Bug 1392106
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) == threshold-1a.html threshold-1a.html
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) == threshold-1b.html threshold-1b-ref.html
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) == threshold-1c.html threshold-1c-ref.html
 test-pref(font.size.inflation.emPerLine,15) test-pref(font.size.inflation.forceEnabled,true) test-pref(font.size.inflation.lineThreshold,100) == threshold-2.html threshold-2-ref.html