Bug 1351535 - Part 2: Explicitly indicate when a ContentRangeInserted call is for frame reconstruction. r=bholley draft
authorCameron McCormack <cam@mcc.id.au>
Tue, 04 Apr 2017 19:17:35 +0800
changeset 558861 417057c1fde32fec6e13a8b66a68fbd24cb7803c
parent 558860 e9c76a85b60cfe1c3eac4bd385a5933837f02d85
child 558862 30d4c9f84cfdd9c3ae0b8c8d5eff90c573d4f27c
push id52975
push userbmo:cam@mcc.id.au
push dateSat, 08 Apr 2017 09:24:07 +0000
reviewersbholley
bugs1351535
milestone55.0a1
Bug 1351535 - Part 2: Explicitly indicate when a ContentRangeInserted call is for frame reconstruction. r=bholley It wasn't clear to me whether the existing check for whether we are under a restyle was sufficient to cover all cases of reconstructing frames, so this patch makes it more explicit by passing that state into ContentAppended and ContentRangeInserted. MozReview-Commit-ID: HjlDCzJv97n
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7230,23 +7230,28 @@ nsCSSFrameConstructor::CreateNeededFrame
         inRun = true;
         firstChildInRun = child;
       }
     } else {
       if (inRun) {
         inRun = false;
         // generate a ContentRangeInserted for [startOfRun,i)
         ContentRangeInserted(aContent, firstChildInRun, child, nullptr,
-                             false, &aTreeMatchContext);
+                             false, // aAllowLazyConstruction
+                             true,  // aForReconstruction
+                             &aTreeMatchContext);
       }
     }
   }
 
   if (inRun) {
-    ContentAppended(aContent, firstChildInRun, false, &aTreeMatchContext);
+    ContentAppended(aContent, firstChildInRun,
+                    false, // aAllowLazyConstruction
+                    true,  // aForReconstruction
+                    &aTreeMatchContext);
   }
 
   // Now descend.
   FlattenedChildIterator iter(aContent);
   for (nsIContent* child = iter.GetNextChild(); child; child = iter.GetNextChild()) {
     if (child->HasFlag(NODE_DESCENDANTS_NEED_FRAMES)) {
       TreeMatchContext::AutoAncestorPusher insertionPointPusher(
           aTreeMatchContext);
@@ -7281,45 +7286,48 @@ void nsCSSFrameConstructor::CreateNeeded
     EndUpdate();
   }
 }
 
 void
 nsCSSFrameConstructor::IssueSingleInsertNofications(nsIContent* aContainer,
                                                     nsIContent* aStartChild,
                                                     nsIContent* aEndChild,
-                                                    bool aAllowLazyConstruction)
+                                                    bool aAllowLazyConstruction,
+                                                    bool aForReconstruction)
 {
   for (nsIContent* child = aStartChild;
        child != aEndChild;
        child = child->GetNextSibling()) {
     if ((child->GetPrimaryFrame() || GetUndisplayedContent(child) ||
          GetDisplayContentsStyleFor(child))
 #ifdef MOZ_XUL
         //  Except listboxes suck, so do NOT skip anything here if
         //  we plan to notify a listbox.
         && !MaybeGetListBoxBodyFrame(aContainer, child)
 #endif
         ) {
       // Already have a frame or undisplayed entry for this content; a
-      // previous ContentInserted in this loop must have reconstructed
+      // previous ContentRangeInserted in this loop must have reconstructed
       // its insertion parent.  Skip it.
       continue;
     }
-    // Call ContentInserted with this node.
-    ContentInserted(aContainer, child, mTempFrameTreeState,
-                    aAllowLazyConstruction);
+    // Call ContentRangeInserted with this node.
+    ContentRangeInserted(aContainer, child, child->GetNextSibling(),
+                         mTempFrameTreeState, aAllowLazyConstruction,
+                         aForReconstruction, nullptr);
   }
 }
 
 nsCSSFrameConstructor::InsertionPoint
 nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aContainer,
                                               nsIContent* aStartChild,
                                               nsIContent* aEndChild,
