Backed out changeset 200ab1e8d571 (bug 1533317) for multiple reftest failures
authorshindli <shindli@mozilla.com>
Mon, 11 Mar 2019 02:26:27 +0200
changeset 521304 d0a427040b4f57fffd1e0691006461bbd5a2eaa8
parent 521303 d9ed942f6b002499529e25bf7b31b118051fcc73
child 521305 7f22a46bf802b3b0a535b56aa43d97e90688c32e
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1533317
milestone67.0a1
backs out200ab1e8d571aca8e147a89c75394273c808c388
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
Backed out changeset 200ab1e8d571 (bug 1533317) for multiple reftest failures
layout/generic/nsFrame.cpp
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsIFrame.h
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
layout/reftests/display-list/1533317-1-ref.html
layout/reftests/display-list/1533317-1.html
layout/reftests/display-list/reftest.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -914,27 +914,16 @@ bool nsIFrame::HasDisplayItem(uint32_t a
   for (nsDisplayItem* i : *items) {
     if (i->GetPerFrameKey() == aKey) {
       return true;
     }
   }
   return false;
 }
 
-void nsIFrame::DiscardItems() {
-  DisplayItemArray* items = GetProperty(DisplayItems());
-  if (!items) {
-    return;
-  }
-
-  for (nsDisplayItem* i : *items) {
-    i->SetDontReuse();
-  }
-}
-
 void nsIFrame::RemoveDisplayItemDataForDeletion() {
   // Destroying a WebRenderUserDataTable can cause destruction of other objects
   // which can remove frame properties in their destructor. If we delete a frame
   // property it runs the destructor of the stored object in the middle of
   // updating the frame property table, so if the destruction of that object
   // causes another update to the frame property table it would leave the frame
   // property table in an inconsistent state. So we remove it from the table and
   // then destroy it. (bug 1530657)
@@ -3430,52 +3419,47 @@ static nsDisplayItem* WrapInWrapList(nsD
                                      const ActiveScrolledRoot* aContainerASR,
                                      bool aBuiltContainerItem = false) {
   nsDisplayItem* item = aList->GetBottom();
   if (!item) {
     return nullptr;
   }
 
   // We need a wrap list if there are multiple items, or if the single
-  // item has a different frame. This can change in a partial build depending
-  // on which items we build, so we need to ensure that we don't transition
-  // to/from a wrap list without invalidating correctly.
-  bool needsWrapList =
-      item->GetAbove() || item->Frame() != aFrame || item->GetChildren();
-
-  // If we have an explicit container item (that can't change without an
-  // invalidation) or we're doing a full build and don't need a wrap list, then
-  // we can skip adding one.
-  if (aBuiltContainerItem || (!aBuilder->IsPartialUpdate() && !needsWrapList)) {
+  // item has a different frame.
+  bool needsWrapList = item->GetAbove() || item->Frame() != aFrame;
+
+  // If we don't need a wrap list, and we're doing a full build, or if
+  // we have an explicit container item that guarantees to wrap all other
+  // items then we can skip.
+  if (!needsWrapList && (!aBuilder->IsPartialUpdate() || aBuiltContainerItem)) {
     aList->RemoveBottom();
     return item;
   }
 
   // If we're doing a partial build and we didn't need a wrap list
   // previously then we can try to work from there.
   if (aBuilder->IsPartialUpdate() &&
       !aFrame->HasDisplayItem(uint32_t(DisplayItemType::TYPE_WRAP_LIST))) {
-    // If we now need a wrap list, we must previously have had no display items
-    // or a single one belonging to this frame. Mark the item itself as
-    // discarded so that RetainedDisplayListBuilder uses the ones we just built.
-    // We don't want to mark the frame as modified as that would invalidate
-    // positioned descendants that might be outside of this list, and might not
-    // have been rebuilt this time.
+    // If we now need a wrap list, mark this frame as modified so that merging
+    // removes the unwrapped item. We don't need to add to the dirty rect since
+    // not needing a wrap list means that we must have only had a display item
+    // from this frame, which we're already building items for. All new items
+    // will have already been included in the dirty area.
     if (needsWrapList) {
-      aFrame->DiscardItems();
+      aBuilder->MarkFrameModifiedDuringBuilding(aFrame);
     } else {
       aList->RemoveBottom();
       return item;
     }
   }
 
-  // The last case we could try to handle is when we previously had a wrap list,
-  // but no longer need it. Unfortunately we can't differentiate this case from
-  // a partial build where other children exist but we just didn't build them
-  // this time.
+  // The last case is when we previously had a wrap list, but no longer
+  // need it. Unfortunately we can't differentiate this case from a partial
+  // build where other children exist but we just didn't build them.
   // TODO:RetainedDisplayListBuilder's merge phase has the full list and
   // could strip them out.
 
   // Clear clip rect for the construction of the items below. Since we're
   // clipping all their contents, they themselves don't need to be clipped.
   return MakeDisplayItem<nsDisplayWrapList>(aBuilder, aFrame, aList,
                                             aContainerASR, true);
 }
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -3737,18 +3737,17 @@ void ScrollFrameHelper::MaybeAddTopLayer
   if (mIsRoot) {
     if (ViewportFrame* viewportFrame = do_QueryFrame(mOuter->GetParent())) {
       nsDisplayList topLayerList;
       viewportFrame->BuildDisplayListForTopLayer(aBuilder, &topLayerList);
       if (!topLayerList.IsEmpty()) {
         // Wrap the whole top layer in a single item with maximum z-index,
         // and append it at the very end, so that it stays at the topmost.
         nsDisplayWrapList* wrapList = MakeDisplayItem<nsDisplayWrapList>(
-            aBuilder, viewportFrame, &topLayerList,
-            aBuilder->CurrentActiveScrolledRoot(), false, 2);
+            aBuilder, viewportFrame, &topLayerList);
         wrapList->SetOverrideZIndex(
             std::numeric_limits<decltype(wrapList->ZIndex())>::max());
         aLists.PositionedDescendants()->AppendToTop(wrapList);
       }
     }
   }
 }
 
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -4133,17 +4133,16 @@ class nsIFrame : public nsQueryFrame {
   }
 
   void AddDisplayItem(nsDisplayItem* aItem);
   bool RemoveDisplayItem(nsDisplayItem* aItem);
   void RemoveDisplayItemDataForDeletion();
   bool HasDisplayItems();
   bool HasDisplayItem(nsDisplayItem* aItem);
   bool HasDisplayItem(uint32_t aKey);
