Bug 1447506: Do LazyFC for anonymous nodes too, and cleanup MaybeConstructLazily. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 21 Mar 2018 00:22:20 +0100
changeset 770347 9b60d377565141021f026d922e474da10f0614bb
parent 770346 bcd28130878e0880d045b2f7eac98d0481975091
push id103385
push userbmo:emilio@crisal.io
push dateWed, 21 Mar 2018 01:40:56 +0000
reviewersbz
bugs1447506
milestone61.0a1
Bug 1447506: Do LazyFC for anonymous nodes too, and cleanup MaybeConstructLazily. r?bz Servo knows how to walk through them via StyleChildrenIterator, so no reason this is not done for them. The old style system only used FlattenedTreeIterator, so couldn't find them. Also, not check the container to do lazy fc on the children, it makes no sense. The only reason we don't do lazy frame construction in XUL is because of XBL bindings, and those are loaded for the parent already, if we're about to construct frames for the children. Also, assert more tightly, we don't insert NAC lazily, that makes no sense. I can bring back the XUL container check if you want, though the devtools test for that widget is the only thing that broke, and I'd prefer to remove it. MozReview-Commit-ID: 2TmRNgpWaM
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6884,18 +6884,17 @@ nsCSSFrameConstructor::CheckBitsForLazyF
   // It's fine if one of node without primary frame is in a display:none
   // subtree.
   //
   // Also, it's fine if one of the nodes without primary frame is a display:
   // contents node.
   bool noPrimaryFrame = false;
   bool needsFrameBitSet = false;
   nsIContent* content = aParent;
-  while (content &&
-         !content->HasFlag(NODE_DESCENDANTS_NEED_FRAMES)) {
+  while (content && !content->HasFlag(NODE_DESCENDANTS_NEED_FRAMES)) {
     if (content->GetPrimaryFrame() && content->GetPrimaryFrame()->IsLeaf()) {
       noPrimaryFrame = needsFrameBitSet = false;
     }
     if (!noPrimaryFrame && !content->GetPrimaryFrame()) {
       nsStyleContext* sc = GetDisplayNoneStyleFor(content);
       noPrimaryFrame = !GetDisplayContentsStyleFor(content) &&
         (sc && !sc->IsInDisplayNoneSubtree());
     }
@@ -6911,49 +6910,40 @@ nsCSSFrameConstructor::CheckBitsForLazyF
   }
   NS_ASSERTION(!noPrimaryFrame, "Ancestors of nodes with frames to be "
     "constructed lazily should have frames");
   NS_ASSERTION(!needsFrameBitSet, "Ancestors of nodes with frames to be "
     "constructed lazily should not have NEEDS_FRAME bit set");
 }
 #endif
 
-// For inserts aChild should be valid, for appends it should be null.
 // Returns true if this operation can be lazy, false if not.
 //
 // FIXME(emilio, bug 1410020): This function assumes that the flattened tree
 // parent of all the appended children is the same, which, afaict, is not
 // necessarily true.
 //