-                                              bool aAllowLazyConstruction)
+                                              bool aAllowLazyConstruction,
+                                              bool aForReconstruction)
 {
   // 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
   // and we just bail.
   InsertionPoint insertionPoint = GetInsertionPoint(aContainer, nullptr);
   if (!insertionPoint.mParentFrame && !insertionPoint.mMultiple) {
     return insertionPoint; // Don't build the frames.
   }
@@ -7358,17 +7366,17 @@ nsCSSFrameConstructor::GetRangeInsertion
     // If we have multiple insertion points or if we have an insertion point
     // and the operation is not a true append or if the insertion point already
     // has explicit children, then we must fall back.
     if (insertionPoint.mMultiple || aEndChild != nullptr || childCount > 0) {
       // Now comes the fun part.  For each inserted child, make a
       // ContentInserted call as if it had just gotten inserted and
       // let ContentInserted handle the mess.
       IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
-                                   aAllowLazyConstruction);
+                                   aAllowLazyConstruction, aForReconstruction);
       insertionPoint.mParentFrame = nullptr;
     }
   }
 
   return insertionPoint;
 }
 
 bool
@@ -7432,16 +7440,17 @@ nsCSSFrameConstructor::StyleNewChildRang
     }
   }
 }
 
 void
 nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer,
                                        nsIContent* aFirstNewContent,
                                        bool aAllowLazyConstruction,
+                                       bool aForReconstruction,
                                        TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT_IF(aProvidedTreeMatchContext, !aAllowLazyConstruction);
   MOZ_ASSERT_IF(aAllowLazyConstruction, !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while creating frames");
@@ -7485,17 +7494,17 @@ nsCSSFrameConstructor::ContentAppended(n
 #endif // MOZ_XUL
 
   // The frame constructor uses this codepath both for bonafide newly-added
   // content and for RestyleManager-driven frame construction (RECONSTRUCT_FRAME
   // and lazy frame construction). If we're using the Servo style system, we
   // want to ensure that styles get resolved in the first case, whereas for the
   // second case they should have already been resolved if needed.
   bool isNewlyAddedContentForServo = aContainer->IsStyledByServo() &&
-                                     !RestyleManager()->IsInStyleRefresh();
+                                     !aForReconstruction;
 
   bool isNewShadowTreeContent =
     aContainer && aContainer->HasFlag(NODE_IS_IN_SHADOW_TREE) &&
     !aContainer->IsInNativeAnonymousSubtree() &&
     !aFirstNewContent->IsInNativeAnonymousSubtree();
 
   if (!isNewShadowTreeContent) {
     // See comment in ContentRangeInserted for why this is necessary.
@@ -7535,17 +7544,17 @@ nsCSSFrameConstructor::ContentAppended(n
                              REMOVE_FOR_RECONSTRUCTION, nullptr);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
   InsertionPoint insertion =
     GetRangeInsertionPoint(aContainer, aFirstNewContent, nullptr,
-                           aAllowLazyConstruction);
+                           aAllowLazyConstruction, aForReconstruction);
   nsContainerFrame*& parentFrame = insertion.mParentFrame;
   LAYOUT_PHASE_TEMP_REENTER();
   if (!parentFrame) {
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
   if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr)) {
@@ -7833,16 +7842,17 @@ nsCSSFrameConstructor::ContentInserted(n
 // (because when we treat the caption frames the other nodes have had their
 // frames constructed but not yet inserted into the frame tree).
 void
 nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer,
                                             nsIContent* aStartChild,
                                             nsIContent* aEndChild,
                                             nsILayoutHistoryState* aFrameState,
                                             bool aAllowLazyConstruction,
+                                            bool aForReconstruction,
                                             TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT_IF(aProvidedTreeMatchContext, !aAllowLazyConstruction);
   MOZ_ASSERT_IF(aAllowLazyConstruction, !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while creating frames");
@@ -7895,17 +7905,17 @@ nsCSSFrameConstructor::ContentRangeInser
                             aStartChild, nullptr, nullptr, CONTENT_INSERTED)) {
         return;
       }
     } else {
       // We don't handle a range insert to a listbox parent, issue single
       // ContertInserted calls for each node inserted.
       LAYOUT_PHASE_TEMP_EXIT();
       IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
-                                   aAllowLazyConstruction);
+                                   aAllowLazyConstruction, aForReconstruction);
       LAYOUT_PHASE_TEMP_REENTER();
       return;
     }
   }
 #endif // MOZ_XUL
 
   // If we have a null parent, then this must be the document element being
   // inserted, or some other child of the document in the DOM (might be a PI,
@@ -7958,17 +7968,17 @@ nsCSSFrameConstructor::ContentRangeInser
   }
 
   // The frame constructor uses this codepath both for bonafide newly-added
   // content and for RestyleManager-driven frame construction (RECONSTRUCT_FRAME
   // and lazy frame construction). If we're using the Servo style system, we
   // want to ensure that styles get resolved in the first case, whereas for the
   // second case they should have already been resolved if needed.
   bool isNewlyAddedContentForServo = aContainer->IsStyledByServo() &&
