Bug 1397091: Remove aForReconstruct. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 07 Sep 2017 18:38:27 +0200
changeset 429241 1e3f27bf4b47013a2a94efa3dfc9c1ba128458f4
parent 429240 cb5a5cf16f1bd33cb02e238f80f6fe48d6945c53
child 429242 3c698f2b2eed6e29d6346c798e6234bd5b2696c4
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
bugs1397091
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 1397091: Remove aForReconstruct. r=bz It's only used to know whether we may potentially need to sync-style the subtree. Given with these patches we guarantee that when inserting sync, we have the style tree up-to-date, we can just check aInsertionKind for the same effect. MozReview-Commit-ID: ADvcjkGq5hi
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7376,26 +7376,24 @@ nsCSSFrameConstructor::CreateNeededFrame
         firstChildInRun = child;
       }
     } else {
       if (inRun) {
         inRun = false;
         // generate a ContentRangeInserted for [startOfRun,i)
         ContentRangeInserted(aContent, firstChildInRun, child, nullptr,
                              InsertionKind::Sync,
-                             true,  // aForReconstruction
                              &aTreeMatchContext);
       }
     }
   }
 
   if (inRun) {
     ContentAppended(aContent, firstChildInRun,
                     InsertionKind::Sync,
-                    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(
@@ -7433,18 +7431,17 @@ nsCSSFrameConstructor::CreateNeededFrame
     EndUpdate();
   }
 }
 
 void
 nsCSSFrameConstructor::IssueSingleInsertNofications(nsIContent* aContainer,
                                                     nsIContent* aStartChild,
                                                     nsIContent* aEndChild,
-                                                    InsertionKind aInsertionKind,
-                                                    bool aForReconstruction)
+                                                    InsertionKind aInsertionKind)
 {
   for (nsIContent* child = aStartChild;
        child != aEndChild;
        child = child->GetNextSibling()) {
     if ((child->GetPrimaryFrame() || GetDisplayNoneStyleFor(child) ||
          GetDisplayContentsStyleFor(child))
 #ifdef MOZ_XUL
         //  Except listboxes suck, so do NOT skip anything here if
@@ -7454,27 +7451,25 @@ nsCSSFrameConstructor::IssueSingleInsert
         ) {
       // Already have a frame or undisplayed entry for this content; a
       // previous ContentRangeInserted in this loop must have reconstructed
       // its insertion parent.  Skip it.
       continue;
     }
     // Call ContentRangeInserted with this node.
     ContentRangeInserted(aContainer, child, child->GetNextSibling(),
-                         mTempFrameTreeState, aInsertionKind,
-                         aForReconstruction, nullptr);
+                         mTempFrameTreeState, aInsertionKind, nullptr);
   }
 }
 
 nsCSSFrameConstructor::InsertionPoint
 nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aContainer,
                                               nsIContent* aStartChild,
                                               nsIContent* aEndChild,
-                                              InsertionKind aInsertionKind,
-                                              bool aForReconstruction)
+                                              InsertionKind aInsertionKind)
 {
   // 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.
   }
@@ -7513,17 +7508,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,
-                                   aInsertionKind, aForReconstruction);
+                                   aInsertionKind);
       insertionPoint.mParentFrame = nullptr;
     }
   }
 
   return insertionPoint;
 }
 
 bool
@@ -7586,17 +7581,16 @@ nsCSSFrameConstructor::StyleNewChildRang
     }
   }
 }
 
 void
 nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer,
                                        nsIContent* aFirstNewContent,
                                        InsertionKind aInsertionKind,
-                                       bool aForReconstruction,
                                        TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT(!aProvidedTreeMatchContext ||
              aInsertionKind == InsertionKind::Sync);
   MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
@@ -7635,55 +7629,49 @@ nsCSSFrameConstructor::ContentAppended(n
     // Just ignore tree tags, anyway we don't create any frames for them.
     if (tag == nsGkAtoms::treechildren ||
         tag == nsGkAtoms::treeitem ||
         tag == nsGkAtoms::treerow)
       return;
   }
 #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() &&
