Bug 1395719: Reconstruct ancestors asynchronously when lazy frame construction is allowed. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 07 Sep 2017 13:47:30 +0200
changeset 429236 45f8148d33c85fb34b5e4f0dfffaea8dde91f884
parent 429235 08951d83c938c02159e4ab6e5c0a0c1c79ba0087
child 429237 4ae543bcdc2f8c350783318cf28a2dce199e7871
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1395719, 1397091
milestone57.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 1395719: Reconstruct ancestors asynchronously when lazy frame construction is allowed. r=bz The main purpose of this change is avoiding doing frame construction when the style tree is not up-to-date. In the test-case above, for example, a MathML element is changed, and we schedule style invalidation for it, but another MathML element is inserted, and we reconstruct the whole thing, using the out-to-date styles from the MathML element. There are other cases where this is more problematic than "we just happened to use out-of-date styles, and we'll restyle eventually", which is when this reconstruct happens to hit an element which was recently inserted but happened to lazily construct. In that case, we haven't even performed initial styling on the element, so we just panic and crash. After this the only sync frame construction case remaining when the style tree is not setup is CharacterDataChanged, which I'll address in bug 1397091. MozReview-Commit-ID: FSI2Zb34SaZ
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7524,27 +7524,27 @@ nsCSSFrameConstructor::GetRangeInsertion
   }
 
   return insertionPoint;
 }
 
 bool
 nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame,
                                                 nsIContent* aStartChild,
-                                                nsIContent* aEndChild)
+                                                nsIContent* aEndChild,
+                                                InsertionKind aInsertionKind)
 {
   if (aParentFrame->IsFrameSetFrame()) {
     // Check whether we have any kids we care about.
     for (nsIContent* cur = aStartChild;
          cur != aEndChild;
          cur = cur->GetNextSibling()) {
       if (IsSpecialFramesetChild(cur)) {
         // Just reframe the parent, since framesets are weird like that.
-        RecreateFramesForContent(aParentFrame->GetContent(),
-                                 InsertionKind::Sync);
+        RecreateFramesForContent(aParentFrame->GetContent(), aInsertionKind);
         return true;
       }
     }
   }
   return false;
 }
 
 void