-                                     !RestyleManager()->IsInStyleRefresh();
+                                     !aForReconstruction;
 
   bool isNewShadowTreeContent =
     aContainer->HasFlag(NODE_IS_IN_SHADOW_TREE) &&
     !aContainer->IsInNativeAnonymousSubtree() &&
     (!aStartChild || !aStartChild->IsInNativeAnonymousSubtree()) &&
     (!aEndChild || !aEndChild->IsInNativeAnonymousSubtree());
 
   if (!isNewShadowTreeContent) {
@@ -8023,34 +8033,35 @@ nsCSSFrameConstructor::ContentRangeInser
     // real parent frame; if not, then the frame hasn't been built yet
     // and we just bail.
     insertion = GetInsertionPoint(aContainer, aStartChild);
   } else {
     // Get our insertion point. If we need to issue single ContentInserted's
     // GetRangeInsertionPoint will take care of that for us.
     LAYOUT_PHASE_TEMP_EXIT();
     insertion = GetRangeInsertionPoint(aContainer, aStartChild, aEndChild,
-                                       aAllowLazyConstruction);
+                                       aAllowLazyConstruction,
+                                       aForReconstruction);
     LAYOUT_PHASE_TEMP_REENTER();
   }
 
   if (!insertion.mParentFrame) {
     return;
   }
 
   bool isAppend, isRangeInsertSafe;
   nsIFrame* prevSibling = GetInsertionPrevSibling(&insertion, aStartChild,
                                                   &isAppend, &isRangeInsertSafe);
 
   // check if range insert is safe
   if (!isSingleInsert && !isRangeInsertSafe) {
     // must fall back to a single ContertInserted for each child in the range
     LAYOUT_PHASE_TEMP_EXIT();
     IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
-                                 aAllowLazyConstruction);
+                                 aAllowLazyConstruction, aForReconstruction);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   nsIContent* container = insertion.mParentFrame->GetContent();
 
   nsIAtom* frameType = insertion.mParentFrame->GetType();
   LAYOUT_PHASE_TEMP_EXIT();
@@ -8180,17 +8191,17 @@ nsCSSFrameConstructor::ContentRangeInser
       // Need check whether a range insert is still safe.
       if (!isSingleInsert && !isRangeInsertSafe) {
         // Need to recover the letter frames first.
         RecoverLetterFrames(state.mFloatedItems.containingBlock);
 
         // must fall back to a single ContertInserted for each child in the range
         LAYOUT_PHASE_TEMP_EXIT();
         IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
-                                     aAllowLazyConstruction);
+                                     aAllowLazyConstruction, aForReconstruction);
         LAYOUT_PHASE_TEMP_REENTER();
         return;
       }
 
       container = insertion.mParentFrame->GetContent();
       frameType = insertion.mParentFrame->GetType();
     }
   }
