Bug 1549909 - Use display list building order for walking parents in AddFramesForContainingBlock. r=kamidphish
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 15 May 2019 03:33:26 +0000
changeset 532713 2c73bd0fae85c2e877932e9f98ffd28fe91538f1
parent 532712 b2bd4ee26110bb05115a37fa6affd756b048ee46
child 532714 556e4f98beeb3442f8d588be05ae6afdaff44d13
push id11270
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 15:07:19 +0000
treeherdermozilla-beta@571bc76da583 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskamidphish
bugs1549909
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 1549909 - Use display list building order for walking parents in AddFramesForContainingBlock. r=kamidphish Differential Revision: https://phabricator.services.mozilla.com/D30453
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/painting/RetainedDisplayListBuilder.cpp
layout/painting/crashtests/1549909.html
layout/painting/crashtests/crashtests.list
layout/painting/nsDisplayList.cpp
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -4702,16 +4702,23 @@ nsIFrame* nsLayoutUtils::GetParentOrPlac
 }
 
 nsIFrame* nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(nsIFrame* aFrame) {
   nsIFrame* f = GetParentOrPlaceholderFor(aFrame);
   if (f) return f;
   return GetCrossDocParentFrame(aFrame);
 }
 
+nsIFrame* nsLayoutUtils::GetDisplayListParent(nsIFrame* aFrame) {
+  if (aFrame->GetStateBits() & NS_FRAME_IS_PUSHED_FLOAT) {
+    return aFrame->GetParent();
+  }
+  return nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(aFrame);
+}
+
 nsIFrame* nsLayoutUtils::GetNextContinuationOrIBSplitSibling(nsIFrame* aFrame) {
   nsIFrame* result = aFrame->GetNextContinuation();
   if (result) return result;
 
   if ((aFrame->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT) != 0) {
     // We only store the ib-split sibling annotation with the first
     // frame in the continuation chain. Walk back to find that frame now.
     aFrame = aFrame->FirstContinuation();
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -1392,16 +1392,25 @@ class nsLayoutUtils {
 
   /**
    * If aFrame is an out of flow frame, return its placeholder, otherwise
    * return its (possibly cross-doc) parent.
    */
   static nsIFrame* GetParentOrPlaceholderForCrossDoc(nsIFrame* aFrame);
 
   /**
+   * Returns the frame that would act as the parent of aFrame when
+   * descending through the frame tree in display list building.
+   * Usually the same as GetParentOrPlaceholderForCrossDoc, except
+   * that pushed floats are treated as children of their containing
+   * block.
+   */
+  static nsIFrame* GetDisplayListParent(nsIFrame* aFrame);
+
+  /**
    * Get a frame's next-in-flow, or, if it doesn't have one, its
    * block-in-inline-split sibling.
    */
   static nsIFrame* GetNextContinuationOrIBSplitSibling(nsIFrame* aFrame);
 
   /**
    * Get the first frame in the continuation-plus-ib-split-sibling chain
    * containing aFrame.
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -292,21 +292,17 @@ bool AnyContentAncestorModified(nsIFrame
     if (f->IsFrameModified()) {
       return true;
     }
 
     if (aStopAtFrame && f == aStopAtFrame) {
       break;
     }
 
-    if (f->GetStateBits() & NS_FRAME_IS_PUSHED_FLOAT) {
-      f = f->GetParent();
-    } else {
-      f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f);
-    }
+    f = nsLayoutUtils::GetDisplayListParent(f);
   }
 
   return false;
 }
 
 static Maybe<const ActiveScrolledRoot*> SelectContainerASR(
     const DisplayItemClipChain* aClipChain, const ActiveScrolledRoot* aItemASR,
     Maybe<const ActiveScrolledRoot*>& aContainerASR) {
@@ -1201,17 +1197,17 @@ static void AddFramesForContainingBlock(
 // descendants of a modified frame (us, or another frame we'll get to soon).
 // This is combined with the work required for MarkFrameForDisplayIfVisible,
 // so that we can avoid an extra ancestor walk, and we can reuse the flag
 // to detect when we've already visited an ancestor (and thus all further
 // ancestors must also be visited).
 static void FindContainingBlocks(nsIFrame* aFrame,
                                  nsTArray<nsIFrame*>& aExtraFrames) {
   for (nsIFrame* f = aFrame; f;
-       f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f)) {
+       f = nsLayoutUtils::GetDisplayListParent(f)) {
     if (f->ForceDescendIntoIfVisible()) return;
     f->SetForceDescendIntoIfVisible(true);
     CRR_LOG("Considering OOFs for %p\n", f);
 
     AddFramesForContainingBlock(f, f->GetChildList(nsIFrame::kFloatList),
                                 aExtraFrames);
     AddFramesForContainingBlock(f, f->GetChildList(f->GetAbsoluteListID()),
                                 aExtraFrames);
new file mode 100644
--- /dev/null
+++ b/layout/painting/crashtests/1549909.html
@@ -0,0 +1,9 @@
+<style>
+* {
+  -webkit-column-break-after: always;
+  float: left;
+  column-width: 0px;
+}
+</style>
+}
+<video controls="controls">
--- a/layout/painting/crashtests/crashtests.list
+++ b/layout/painting/crashtests/crashtests.list
@@ -13,9 +13,10 @@ load 1454105-1.html
 load 1455944-1.html
 load 1465305-1.html
 load 1468124-1.html
 load 1469472.html
 load 1477831-1.html
 load 1504033.html
 load 1514544-1.html
 load 1547420-1.html
+load 1549909.html
 
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1250,17 +1250,17 @@ void nsDisplayListBuilder::MarkFrameForD
 void nsDisplayListBuilder::AddFrameMarkedForDisplayIfVisible(nsIFrame* aFrame) {
   mFramesMarkedForDisplayIfVisible.AppendElement(aFrame);
 }
 
 void nsDisplayListBuilder::MarkFrameForDisplayIfVisible(
     nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
   AddFrameMarkedForDisplayIfVisible(aFrame);
   for (nsIFrame* f = aFrame; f;
-       f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f)) {
+       f = nsLayoutUtils::GetDisplayListParent(f)) {
     if (f->ForceDescendIntoIfVisible()) return;
     f->SetForceDescendIntoIfVisible(true);
     if (f == aStopAtFrame) {
       // we've reached a frame that we know will be painted, so we can stop.
       break;
     }
   }
 }
@@ -1380,17 +1380,17 @@ static void UnmarkFrameForDisplay(nsIFra
       // we've reached a frame that we know will be painted, so we can stop.
       break;
     }
   }
 }
 
 static void UnmarkFrameForDisplayIfVisible(nsIFrame* aFrame) {
   for (nsIFrame* f = aFrame; f;
-       f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f)) {
+       f = nsLayoutUtils::GetDisplayListParent(f)) {
     if (!f->ForceDescendIntoIfVisible()) return;
     f->SetForceDescendIntoIfVisible(false);
   }
 }
 
 nsDisplayListBuilder::~nsDisplayListBuilder() {
   NS_ASSERTION(mFramesMarkedForDisplay.Length() == 0,
                "All frames should have been unmarked");