Backed out 2 changesets (bug 1547802, bug 1555819) mochitest and marionette failures CLOSED TREE

Backed out changeset 10ba83441846 (bug 1555819)
Backed out changeset b9af6aea762d (bug 1547802)

layout/generic/nsSubDocumentFrame.cpp

@@ -465,6 +465,30 @@ void nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
     needsOwnLayer = true;
   }
 
+  if (subdocRootFrame && aBuilder->IsRetainingDisplayList()) {
+    // Caret frame changed, rebuild the entire subdoc.
+    // We could just invalidate the old and new frame
+    // areas and save some work here. RetainedDisplayListBuilder
+    // does this, so we could teach it to find and check all
+    // subdocs in advance.
+    if (mPreviousCaret != aBuilder->GetCaretFrame()) {
+      dirty = visible;
+      aBuilder->MarkFrameModifiedDuringBuilding(subdocRootFrame);
+      aBuilder->RebuildAllItemsInCurrentSubtree();
+      // Mark the old caret frame as invalid so that we remove the
+      // old nsDisplayCaret. We don't mark the current frame as invalid
+      // since we want the nsDisplaySubdocument to retain it's place
+      // in the retained display list.
+      if (mPreviousCaret) {
+        aBuilder->MarkFrameModifiedDuringBuilding(mPreviousCaret);
+      }
+      if (aBuilder->GetCaretFrame()) {
+        aBuilder->MarkFrameModifiedDuringBuilding(aBuilder->GetCaretFrame());
+      }
+    }
+    mPreviousCaret = aBuilder->GetCaretFrame();
+  }
+
   nsDisplayList childItems;
 
   {

layout/generic/nsSubDocumentFrame.h

@@ -162,6 +162,7 @@ class nsSubDocumentFrame final : public nsAtomicContainerFrame,
   bool mPostedReflowCallback;
   bool mDidCreateDoc;
   bool mCallingShow;
+  WeakFrame mPreviousCaret;
 };
 
 /**

layout/painting/RetainedDisplayListBuilder.cpp

@@ -114,23 +114,6 @@ static AnimatedGeometryRoot* SelectAGRForFrame(
   return data && data->mModifiedAGR ? data->mModifiedAGR.get() : nullptr;
 }
 
-bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
-  nsIFrame* f = aFrame;
-  while (f) {
-    if (f->IsFrameModified()) {
-      return true;
-    }
-
-    if (aStopAtFrame && f == aStopAtFrame) {
-      break;
-    }
-
-    f = nsLayoutUtils::GetDisplayListParent(f);
-  }
-
-  return false;
-}
-
 // Removes any display items that belonged to a frame that was deleted,
 // and mark frames that belong to a different AGR so that get their
 // items built again.
@@ -139,8 +122,8 @@ bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
 // jump into those immediately rather than walking the entire thing.
 bool RetainedDisplayListBuilder::PreProcessDisplayList(
     RetainedDisplayList* aList, AnimatedGeometryRoot* aAGR,
-    PartialUpdateResult& aUpdated, nsIFrame* aOuterFrame, uint32_t aCallerKey,
-    uint32_t aNestingDepth, bool aKeepLinked) {
+    PartialUpdateResult& aUpdated, uint32_t aCallerKey, uint32_t aNestingDepth,
+    bool aKeepLinked) {
   // The DAG merging algorithm does not have strong mechanisms in place to keep
   // the complexity of the resulting DAG under control. In some cases we can
   // build up edges very quickly. Detect those cases and force a full display
@@ -196,8 +179,7 @@ bool RetainedDisplayListBuilder::PreProcessDisplayList(
       }
     }
 
-    if (!item->CanBeReused() || item->HasDeletedFrame() ||
-        AnyContentAncestorModified(item->FrameForInvalidation(), aOuterFrame)) {
+    if (!item->CanBeReused() || item->HasDeletedFrame()) {
       if (initializeOldItems) {
         aList->mOldItems.AppendElement(OldItemInfo(nullptr));
       } else {
@@ -249,10 +231,9 @@ bool RetainedDisplayListBuilder::PreProcessDisplayList(
         keepLinked = true;
       }
 
-      if (!PreProcessDisplayList(item->GetChildren(),
-                                 SelectAGRForFrame(f, aAGR), aUpdated,
-                                 item->Frame(), item->GetPerFrameKey(),
-                                 aNestingDepth + 1, keepLinked)) {
+      if (!PreProcessDisplayList(
+              item->GetChildren(), SelectAGRForFrame(f, aAGR), aUpdated,
+              item->GetPerFrameKey(), aNestingDepth + 1, keepLinked)) {
         MOZ_RELEASE_ASSERT(
             !aKeepLinked,
             "Can't early return since we need to move the out list back");
@@ -314,6 +295,23 @@ void RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount(
   mBuilder.IncrementPresShellPaintCount(presShell);
 }
 
+bool AnyContentAncestorModified(nsIFrame* aFrame, nsIFrame* aStopAtFrame) {
+  nsIFrame* f = aFrame;
+  while (f) {
+    if (f->IsFrameModified()) {
+      return true;
+    }
+
+    if (aStopAtFrame && f == aStopAtFrame) {
+      break;
+    }
+
+    f = nsLayoutUtils::GetDisplayListParent(f);
+  }
+
+  return false;
+}
+
 static Maybe<const ActiveScrolledRoot*> SelectContainerASR(
     const DisplayItemClipChain* aClipChain, const ActiveScrolledRoot* aItemASR,
     Maybe<const ActiveScrolledRoot*>& aContainerASR) {
@@ -622,12 +620,10 @@ class MergeState {
     return false;
   }
 
-#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   bool HasModifiedFrame(nsDisplayItem* aItem) {
     nsIFrame* stopFrame = mOuterItem ? mOuterItem->Frame() : nullptr;
     return AnyContentAncestorModified(aItem->FrameForInvalidation(), stopFrame);
   }
-#endif
 
   void UpdateContainerASR(nsDisplayItem* aItem) {
     mContainerASR = SelectContainerASR(
@@ -665,7 +661,7 @@ class MergeState {
   void ProcessOldNode(OldListIndex aNode,
                       nsTArray<MergedListIndex>&& aDirectPredecessors) {
     nsDisplayItem* item = mOldItems[aNode.val].mItem;
-    if (mOldItems[aNode.val].IsChanged()) {
+    if (mOldItems[aNode.val].IsChanged() || HasModifiedFrame(item)) {
       if (item && item->IsGlassItem() &&
           item == mBuilder->Builder()->GetGlassDisplayItem()) {
         mBuilder->Builder()->ClearGlassDisplayItem();
@@ -1301,12 +1297,8 @@ bool RetainedDisplayListBuilder::ComputeRebuildRegion(
     }
   }
 
-  // Since we set modified to true on the extraFrames, add them to
-  // aModifiedFrames so that it will get reverted.
-  aModifiedFrames.AppendElements(extraFrames);
-
   for (nsIFrame* f : extraFrames) {
-    f->SetFrameIsModified(true);
+    mBuilder.MarkFrameModifiedDuringBuilding(f);
 
     if (!ProcessFrame(f, &mBuilder, mBuilder.RootReferenceFrame(),
                       aOutFramesWithProps, true, aOutDirty, aOutModifiedAGR)) {
@@ -1362,18 +1354,21 @@ bool RetainedDisplayListBuilder::ShouldBuildPartial(
   return true;
 }
 
-void RetainedDisplayListBuilder::InvalidateCaretFramesIfNeeded() {
+void RetainedDisplayListBuilder::InvalidateCaretFramesIfNeeded(
+    nsTArray<nsIFrame*>& aModifiedFrames) {
   if (mPreviousCaret == mBuilder.GetCaretFrame()) {
     // The current caret frame is the same as the previous one.
     return;
   }
 
-  if (mPreviousCaret) {
-    mPreviousCaret->MarkNeedsDisplayItemRebuild();
+  if (mPreviousCaret &&
+      mBuilder.MarkFrameModifiedDuringBuilding(mPreviousCaret)) {
+    aModifiedFrames.AppendElement(mPreviousCaret);
   }
 
-  if (mBuilder.GetCaretFrame()) {
-    mBuilder.GetCaretFrame()->MarkNeedsDisplayItemRebuild();
+  if (mBuilder.GetCaretFrame() &&
+      mBuilder.MarkFrameModifiedDuringBuilding(mBuilder.GetCaretFrame())) {
+    aModifiedFrames.AppendElement(mBuilder.GetCaretFrame());
   }
 
   mPreviousCaret = mBuilder.GetCaretFrame();
@@ -1421,8 +1416,6 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate(
     MarkFramesWithItemsAndImagesModified(&mList);
   }
 
-  InvalidateCaretFramesIfNeeded();
-
   mBuilder.EnterPresShell(mBuilder.RootReferenceFrame());
 
   // We set the override dirty regions during ComputeRebuildRegion or in
@@ -1436,6 +1429,10 @@ PartialUpdateResult RetainedDisplayListBuilder::AttemptPartialUpdate(
   // Do not allow partial builds if the |ShouldBuildPartial()| heuristic fails.
   bool shouldBuildPartial = ShouldBuildPartial(modifiedFrames.Frames());
 
+  if (shouldBuildPartial) {
+    InvalidateCaretFramesIfNeeded(modifiedFrames.Frames());
+  }
+
   nsRect modifiedDirty;
   AnimatedGeometryRoot* modifiedAGR = nullptr;
   PartialUpdateResult result = PartialUpdateResult::NoChange;

layout/painting/RetainedDisplayListBuilder.h

@@ -194,7 +194,7 @@ struct RetainedDisplayListBuilder {
   /**
    * Invalidates the current and previous caret frame if they have changed.
    */