@@ -7594,16 +7594,21 @@ nsCSSFrameConstructor::ContentAppended(n
                                        bool aForReconstruction,
                                        TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT(!aProvidedTreeMatchContext ||
              aLazyConstructionAllowed == LazyConstructionAllowed::No);
   MOZ_ASSERT(aLazyConstructionAllowed == LazyConstructionAllowed::No ||
              !RestyleManager()->IsInStyleRefresh());
 
+  const InsertionKind insertionKind =
+    aLazyConstructionAllowed == LazyConstructionAllowed::Yes
+      ? InsertionKind::Async
+      : InsertionKind::Sync;
+
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while creating frames");
 
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentAppended container=%p "
            "first-child=%p lazy=%d\n",
@@ -7685,48 +7690,48 @@ nsCSSFrameConstructor::ContentAppended(n
 
   if (isNewShadowTreeContent) {
     // Recreate frames if content is appended into a ShadowRoot
     // because children of ShadowRoot are rendered in place of children
     // of the host.
     //XXXsmaug This is super unefficient!
     nsIContent* bindingParent = aContainer->GetBindingParent();
     LAYOUT_PHASE_TEMP_EXIT();
-    RecreateFramesForContent(bindingParent, InsertionKind::Sync);
+    RecreateFramesForContent(bindingParent, insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
   InsertionPoint insertion =
     GetRangeInsertionPoint(aContainer, aFirstNewContent, nullptr,
                            aLazyConstructionAllowed, aForReconstruction);
   nsContainerFrame*& parentFrame = insertion.mParentFrame;
   LAYOUT_PHASE_TEMP_REENTER();
   if (!parentFrame) {
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
-  if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr)) {
+  if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr, insertionKind)) {
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
   LAYOUT_PHASE_TEMP_REENTER();
 
   if (parentFrame->IsLeaf()) {
     // Nothing to do here; we shouldn't be constructing kids of leaves
     // Clear lazy bits so we don't try to construct again.
     ClearLazyBits(aFirstNewContent, nullptr);
     return;
   }
 
   if (parentFrame->IsFrameOfType(nsIFrame::eMathML)) {
     LAYOUT_PHASE_TEMP_EXIT();
-    RecreateFramesForContent(parentFrame->GetContent(), InsertionKind::Sync);
+    RecreateFramesForContent(parentFrame->GetContent(), insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   // If the frame we are manipulating is a ib-split frame (that is, one
   // that's been created as a result of a block-in-inline situation) then we
   // need to append to the last ib-split sibling, not to the frame itself.
   bool parentIBSplit = IsFramePartOfIBSplit(parentFrame);
@@ -7821,17 +7826,17 @@ nsCSSFrameConstructor::ContentAppended(n
   nsIFrame* prevSibling = ::FindAppendPrevSibling(parentFrame, parentAfterFrame);
 
   // Perform special check for diddling around with the frames in
   // a ib-split inline frame.
   // If we're appending before :after content, then we're not really
   // appending, so let WipeContainingBlock know that.
   LAYOUT_PHASE_TEMP_EXIT();
   if (WipeContainingBlock(state, containingBlock, parentFrame, items,
-                          true, prevSibling)) {
+                          true, prevSibling, insertionKind)) {
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
   LAYOUT_PHASE_TEMP_REENTER();
 
   // If the parent is a block frame, and we're not in a special case
   // where frames can be moved around, determine if the list is for the
   // start or end of the block.
@@ -8016,16 +8021,21 @@ nsCSSFrameConstructor::ContentRangeInser
              !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while creating frames");
 
   NS_PRECONDITION(aStartChild, "must always pass a child");
 
+  const InsertionKind insertionKind =
+    aLazyConstructionAllowed == LazyConstructionAllowed::Yes
+      ? InsertionKind::Async
+      : InsertionKind::Sync;
+
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentRangeInserted container=%p "
            "start-child=%p end-child=%p lazy=%d\n",
            static_cast<void*>(aContainer),
            static_cast<void*>(aStartChild), static_cast<void*>(aEndChild),
            static_cast<int>(aLazyConstructionAllowed));
     if (gReallyNoisyContentUpdates) {
@@ -8171,17 +8181,17 @@ nsCSSFrameConstructor::ContentRangeInser
 
   if (isNewShadowTreeContent) {
     // Recreate frames if content is inserted into a ShadowRoot
     // because children of ShadowRoot are rendered in place of
     // the children of the host.
     //XXXsmaug This is super unefficient!
     nsIContent* bindingParent = aContainer->GetBindingParent();
     LAYOUT_PHASE_TEMP_EXIT();
-    RecreateFramesForContent(bindingParent, InsertionKind::Sync);
+    RecreateFramesForContent(bindingParent, insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   InsertionPoint insertion;
   if (isSingleInsert) {
     // See if we have an XBL insertion point. If so, then that's our
     // real parent frame; if not, then the frame hasn't been built yet
@@ -8214,17 +8224,17 @@ nsCSSFrameConstructor::ContentRangeInser
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   nsIContent* container = insertion.mParentFrame->GetContent();
 
   LayoutFrameType frameType = insertion.mParentFrame->Type();
   LAYOUT_PHASE_TEMP_EXIT();
-  if (MaybeRecreateForFrameset(insertion.mParentFrame, aStartChild, aEndChild)) {
+  if (MaybeRecreateForFrameset(insertion.mParentFrame, aStartChild, aEndChild, insertionKind)) {
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
   LAYOUT_PHASE_TEMP_REENTER();
 
   // We should only get here with fieldsets when doing a single insert, because
   // fieldsets have multiple insertion points.
   NS_ASSERTION(isSingleInsert || frameType != LayoutFrameType::FieldSet,
@@ -8234,47 +8244,47 @@ nsCSSFrameConstructor::ContentRangeInser
     // Just reframe the parent, since figuring out whether this
     // should be the new legend and then handling it is too complex.
     // We could do a little better here --- check if the fieldset already
     // has a legend which occurs earlier in its child list than this node,
     // and if so, proceed. But we'd have to extend nsFieldSetFrame
     // to locate this legend in the inserted frames and extract it.
     LAYOUT_PHASE_TEMP_EXIT();
     RecreateFramesForContent(insertion.mParentFrame->GetContent(),
-                             InsertionKind::Sync);
+                             insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   // We should only get here with details when doing a single insertion because
   // we treat details frame as if it has multiple insertion points.
   MOZ_ASSERT(isSingleInsert || frameType != LayoutFrameType::Details);
   if (frameType == LayoutFrameType::Details) {
     // When inserting an element into <details>, just reframe the details frame
     // and let it figure out where the element should be laid out. It might seem
     // expensive to recreate the entire details frame, but it's the simplest way
     // to handle the insertion.
     LAYOUT_PHASE_TEMP_EXIT();
     RecreateFramesForContent(insertion.mParentFrame->GetContent(),
-                             InsertionKind::Sync);
+                             insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   // Don't construct kids of leaves
   if (insertion.mParentFrame->IsLeaf()) {
     // Clear lazy bits so we don't try to construct again.
     ClearLazyBits(aStartChild, aEndChild);
     return;
   }
 
   if (insertion.mParentFrame->IsFrameOfType(nsIFrame::eMathML)) {
     LAYOUT_PHASE_TEMP_EXIT();
     RecreateFramesForContent(insertion.mParentFrame->GetContent(),
-                             InsertionKind::Sync);
+                             insertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   Maybe<TreeMatchContext> matchContext;
   if (!aProvidedTreeMatchContext && !aContainer->IsStyledByServo()) {
     matchContext.emplace(mDocument, TreeMatchContext::ForFrameConstruction);
     matchContext->InitAncestors(aContainer ? aContainer->AsElement() : nullptr);
@@ -8412,17 +8422,17 @@ nsCSSFrameConstructor::ContentRangeInser
   }
 
   // Perform special check for diddling around with the frames in
   // a special inline frame.
   // If we're appending before :after content, then we're not really
   // appending, so let WipeContainingBlock know that.
   LAYOUT_PHASE_TEMP_EXIT();
   if (WipeContainingBlock(state, containingBlock, insertion.mParentFrame, items,
-                          isAppend, prevSibling)) {
+                          isAppend, prevSibling, insertionKind)) {
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
   LAYOUT_PHASE_TEMP_REENTER();
 
   // If the container is a table and a caption will be appended, it needs to be
   // put in the table wrapper frame's additional child list.
   // We make no attempt here to set flags to indicate whether the list
@@ -12763,31 +12773,36 @@ IsSafeToAppendToIBSplitInline(nsIFrame* 
 }
 
 bool
 nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState,
                                            nsIFrame* aContainingBlock,
                                            nsIFrame* aFrame,
                                            FrameConstructionItemList& aItems,
                                            bool aIsAppend,
-                                           nsIFrame* aPrevSibling)
-{
+                                           nsIFrame* aPrevSibling,
+                                           InsertionKind aInsertionKind)
+{
+  // FIXME(emilio): Use the argument instead of always reconstruct async
+  // (to be landed in a separate bug, see bug 1395719 comment 17).
+  aInsertionKind = InsertionKind::Async;
+
   if (aItems.IsEmpty()) {
     return false;
   }
 
   // Before we go and append the frames, we must check for several
   // special situations.
 
   // Situation #1 is a XUL frame that contains frames that are required
   // to be wrapped in blocks.
   if (aFrame->IsXULBoxFrame() &&
       !(aFrame->GetStateBits() & NS_STATE_BOX_WRAPS_KIDS_IN_BLOCK) &&
       aItems.AnyItemsNeedBlockParent()) {
-    RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async);
+    RecreateFramesForContent(aFrame->GetContent(), aInsertionKind);
     return true;
   }
 
   nsIFrame* nextSibling = ::GetInsertNextSibling(aFrame, aPrevSibling);
 
   // Situation #2 is a flex or grid container frame into which we're inserting
   // new inline non-replaced children, adjacent to an existing anonymous
   // flex or grid item.
@@ -12796,28 +12811,28 @@ nsCSSFrameConstructor::WipeContainingBlo
       frameType == LayoutFrameType::GridContainer) {
     FCItemIterator iter(aItems);
 
     // Check if we're adding to-be-wrapped content right *after* an existing
     // anonymous flex or grid item (which would need to absorb this content).
     const bool isWebkitBox = IsFlexContainerForLegacyBox(aFrame);
     if (aPrevSibling && IsAnonymousFlexOrGridItem(aPrevSibling) &&
         iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) {
-      RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async);
+      RecreateFramesForContent(aFrame->GetContent(), aInsertionKind);
       return true;
     }
 
     // Check if we're adding to-be-wrapped content right *before* an existing
     // anonymous flex or grid item (which would need to absorb this content).
     if (nextSibling && IsAnonymousFlexOrGridItem(nextSibling)) {
       // Jump to the last entry in the list
       iter.SetToEnd();
       iter.Prev();
       if (iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) {
-        RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async);
+        RecreateFramesForContent(aFrame->GetContent(), aInsertionKind);
         return true;
       }
     }
   }
 
   // Situation #3 is an anonymous flex or grid item that's getting new children
   // who don't want to be wrapped.
   if (IsAnonymousFlexOrGridItem(aFrame)) {
@@ -12832,22 +12847,20 @@ nsCSSFrameConstructor::WipeContainingBlo
     nsFrameConstructorSaveState floatSaveState;
     aState.PushFloatContainingBlock(nullptr, floatSaveState);
 
     FCItemIterator iter(aItems);
     // Skip over things that _do_ need an anonymous flex item, because
     // they're perfectly happy to go here -- they won't cause a reframe.
     nsIFrame* containerFrame = aFrame->GetParent();
     const bool isWebkitBox = IsFlexContainerForLegacyBox(containerFrame);
-    if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState,
-                                                  isWebkitBox)) {
+    if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState, isWebkitBox)) {
       // We hit something that _doesn't_ need an anonymous flex item!
       // Rebuild the flex container to bust it out.
-      RecreateFramesForContent(containerFrame->GetContent(),
-                               InsertionKind::Async);
+      RecreateFramesForContent(containerFrame->GetContent(), aInsertionKind);
       return true;
     }
 
     // If we get here, then everything in |aItems| needs to be wrapped in
     // an anonymous flex or grid item.  That's where it's already going - good!
   }
 
   // Situation #4 is a ruby-related frame that's getting new children.
@@ -12860,17 +12873,17 @@ nsCSSFrameConstructor::WipeContainingBlo
   //    their sibling changes.
   // 2) The first effective child of a ruby frame must always be a ruby
   //    base container. It should be created or destroyed accordingly.
   if (IsRubyPseudo(aFrame) || frameType == LayoutFrameType::Ruby ||
       RubyUtils::IsRubyContainerBox(frameType)) {
     // We want to optimize it better, and avoid reframing as much as
     // possible. But given the cases above, and the fact that a ruby
     // usually won't be very large, it should be fine to reframe it.
-    RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async);
+    RecreateFramesForContent(aFrame->GetContent(), aInsertionKind);
     return true;
   }
 
   // Situation #5 is a case when table pseudo-frames don't work out right
   ParentType parentType = GetParentType(aFrame);
   // If all the kids want a parent of the type that aFrame is, then we're all
   // set to go.  Indeed, there won't be any table pseudo-frames created between
   // aFrame and the kids, so those won't need to be merged with any table
@@ -13026,17 +13039,17 @@ nsCSSFrameConstructor::WipeContainingBlo
     // it might be empty, so recheck that too.
     if (aItems.IsEmpty()) {
       return false;
     }
 
     if (!aItems.AllWantParentType(parentType)) {
       // Reframing aFrame->GetContent() is good enough, since the content of
       // table pseudo-frames is the ancestor content.
-      RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async);
+      RecreateFramesForContent(aFrame->GetContent(), aInsertionKind);
       return true;
     }
   }
 
   // Now we have several cases involving {ib} splits.  Put them all in a
   // do/while with breaks to take us to the "go and reconstruct" code.
   do {
     if (IsInlineFrame(aFrame)) {
@@ -13107,24 +13120,24 @@ nsCSSFrameConstructor::WipeContainingBlo
                  "Must have non-inline, non-ib-split, non-pseudo frame as "
                  "root (or child of root, for a table root)!");
   }
 
   // Tell parent of the containing block to reformulate the
   // entire block. This is painful and definitely not optimal
   // but it will *always* get the right answer.
 
-  nsIContent *blockContent = aContainingBlock->GetContent();
+  nsIContent* blockContent = aContainingBlock->GetContent();
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::WipeContainingBlock: blockContent=%p\n",
            static_cast<void*>(blockContent));
   }
 #endif
-  RecreateFramesForContent(blockContent, InsertionKind::Async);
+  RecreateFramesForContent(blockContent, aInsertionKind);
   return true;
 }
 
 void
 nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame,
                                               InsertionKind aInsertionKind)
 {
 
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -102,16 +102,17 @@ public:
 
   // Create frames for content nodes that are marked as needing frames. This
   // should be called before ProcessPendingRestyles.
   // Note: It's the caller's responsibility to make sure to wrap a
   // CreateNeededFrames call in a view update batch and a script blocker.
   void CreateNeededFrames();
 
 private:
+
   void CreateNeededFrames(nsIContent* aContent,
                           TreeMatchContext& aTreeMatchContext);
 
   enum Operation {
     CONTENTAPPEND,
     CONTENTINSERT
   };
 
@@ -177,17 +178,18 @@ private:
                                         nsIContent* aStartChild,
                                         nsIContent* aEndChild,
                                         LazyConstructionAllowed,
                                         bool aForReconstruction);
 
   // Returns true if parent was recreated due to frameset child, false otherwise.
   bool MaybeRecreateForFrameset(nsIFrame* aParentFrame,
                                 nsIContent* aStartChild,
-                                nsIContent* aEndChild);
+                                nsIContent* aEndChild,
+                                InsertionKind);
 
   /**
    * For each child in the aStartChild/aEndChild range, calls
    * NoteDirtyDescendantsForServo on their flattened tree parents.  This is
    * used when content is inserted into the document and we decide that
    * we can do lazy frame construction.  It handles children being rebound to
    * different insertion points by calling NoteDirtyDescendantsForServo on each
    * child's flattened tree parent.  Only used when we are styled by Servo.
@@ -1934,21 +1936,22 @@ private:
   // aIsAppend cases.  Passing aIsAppend false even when an append is happening
   // is ok in terms of correctness, but can lead to unnecessary reframing.  If
   // aIsAppend is true, then the caller MUST call
   // nsCSSFrameConstructor::AppendFramesToParent (as opposed to
   // nsFrameManager::InsertFrames directly) to add the new frames.
   // @return true if we reconstructed the containing block, false
   // otherwise
   bool WipeContainingBlock(nsFrameConstructorState& aState,
-                             nsIFrame*                aContainingBlock,
-                             nsIFrame*                aFrame,
-                             FrameConstructionItemList& aItems,
-                             bool                     aIsAppend,
-                             nsIFrame*                aPrevSibling);
+                           nsIFrame* aContainingBlock,
+                           nsIFrame* aFrame,
+                           FrameConstructionItemList& aItems,
+                           bool aIsAppend,
+                           nsIFrame* aPrevSibling,
+                           InsertionKind);
 
   void ReframeContainingBlock(nsIFrame* aFrame, InsertionKind aInsertionKind);
 
   //----------------------------------------
 
   // Methods support :first-letter style
 
   nsFirstLetterFrame*