-// But we disable lazy frame construction for shadow trees... We should fix
-// that, too.
-//
-// NOTE(emilio): The IsXULElement check is pretty unfortunate, but there's tons
-// of browser chrome code that rely on XBL bindings getting synchronously loaded
-// as soon as the elements get inserted in the DOM.
+// NOTE(emilio): The IsXULElement checks are pretty unfortunate, but there's
+// tons of browser chrome code that rely on XBL bindings getting synchronously
+// loaded as soon as the elements get inserted in the DOM.
 bool
 nsCSSFrameConstructor::MaybeConstructLazily(Operation aOperation,
-                                            nsIContent* aContainer,
                                             nsIContent* aChild)
 {
-  if (!aContainer || aContainer->IsInNativeAnonymousSubtree() ||
-      aContainer->IsXULElement()) {
-    return false;
-  }
-
+  MOZ_ASSERT(aChild->GetParent());
   if (aOperation == CONTENTINSERT) {
-    if (aChild->IsRootOfAnonymousSubtree() || aChild->IsXULElement()) {
+    MOZ_ASSERT(!aChild->IsRootOfAnonymousSubtree());
+    if (aChild->IsXULElement()) {
       return false;
     }
   } else { // CONTENTAPPEND
-    NS_ASSERTION(aOperation == CONTENTAPPEND,
-                 "operation should be either insert or append");
+    MOZ_ASSERT(aOperation == CONTENTAPPEND,
+               "operation should be either insert or append");
     for (nsIContent* child = aChild; child; child = child->GetNextSibling()) {
-      NS_ASSERTION(!child->IsRootOfAnonymousSubtree(),
-                   "Should be coming through the CONTENTINSERT case");
+      MOZ_ASSERT(!child->IsRootOfAnonymousSubtree());
       if (child->IsXULElement()) {
         return false;
       }
     }
   }
 
   // We can construct lazily; just need to set suitable bits in the content
   // tree.
@@ -6980,25 +6970,18 @@ nsCSSFrameConstructor::MaybeConstructLaz
                    //XXX the child->GetPrimaryFrame()->GetContent() != child
                    // check is needed due to bug 135040. Remove it once that's
                    // fixed.
                    "setting NEEDS_FRAME on a node that already has a frame?");
       child->SetFlags(NODE_NEEDS_FRAME);
     }
   }
 
-  // Walk up the tree setting the NODE_DESCENDANTS_NEED_FRAMES bit as we go.
-  // We need different handling for servo given the scoped restyle roots.
   CheckBitsForLazyFrameConstruction(parent);
-
-  if (RestyleManager()->IsGecko()) {
-    MOZ_CRASH("old style system disabled");
-  } else {
-    parent->AsElement()->NoteDescendantsNeedFramesForServo();
-  }
+  parent->AsElement()->NoteDescendantsNeedFramesForServo();
 
   return true;
 }
 
 
 void
 nsCSSFrameConstructor::IssueSingleInsertNofications(nsIContent* aContainer,
                                                     nsIContent* aStartChild,
@@ -7206,17 +7189,17 @@ nsCSSFrameConstructor::ContentAppended(n
     if (aContainer->IsStyledByServo() &&
         aInsertionKind == InsertionKind::Async) {
       LazilyStyleNewChildRange(aFirstNewContent, nullptr);
     }
     return;
   }
 
   if (aInsertionKind == InsertionKind::Async &&
-      MaybeConstructLazily(CONTENTAPPEND, aContainer, aFirstNewContent)) {
+      MaybeConstructLazily(CONTENTAPPEND, aFirstNewContent)) {
     if (aContainer->IsStyledByServo()) {
       LazilyStyleNewChildRange(aFirstNewContent, nullptr);
     }
     return;
   }
 
   // We couldn't construct lazily. Make Servo eagerly traverse the new content
   // if needed (when aInsertionKind == InsertionKind::Sync, we know that the
@@ -7676,17 +7659,17 @@ nsCSSFrameConstructor::ContentRangeInser
 
   MOZ_ASSERT_IF(aContainer->IsShadowRoot(), !parentFrame);
 
   // 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)) {
+      MaybeConstructLazily(CONTENTINSERT, aStartChild)) {
     if (aContainer->IsStyledByServo()) {
       LazilyStyleNewChildRange(aStartChild, aEndChild);
     }
     return;
   }
 
   // We couldn't construct lazily. Make Servo eagerly traverse the new content
   // if needed.
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -106,19 +106,17 @@ private:
 
   enum Operation {
     CONTENTAPPEND,
     CONTENTINSERT
   };
 
   // aChild is the child being inserted for inserts, and the first
   // child being appended for appends.
-  bool MaybeConstructLazily(Operation aOperation,
-                            nsIContent* aContainer,
-                            nsIContent* aChild);
+  bool MaybeConstructLazily(Operation aOperation, nsIContent* aChild);
 
 #ifdef DEBUG
   void CheckBitsForLazyFrameConstruction(nsIContent* aParent);
 #else
   void CheckBitsForLazyFrameConstruction(nsIContent*) {}
 #endif
 
   // Issues a single ContentInserted for each child of aContainer in the range