-  void InvalidateCaretFramesIfNeeded();
+  void InvalidateCaretFramesIfNeeded(nsTArray<nsIFrame*>& aModifiedFrames);
 
   /**
    * A simple early exit heuristic to avoid slow partial display list rebuilds.
@@ -215,7 +215,6 @@ struct RetainedDisplayListBuilder {
   bool PreProcessDisplayList(RetainedDisplayList* aList,
                              AnimatedGeometryRoot* aAGR,
                              PartialUpdateResult& aUpdated,
-                             nsIFrame* aOuterFrame = nullptr,
                              uint32_t aCallerKey = 0,
                              uint32_t aNestingDepth = 0,
                              bool aKeepLinked = false);

layout/painting/crashtests/1555819-1.html

@@ -1,12 +0,0 @@
-<style>
-body {
-  width: 1px;
-  height: 5vmax;
-  -webkit-filter: brightness(1);
-}
-* {
-  grid-template-areas: '';
-  columns: 1px;
-}
-</style>
-<keygen autofocus="">aaaa

layout/painting/crashtests/crashtests.list

@@ -20,5 +20,4 @@ load 1514544-1.html
 load 1547420-1.html
 load 1549909.html
 asserts(6) load 1551389-1.html # bug 847368
-asserts(0-2) load 1555819-1.html
 

layout/painting/nsDisplayList.cpp

@@ -96,7 +96,6 @@
 #include "nsTableColFrame.h"
 #include "nsTextFrame.h"
 #include "nsSliderFrame.h"
-#include "nsFocusManager.h"
 #include "ClientLayerManager.h"
 #include "mozilla/layers/RenderRootStateManager.h"
 #include "mozilla/layers/StackingContextHelper.h"
@@ -1234,7 +1233,6 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame,
       mUsedAGRBudget(0),
       mDirtyRect(-1, -1, -1, -1),
       mGlassDisplayItem(nullptr),
-      mCaretFrame(nullptr),
       mScrollInfoItemsForHoisting(nullptr),
       mFirstClipChainToDestroy(nullptr),
       mActiveScrolledRootForRootScrollframe(nullptr),
@@ -1293,21 +1291,6 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame,
       "Check TYPE_MAX should not overflow");
 }
 
-static PresShell* GetFocusedPresShell() {
-  nsPIDOMWindowOuter* focusedWnd =
-      nsFocusManager::GetFocusManager()->GetFocusedWindow();
-  if (!focusedWnd) {
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIDocShell> focusedDocShell = focusedWnd->GetDocShell();
-  if (!focusedDocShell) {
-    return nullptr;
-  }
-
-  return focusedDocShell->GetPresShell();
-}
-
 void nsDisplayListBuilder::BeginFrame() {
   nsCSSRendering::BeginFrameTreesLocked();
   mCurrentAGR = mRootAGR;
@@ -1325,21 +1308,6 @@ void nsDisplayListBuilder::BeginFrame() {
   for (auto& renderRootRect : mRenderRootRects) {
     renderRootRect = LayoutDeviceRect();
   }
-
-  RefPtr<PresShell> presShell = GetFocusedPresShell();
-  if (presShell) {
-    RefPtr<nsCaret> caret = presShell->GetCaret();
-    mCaretFrame = caret->GetPaintGeometry(&mCaretRect);
-
-    // The focused pres shell may not be in the document that we're
-    // painting, or be in a popup. Check if the display root for
-    // the caret matches the display root that we're painting, and
-    // only use it if it matches.
-    if (mCaretFrame &&
-        nsLayoutUtils::GetDisplayRootFrame(mCaretFrame) != mReferenceFrame) {
-      mCaretFrame = nullptr;
-    }
-  }
 }
 
 void nsDisplayListBuilder::EndFrame() {
@@ -1352,7 +1320,6 @@ void nsDisplayListBuilder::EndFrame() {
   FreeClipChains();
   FreeTemporaryItems();
   nsCSSRendering::EndFrameTreesLocked();
-  mCaretFrame = nullptr;
 }
 
 void nsDisplayListBuilder::MarkFrameForDisplay(nsIFrame* aFrame,
@@ -1630,6 +1597,7 @@ void nsDisplayListBuilder::EnterPresShell(nsIFrame* aReferenceFrame,
                                           bool aPointerEventsNoneDoc) {
   PresShellState* state = mPresShellStates.AppendElement();
   state->mPresShell = aReferenceFrame->PresShell();
+  state->mCaretFrame = nullptr;
   state->mFirstFrameMarkedForDisplay = mFramesMarkedForDisplay.Length();
   state->mFirstFrameWithOOFData = mFramesWithOOFData.Length();
 
@@ -1691,11 +1659,10 @@ void nsDisplayListBuilder::EnterPresShell(nsIFrame* aReferenceFrame,
     return;
   }
 
-  // Caret frames add visual area to their frame, but we don't update the
-  // overflow area. Use flags to make sure we build display items for that frame
-  // instead.
-  if (mCaretFrame && mCaretFrame->PresShell() == state->mPresShell) {
-    MarkFrameForDisplay(mCaretFrame, aReferenceFrame);
+  RefPtr<nsCaret> caret = state->mPresShell->GetCaret();
+  state->mCaretFrame = caret->GetPaintGeometry(&state->mCaretRect);
+  if (state->mCaretFrame) {
+    MarkFrameForDisplay(state->mCaretFrame, aReferenceFrame);
   }
 }
 

layout/painting/nsDisplayList.h

@@ -628,6 +628,10 @@ class nsDisplayListBuilder {
   bool IsBuilding() const { return mIsBuilding; }
   void SetIsBuilding(bool aIsBuilding) {
     mIsBuilding = aIsBuilding;
+    for (nsIFrame* f : mModifiedFramesDuringBuilding) {
+      f->SetFrameIsModified(false);
+    }
+    mModifiedFramesDuringBuilding.Clear();
   }
 
   bool InInvalidSubtree() const { return mInInvalidSubtree; }
@@ -773,11 +777,11 @@ class nsDisplayListBuilder {
    * Get the frame that the caret is supposed to draw in.
    * If the caret is currently invisible, this will be null.
    */