-  void DiscardItems();
 
   bool ForceDescendIntoIfVisible() const { return mForceDescendIntoIfVisible; }
   void SetForceDescendIntoIfVisible(bool aForce) {
     mForceDescendIntoIfVisible = aForce;
   }
 
   bool BuiltDisplayList() { return mBuiltDisplayList; }
   void SetBuiltDisplayList(bool aBuilt) { mBuiltDisplayList = aBuilt; }
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -3098,18 +3098,17 @@ nsDisplayItem::nsDisplayItem(nsDisplayLi
                              bool aAnonymous)
     : mFrame(aFrame),
       mActiveScrolledRoot(aActiveScrolledRoot),
       mAnimatedGeometryRoot(nullptr),
       mForceNotVisible(aBuilder->IsBuildingInvisibleItems()),
       mDisableSubpixelAA(false),
       mReusedItem(false),
       mBackfaceHidden(mFrame->In3DContextAndBackfaceIsHidden()),
-      mPaintRectValid(false),
-      mCanBeReused(true)
+      mPaintRectValid(false)
 #ifdef MOZ_DUMP_PAINTING
       ,
       mPainted(false)
 #endif
 {
   MOZ_COUNT_CTOR(nsDisplayItem);
   if (aBuilder->IsRetainingDisplayList() && !aAnonymous) {
     mFrame->AddDisplayItem(this);
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -2096,18 +2096,17 @@ class nsDisplayItem : public nsDisplayIt
         mClip(nullptr),
         mActiveScrolledRoot(nullptr),
         mReferenceFrame(nullptr),
         mAnimatedGeometryRoot(nullptr),
         mForceNotVisible(false),
         mDisableSubpixelAA(false),
         mReusedItem(false),
         mBackfaceHidden(mFrame->In3DContextAndBackfaceIsHidden()),
-        mPaintRectValid(false),
-        mCanBeReused(true)
+        mPaintRectValid(false)
 #ifdef MOZ_DUMP_PAINTING
         ,
         mPainted(false)
 #endif
   {
     MOZ_COUNT_CTOR(nsDisplayItem);
   }
 
@@ -2171,18 +2170,17 @@ class nsDisplayItem : public nsDisplayIt
         mAnimatedGeometryRoot(aOther.mAnimatedGeometryRoot),
         mToReferenceFrame(aOther.mToReferenceFrame),
         mBuildingRect(aOther.mBuildingRect),
         mPaintRect(aOther.mPaintRect),
         mForceNotVisible(aOther.mForceNotVisible),
         mDisableSubpixelAA(aOther.mDisableSubpixelAA),
         mReusedItem(false),
         mBackfaceHidden(mFrame->In3DContextAndBackfaceIsHidden()),
-        mPaintRectValid(false),
-        mCanBeReused(true)
+        mPaintRectValid(false)
 #ifdef MOZ_DUMP_PAINTING
         ,
         mPainted(false)
 #endif
   {
     MOZ_COUNT_CTOR(nsDisplayItem);
   }
 
@@ -2841,18 +2839,17 @@ class nsDisplayItem : public nsDisplayIt
   bool HasSameContent(const nsDisplayItem* aOther) const {
     return mFrame->GetContent() == aOther->Frame()->GetContent();
   }
 
   bool IsReused() const { return mReusedItem; }
 
   void SetReused(bool aReused) { mReusedItem = aReused; }
 
-  bool CanBeReused() const { return mCanBeReused; }
-  void SetDontReuse() { mCanBeReused = false; }
+  virtual bool CanBeReused() const { return true; }
 
   virtual nsIFrame* GetDependentFrame() { return nullptr; }
 
   virtual mozilla::Maybe<nsRect> GetClipWithRespectToASR(
       nsDisplayListBuilder* aBuilder, const ActiveScrolledRoot* aASR) const;
 
   void SetDisplayItemData(mozilla::DisplayItemData* aDID,
                           mozilla::layers::LayerManager* aLayerManager) {
@@ -2956,17 +2953,16 @@ class nsDisplayItem : public nsDisplayIt
   OldListIndex mOldListIndex;
   uintptr_t mOldList = 0;
 
   bool mForceNotVisible;
   bool mDisableSubpixelAA;
   bool mReusedItem;
   bool mBackfaceHidden;
   bool mPaintRectValid;
-  bool mCanBeReused;
 #ifdef MOZ_DUMP_PAINTING
   // True if this frame has been painted.
   bool mPainted;
 #endif
 
   struct {
     RefPtr<const DisplayItemClipChain> mClipChain;
     const DisplayItemClip* mClip;
@@ -4054,21 +4050,22 @@ class nsDisplaySolidColorBase : public n
   nscolor mColor;
 };
 
 class nsDisplaySolidColor : public nsDisplaySolidColorBase {
  public:
   nsDisplaySolidColor(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                       const nsRect& aBounds, nscolor aColor,
                       bool aCanBeReused = true)
-      : nsDisplaySolidColorBase(aBuilder, aFrame, aColor), mBounds(aBounds) {
+      : nsDisplaySolidColorBase(aBuilder, aFrame, aColor),
+        mBounds(aBounds),
+        mCanBeReused(aCanBeReused) {
     NS_ASSERTION(NS_GET_A(aColor) > 0,
                  "Don't create invisible nsDisplaySolidColors!");
     MOZ_COUNT_CTOR(nsDisplaySolidColor);
-    mCanBeReused = aCanBeReused;
   }
 
 #ifdef NS_BUILD_REFCNT_LOGGING
   ~nsDisplaySolidColor() override { MOZ_COUNT_DTOR(nsDisplaySolidColor); }
 #endif
 
   NS_DISPLAY_DECL_NAME("SolidColor", TYPE_SOLID_COLOR)
 
@@ -4082,30 +4079,32 @@ class nsDisplaySolidColor : public nsDis
   void Paint(nsDisplayListBuilder* aBuilder, gfxContext* aCtx) override;
   void WriteDebugInfo(std::stringstream& aStream) override;
   bool CreateWebRenderCommands(
       mozilla::wr::DisplayListBuilder& aBuilder,
       mozilla::wr::IpcResourceUpdateQueue& aResources,
       const StackingContextHelper& aSc,
       mozilla::layers::RenderRootStateManager* aManager,
       nsDisplayListBuilder* aDisplayListBuilder) override;
+  bool CanBeReused() const override { return mCanBeReused; }
 
   int32_t ZIndex() const override {
     if (mOverrideZIndex) {
       return mOverrideZIndex.value();
     }
     return nsDisplaySolidColorBase::ZIndex();
   }
 
   void SetOverrideZIndex(int32_t aZIndex) {
     mOverrideZIndex = mozilla::Some(aZIndex);
   }
 
  private:
   nsRect mBounds;
+  bool mCanBeReused;
   mozilla::Maybe<int32_t> mOverrideZIndex;
 };
 
 /**
  * A display item that renders a solid color over a region. This is not
  * exposed through CSS, its only purpose is efficient invalidation of
  * the find bar highlighter dimmer.
  */
deleted file mode 100644
--- a/layout/reftests/display-list/1533317-1-ref.html
+++ /dev/null
@@ -1,15 +0,0 @@
-<html>
-  <head>
-    <style>
-    div {
-      width: 400px;
-      height: 200px;
-      background-color: green;
-    }
-
-    </style>
-  </head>
-  <body>
-      <div></div>
-  </body>
-</html>
deleted file mode 100644
--- a/layout/reftests/display-list/1533317-1.html
+++ /dev/null
@@ -1,32 +0,0 @@
-<html class="reftest-wait">
-  <head>
-    <style>
-    div {
-      width: 200px;
-      height: 200px;
-    }
-    .positioned {
-      position: absolute;
-      top: 0px;
-      left: 200px;
-      background-color: green;
-    }
-    .outer {
-      position: relative;
-    }
-    </style>
-  </head>
-  <body>
-    <div class="outer">
-      <div id="test"></div>
-      <div class="positioned"></div>
-    </div>
-    <script>
-      function doTest() {
-        document.getElementById("test").style.backgroundColor = "green";
-        document.documentElement.removeAttribute("class");
-      }
-      document.addEventListener("MozReftestInvalidate", doTest);
-    </script>
-  </body>
-</html>
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -33,9 +33,8 @@ skip-if(!asyncPan) == 1437374-1.html 143
 == 1451971-1.html 1451971-ref.html
 == 1453541-1.html 1453541-ref.html
 == 1453541-2.html 1453541-ref.html
 == 1452805-1.html 1452805-ref.html
 == 1461231-1.html about:blank
 fuzzy(0-2,0-40000) skip-if(!asyncPan) == 1464288-1.html 1464288-ref.html
 == 1482403-1.html 1482403-1-ref.html
 == 1504233-1.html 1504233-1-ref.html
-== 1533317-1.html 1533317-1-ref.html