@@ -9918,17 +9929,21 @@ nsCSSFrameConstructor::RecreateFramesFor
       // Now, recreate the frames associated with this content object. If
       // ContentRemoved triggered reconstruction, then we don't need to do this
       // because the frames will already have been built.
       if (aAsyncInsert) {
         // XXXmats doesn't frame state need to be restored in this case too?
         RestyleManager()->PostRestyleEvent(
           aContent->AsElement(), nsRestyleHint(0), nsChangeHint_ReconstructFrame);
       } else {
-        ContentInserted(container, aContent, mTempFrameTreeState, false);
+        ContentRangeInserted(container, aContent, aContent->GetNextSibling(),
+                             mTempFrameTreeState,
+                             false, // aAllowLazyConstruction
+                             true,  // aForReconstruction
+                             nullptr);
       }
     }
   }
 }
 
 void
 nsCSSFrameConstructor::DestroyFramesFor(nsIContent*  aContent,
                                         nsIContent** aDestroyedFramesFor)
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -101,17 +101,18 @@ private:
                               nsIContent* aContainer,
                               nsIContent* aChild);
 
   // Issues a single ContentInserted for each child of aContainer in the range
   // [aStartChild, aEndChild).
   void IssueSingleInsertNofications(nsIContent* aContainer,
                                     nsIContent* aStartChild,
                                     nsIContent* aEndChild,
-                                    bool aAllowLazyConstruction);
+                                    bool aAllowLazyConstruction,
+                                    bool aForReconstruction);
 
   /**
    * Data that represents an insertion point for some child content.
    */
   struct InsertionPoint
   {
     InsertionPoint()
       : mParentFrame(nullptr), mContainer(nullptr), mMultiple(false) {}
@@ -144,17 +145,18 @@ private:
    * can be inserted/appended to one insertion point together. If so, returns
    * that insertion point. If not, returns with InsertionPoint.mFrame == nullptr
    * and issues single ContentInserted calls for each child.
    * aEndChild = nullptr indicates that we are dealing with an append.
    */
   InsertionPoint GetRangeInsertionPoint(nsIContent* aContainer,
                                         nsIContent* aStartChild,
                                         nsIContent* aEndChild,
-                                        bool aAllowLazyConstruction);
+                                        bool aAllowLazyConstruction,
+                                        bool aForReconstruction);
 
   // Returns true if parent was recreated due to frameset child, false otherwise.
   bool MaybeRecreateForFrameset(nsIFrame* aParentFrame,
                                   nsIContent* aStartChild,
                                   nsIContent* aEndChild);
 
   /**
    * For each child in the aStartChild/aEndChild range, calls
@@ -226,17 +228,21 @@ public:
   //
   // When constructing frames lazily, we can keep the tree match context in a
   // much easier way than nsFrameConstructorState, and thus, we're allowed to
   // provide a TreeMatchContext to avoid calling InitAncestors repeatedly deep
   // in the DOM.
   void ContentAppended(nsIContent* aContainer,
                        nsIContent* aFirstNewContent,
                        bool aAllowLazyConstruction,
-                       TreeMatchContext* aProvidedTreeMatchContext = nullptr);
+                       TreeMatchContext* aProvidedTreeMatchContext = nullptr)
+  {
+    ContentAppended(aContainer, aFirstNewContent, aAllowLazyConstruction, false,
+                    aProvidedTreeMatchContext);
+  }
 
   // If aAllowLazyConstruction is true then frame construction of the new child
   // can be done lazily.
   void ContentInserted(nsIContent* aContainer,
                        nsIContent* aChild,
                        nsILayoutHistoryState* aFrameState,
                        bool aAllowLazyConstruction);
 
@@ -251,18 +257,46 @@ public:
   //
   // See ContentAppended to see why we allow passing an already initialized
   // TreeMatchContext.
   void ContentRangeInserted(nsIContent* aContainer,
                             nsIContent* aStartChild,
                             nsIContent* aEndChild,
                             nsILayoutHistoryState* aFrameState,
                             bool aAllowLazyConstruction,
-                            TreeMatchContext* aProvidedTreeMatchContext = nullptr);
+                            TreeMatchContext* aProvidedTreeMatchContext = nullptr)
+  {
+    ContentRangeInserted(aContainer, aStartChild, aEndChild, aFrameState,
+                         aAllowLazyConstruction, false,
+                         aProvidedTreeMatchContext);
+  }
 
+private:
+  // Helpers for the public ContentAppended, ContentInserted and
+  // ContentRangeInserted functions above.
+  //
+  // aForReconstruction indicates whether this call is for frame reconstruction
+  // via RecreateFramesFor or lazy frame construction via CreateNeededFrames.
+  // (This latter case admittedly isn't always for "reconstruction" per se, but
+  // the important thing is that aForReconstruction is false for real content
+  // insertions, and true for other cases.)
+  void ContentAppended(nsIContent* aContainer,
+                       nsIContent* aFirstNewContent,
+                       bool aAllowLazyConstruction,
+                       bool aForReconstruction,
+                       TreeMatchContext* aProvidedTreeMatchContext);
+  void ContentRangeInserted(nsIContent* aContainer,
+                            nsIContent* aStartChild,
+                            nsIContent* aEndChild,
+                            nsILayoutHistoryState* aFrameState,
+                            bool aAllowLazyConstruction,
+                            bool aForReconstruction,
+                            TreeMatchContext* aProvidedTreeMatchContext);
+
+public:
   enum RemoveFlags {
     REMOVE_CONTENT, REMOVE_FOR_RECONSTRUCTION, REMOVE_DESTROY_FRAMES };
   /**
    * Recreate or destroy frames for aChild in aContainer.
    * aFlags == REMOVE_CONTENT means aChild has been removed from the document.
    * aFlags == REMOVE_FOR_RECONSTRUCTION means the caller will reconstruct the
    *   frames later.
    * In both the above cases, this method will in some cases try to reconstruct