Do LazyFC for anonymous nodes too, and cleanup MaybeConstructLazily. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 21 Mar 2018 00:22:20 +0100
changeset 770313 d0fb2e9f8e61cda5120e4427a2a4c1f14a012dc4
parent 770301 ca154564394d53d8113cbb5067d5fc55e46bd2fe
push id103373
push userbmo:emilio@crisal.io
push dateTue, 20 Mar 2018 23:33:58 +0000
reviewersxidorn
milestone61.0a1
Do LazyFC for anonymous nodes too, and cleanup MaybeConstructLazily. r?xidorn 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. Not doing this makes the ResolveServoStyle call to resolve style for the parent of a text-node assert in some cases, and rightfully so (basically, non-lazy frame construction can always inherit from out-of date styles, we just used to grab the style from the frame tree / display contents map, which happens to not assert). This assertion doesn't trigger unless you look up in the tree, of course, because we don't check the flags on the parent, so that's why it doesn't trigger for elements. The less risky and slightly crappier change is to avoid the assertion in there, using a function that doesn't have the same "out-of-date styles" check as ResolveServoStyle, but I think this is better. If you prefer I can do that in this bug and move this patch to a followup. 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. MozReview-Commit-ID: 2TmRNgpWaM
layout/base/nsCSSFrameConstructor.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6876,18 +6876,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());
     }
@@ -6903,49 +6902,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;
-  }
-
   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.
@@ -6972,25 +6962,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,