-                                     !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.
     if (!GetContentInsertionFrameFor(aContainer) &&
         !aContainer->IsActiveChildrenElement()) {
       // We're punting on frame construction because there's no container frame.
       // The Servo-backed style system handles this case like the lazy frame
       // construction case, except when we're already constructing frames, in
       // which case we shouldn't need to do anything else.
-      if (isNewlyAddedContentForServo &&
+      if (aContainer->IsStyledByServo() &&
           aInsertionKind == InsertionKind::Async) {
         LazilyStyleNewChildRange(aFirstNewContent, nullptr);
       }
       return;
     }
 
     if (aInsertionKind == InsertionKind::Async &&
         MaybeConstructLazily(CONTENTAPPEND, aContainer, aFirstNewContent)) {
-      if (isNewlyAddedContentForServo) {
+      if (aContainer->IsStyledByServo()) {
         LazilyStyleNewChildRange(aFirstNewContent, nullptr);
       }
       return;
     }
   }
 
-  // We couldn't construct lazily. Make Servo eagerly traverse the new content.
-  if (isNewlyAddedContentForServo) {
+  // We couldn't construct lazily. Make Servo eagerly traverse the new content
+  // if needed (when aInsertionKind == InsertionKind::Sync, we know that the
+  // styles are up-to-date already).
+  if (aInsertionKind == InsertionKind::Async && aContainer->IsStyledByServo()) {
     StyleNewChildRange(aFirstNewContent, nullptr);
   }
 
   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!
@@ -7692,17 +7680,17 @@ nsCSSFrameConstructor::ContentAppended(n
     RecreateFramesForContent(bindingParent, aInsertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
   InsertionPoint insertion =
     GetRangeInsertionPoint(aContainer, aFirstNewContent, nullptr,
-                           aInsertionKind, aForReconstruction);
+                           aInsertionKind);
   nsContainerFrame*& parentFrame = insertion.mParentFrame;
   LAYOUT_PHASE_TEMP_REENTER();
   if (!parentFrame) {
     return;
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
   if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr, aInsertionKind)) {
@@ -8000,17 +7988,16 @@ 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,
                                             InsertionKind aInsertionKind,
-                                            bool aForReconstruction,
                                             TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT(!aProvidedTreeMatchContext ||
              aInsertionKind == InsertionKind::Sync);
   MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
   NS_PRECONDITION(mUpdateCount != 0,
@@ -8061,17 +8048,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,
-                                   aInsertionKind, aForReconstruction);
+                                   aInsertionKind);
       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,
@@ -8112,62 +8099,56 @@ nsCSSFrameConstructor::ContentRangeInser
       accService->ContentRangeInserted(mPresShell, aContainer,
                                        aStartChild, aEndChild);
     }
 #endif
 
     return;
   }
 
-  // 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() &&
-                                     !aForReconstruction;
-
   bool isNewShadowTreeContent =
     aContainer->HasFlag(NODE_IS_IN_SHADOW_TREE) &&
     !aContainer->IsInNativeAnonymousSubtree() &&
     (!aStartChild || !aStartChild->IsInNativeAnonymousSubtree()) &&
     (!aEndChild || !aEndChild->IsInNativeAnonymousSubtree());
 
   if (!isNewShadowTreeContent) {
     nsContainerFrame* parentFrame = GetContentInsertionFrameFor(aContainer);
     // The xbl:children element won't have a frame, but default content can have the children as
     // a parent. While its uncommon to change the structure of the default content itself, a label,
     // for example, can be reframed by having its value attribute set or removed.
     if (!parentFrame && !aContainer->IsActiveChildrenElement()) {
       // We're punting on frame construction because there's no container frame.
       // The Servo-backed style system handles this case like the lazy frame
       // construction case, except when we're already constructing frames, in
       // which case we shouldn't need to do anything else.
-      if (isNewlyAddedContentForServo &&
+      if (aContainer->IsStyledByServo() &&
           aInsertionKind == InsertionKind::Async) {
         LazilyStyleNewChildRange(aStartChild, aEndChild);
       }
       return;
     }
 
     // Otherwise, we've got parent content. Find its frame.
     NS_ASSERTION(!parentFrame || parentFrame->GetContent() == aContainer ||
                  GetDisplayContentsStyleFor(aContainer), "New XBL code is possibly wrong!");
 
     if (aInsertionKind == InsertionKind::Async &&
         MaybeConstructLazily(CONTENTINSERT, aContainer, aStartChild)) {
-      if (isNewlyAddedContentForServo) {
+      if (aContainer->IsStyledByServo()) {
         LazilyStyleNewChildRange(aStartChild, aEndChild);
       }
       return;
     }
   }
 
-  // We couldn't construct lazily. Make Servo eagerly traverse the new content.
-  if (isNewlyAddedContentForServo) {
+  // We couldn't construct lazily. Make Servo eagerly traverse the new content
+  // if needed (when aInsertionKind == InsertionKind::Sync, we know that the
+  // styles are up-to-date already).
+  if (aInsertionKind == InsertionKind::Async && aContainer->IsStyledByServo()) {
     StyleNewChildRange(aStartChild, aEndChild);
   }
 
   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!
@@ -8184,35 +8165,34 @@ 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,
-                                       aInsertionKind,
-                                       aForReconstruction);
+                                       aInsertionKind);
     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,
-                                 aInsertionKind, aForReconstruction);
+                                 aInsertionKind);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
   nsIContent* container = insertion.mParentFrame->GetContent();
 
   LayoutFrameType frameType = insertion.mParentFrame->Type();
   LAYOUT_PHASE_TEMP_EXIT();
