Re-grab the document element style to avoid tripping assertions. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 10 Jul 2017 14:32:39 +0200
changeset 606522 9371422202c7eae7fb1a00ce385852223dff6af3
parent 606521 e6e3349557f5830e37e2b15d25b89ca1f9d2f6bf
child 606523 f54e07d2a4059f69816ed5c7d3cc6979df68b16c
child 606531 85fe36cd37947fa172e6a4d9e3e4e97bf38d813d
push id67713
push userbmo:emilio+bugs@crisal.io
push dateTue, 11 Jul 2017 00:43:03 +0000
reviewersheycam
milestone56.0a1
Re-grab the document element style to avoid tripping assertions. r?heycam Also, restrict LazyComputeBehavior::Allow to root elements and the body, since it's pretty much a hack (also, it wasn't doing the right thing before afaict, because we'd never re-resolve the document element style... That looks slightly fishy, but it was pre-existing). MozReview-Commit-ID: 2W6fg9Zmjuw
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsPresContext.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2517,20 +2517,20 @@ nsCSSFrameConstructor::ConstructDocEleme
   // unset all element restyle flags, since we don't have any
   // siblings.
   aDocElement->UnsetRestyleFlagsIfGecko();
 
   // --------- CREATE AREA OR BOX FRAME -------
   // FIXME: Should this use ResolveStyleContext?  (The calls in this
   // function are the only case in nsCSSFrameConstructor where we don't
   // do so for the construction of a style context for an element.)
-  RefPtr<nsStyleContext> styleContext;
-  styleContext = mPresShell->StyleSet()->ResolveStyleFor(aDocElement,
-                                                         nullptr,
-                                                         LazyComputeBehavior::Allow);
+  RefPtr<nsStyleContext> styleContext =
+    mPresShell->StyleSet()->ResolveStyleFor(aDocElement,
+                                            nullptr,
+                                            LazyComputeBehavior::Allow);
 
   const nsStyleDisplay* display = styleContext->StyleDisplay();
 
   // Ensure that our XBL bindings are installed.
   if (display->mBinding) {
     // Get the XBL loader.
     nsresult rv;
     bool resolveStyle;
@@ -2554,27 +2554,38 @@ nsCSSFrameConstructor::ConstructDocEleme
       mDocument->BindingManager()->AddToAttachedQueue(binding);
     }
 
     if (resolveStyle) {
       // FIXME: Should this use ResolveStyleContext?  (The calls in this
       // function are the only case in nsCSSFrameConstructor where we
       // don't do so for the construction of a style context for an
       // element.)
+      //
+      // FIXME(emilio): This LazyComputeBehavior::Allow looks fishy. It really
+      // wants to fully re-resolve the style, but it wont if the element is
+      // already styled afaict...
       styleContext = mPresShell->StyleSet()->ResolveStyleFor(aDocElement,
                                                              nullptr,
                                                              LazyComputeBehavior::Allow);
       display = styleContext->StyleDisplay();
     }
   }
 
   // We delay traversing the entire document until here, since we per above we
   // may invalidate the root style when we load doc stylesheets.
   if (ServoStyleSet* set = mPresShell->StyleSet()->GetAsServo()) {
     set->StyleDocument(TraversalRestyleBehavior::Normal);
+
+    // Grab up-to-date contexts from the element data. This isn't technically
+    // necessary, but it avoids having to special-case some assertions related
+    // to pseudo-element resolution when pseudo-elements are on the root.
+    styleContext =
+      set->ResolveStyleFor(aDocElement, nullptr, LazyComputeBehavior::Assert);
+    display = styleContext->StyleDisplay();
   }
 
   // --------- IF SCROLLABLE WRAP IN SCROLLFRAME --------
 
   NS_ASSERTION(!display->IsScrollableOverflow() ||
                state.mPresContext->IsPaginated() ||
                propagatedScrollFrom == aDocElement,
                "Scrollbars should have been propagated to the viewport");
@@ -5868,19 +5879,18 @@ nsCSSFrameConstructor::AddFrameConstruct
           // XXX: We should have a better way to restyle ourselves.
           ServoRestyleManager::ClearServoDataFromSubtree(element);
           styleSet->StyleNewSubtree(element);
 
           // Servo's should_traverse_children() in traversal.rs skips
           // styling descendants of elements with a -moz-binding the
           // first time. Thus call StyleNewChildren() again.
           styleSet->StyleNewChildren(element);
-
           styleContext =
-            styleSet->ResolveStyleFor(element, nullptr, LazyComputeBehavior::Allow);
+            styleSet->ResolveStyleFor(element, nullptr, LazyComputeBehavior::Assert);
         } else {
           styleContext =
             ResolveStyleContext(styleContext->GetParent(), aContent, &aState);
         }
 
         display = styleContext->StyleDisplay();
         aStyleContext = styleContext;
       }
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1480,19 +1480,18 @@ GetPropagatedScrollbarStylesForViewport(
 
   // docElement might be null if we're doing this after removing it.
   if (!docElement) {
     return nullptr;
   }
 
   // Check the style on the document root element
   StyleSetHandle styleSet = aPresContext->StyleSet();
-  RefPtr<nsStyleContext> rootStyle;
-  rootStyle = styleSet->ResolveStyleFor(docElement, nullptr,
-                                        LazyComputeBehavior::Allow);
+  RefPtr<nsStyleContext> rootStyle =
+    styleSet->ResolveStyleFor(docElement, nullptr, LazyComputeBehavior::Allow);
   if (CheckOverflow(rootStyle->StyleDisplay(), aStyles)) {
     // tell caller we stole the overflow style from the root element
     return docElement;
   }
 
   // Don't look in the BODY for non-HTML documents or HTML documents
   // with non-HTML roots
   // XXX this should be earlier; we shouldn't even look at the document root
@@ -1509,19 +1508,19 @@ GetPropagatedScrollbarStylesForViewport(
   nsCOMPtr<nsIContent> bodyElement = do_QueryInterface(body);
 
   if (!bodyElement ||
       !bodyElement->NodeInfo()->Equals(nsGkAtoms::body)) {
     // The body is not a <body> tag, it's a <frameset>.
     return nullptr;
   }
 
-  RefPtr<nsStyleContext> bodyStyle;
-  bodyStyle = styleSet->ResolveStyleFor(bodyElement->AsElement(), rootStyle,
-                                        LazyComputeBehavior::Allow);
+  RefPtr<nsStyleContext> bodyStyle =
+    styleSet->ResolveStyleFor(bodyElement->AsElement(), rootStyle,
+                              LazyComputeBehavior::Allow);
 
   if (CheckOverflow(bodyStyle->StyleDisplay(), aStyles)) {
     // tell caller we stole the overflow style from the body element
     return bodyElement;
   }
 
   return nullptr;
 }