-  nsIFrame* GetCaretFrame() { return mCaretFrame; }
+  nsIFrame* GetCaretFrame() { return CurrentPresShellState()->mCaretFrame; }
   /**
    * Get the rectangle we're supposed to draw the caret into.
    */
-  const nsRect& GetCaretRect() { return mCaretRect; }
+  const nsRect& GetCaretRect() { return CurrentPresShellState()->mCaretRect; }
   /**
    * Get the caret associated with the current presshell.
    */
@@ -1699,6 +1703,17 @@ class nsDisplayListBuilder {
     mBuildingInvisibleItems = aBuildingInvisibleItems;
   }
 
+  bool MarkFrameModifiedDuringBuilding(nsIFrame* aFrame) {
+    if (!aFrame->IsFrameModified()) {
+      mModifiedFramesDuringBuilding.AppendElement(aFrame);
+      aFrame->SetFrameIsModified(true);
+      return true;
+    }
+    return false;
+  }
+
+  void RebuildAllItemsInCurrentSubtree() { mDirtyRect = mVisibleRect; }
+
   /**
    * This is a convenience function to ease the transition until AGRs and ASRs
    * are unified.
@@ -1815,6 +1830,8 @@ class nsDisplayListBuilder {
 #ifdef DEBUG
     mozilla::Maybe<nsAutoLayoutPhase> mAutoLayoutPhase;
 #endif
+    nsIFrame* mCaretFrame;
+    nsRect mCaretRect;
     mozilla::Maybe<OutOfFlowDisplayData> mFixedBackgroundDisplayData;
     uint32_t mFirstFrameMarkedForDisplay;
     uint32_t mFirstFrameWithOOFData;
@@ -1890,6 +1907,8 @@ class nsDisplayListBuilder {
   // Set of frames already counted in budget
   nsTHashtable<nsPtrHashKey<nsIFrame>> mAGRBudgetSet;
 
+  nsTArray<nsIFrame*> mModifiedFramesDuringBuilding;
+
   nsDataHashtable<nsPtrHashKey<RemoteBrowser>, EffectsInfo> mEffectsUpdates;
 
   // Relative to mCurrentFrame.
@@ -1915,10 +1934,6 @@ class nsDisplayListBuilder {
   // If we've encountered a glass item yet, only used during partial display
   // list builds.
   bool mHasGlassItemDuringPartial;
-
-  nsIFrame* mCaretFrame;
-  nsRect mCaretRect;
-
   // A temporary list that we append scroll info items to while building
   // display items for the contents of frames with SVG effects.
   // Only non-null when ShouldBuildScrollInfoItemsForHoisting() is true.