@@ -8341,17 +8321,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,
-                                     aInsertionKind, aForReconstruction);
+                                     aInsertionKind);
         LAYOUT_PHASE_TEMP_REENTER();
         return;
       }
 
       container = insertion.mParentFrame->GetContent();
       frameType = insertion.mParentFrame->Type();
     }
   }
@@ -10060,17 +10040,16 @@ nsCSSFrameConstructor::RecreateFramesFor
                                            nsChangeHint_ReconstructFrame);
       } else {
         // 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.
         ContentRangeInserted(container, aContent, aContent->GetNextSibling(),
                              mTempFrameTreeState,
                              aInsertionKind,
-                             true,  // aForReconstruction
                              nullptr);
       }
     }
   }
 }
 
 bool
 nsCSSFrameConstructor::DestroyFramesFor(Element* aElement)
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -122,18 +122,17 @@ private:
   void CheckBitsForLazyFrameConstruction(nsIContent*) {}
 #endif
 
   // Issues a single ContentInserted for each child of aContainer in the range
   // [aStartChild, aEndChild).
   void IssueSingleInsertNofications(nsIContent* aContainer,
                                     nsIContent* aStartChild,
                                     nsIContent* aEndChild,
-                                    InsertionKind,
-                                    bool aForReconstruction);
+                                    InsertionKind);
 
   /**
    * Data that represents an insertion point for some child content.
    */
   struct InsertionPoint
   {
     InsertionPoint()
       : mParentFrame(nullptr), mContainer(nullptr), mMultiple(false) {}
@@ -166,18 +165,17 @@ 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,
-                                        InsertionKind,
-                                        bool aForReconstruction);
+                                        InsertionKind);
 
   // Returns true if parent was recreated due to frameset child, false otherwise.
   bool MaybeRecreateForFrameset(nsIFrame* aParentFrame,
                                 nsIContent* aStartChild,
                                 nsIContent* aEndChild,
                                 InsertionKind);
 
   /**
@@ -250,21 +248,17 @@ 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,
                        InsertionKind aInsertionKind,
-                       TreeMatchContext* aProvidedTreeMatchContext = nullptr)
-  {
-    ContentAppended(aContainer, aFirstNewContent, aInsertionKind, false,
-                    aProvidedTreeMatchContext);
-  }
+                       TreeMatchContext* aProvidedTreeMatchContext = nullptr);
 
   // If aInsertionkind is Async then frame construction of the new child
   // can be done lazily.
   void ContentInserted(nsIContent* aContainer,
                        nsIContent* aChild,
                        nsILayoutHistoryState* aFrameState,
                        InsertionKind aInsertionKind);
 
@@ -279,46 +273,18 @@ public:
   //
   // See ContentAppended to see why we allow passing an already initialized
   // TreeMatchContext.
   void ContentRangeInserted(nsIContent* aContainer,
                             nsIContent* aStartChild,
                             nsIContent* aEndChild,
                             nsILayoutHistoryState* aFrameState,
                             InsertionKind aInsertionKind,
-                            TreeMatchContext* aProvidedTreeMatchContext = nullptr)
-  {
-    ContentRangeInserted(aContainer, aStartChild, aEndChild, aFrameState,
-                         aInsertionKind, false,
-                         aProvidedTreeMatchContext);
-  }
+                            TreeMatchContext* aProvidedTreeMatchContext = nullptr);
 
-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,
-                       InsertionKind aInsertionKind,
-                       bool aForReconstruction,
-                       TreeMatchContext* aProvidedTreeMatchContext);
-  void ContentRangeInserted(nsIContent* aContainer,
-                            nsIContent* aStartChild,
-                            nsIContent* aEndChild,
-                            nsILayoutHistoryState* aFrameState,
-                            InsertionKind aInsertionKind,
-                            bool aForReconstruction,
-                            TreeMatchContext* aProvidedTreeMatchContext);
-
-public:
   enum RemoveFlags {
     REMOVE_CONTENT,
     REMOVE_FOR_RECONSTRUCTION,
   };
 
   /**
    * Recreate or destroy frames for aChild in aContainer.
    *