Bug 1370793 - Part 1: Don't try to style unstyled children of elements with newly applied XBL bindings if in a display:none or unstyled subtree. r?bholley draft
authorCameron McCormack <cam@mcc.id.au>
Sun, 11 Jun 2017 19:11:08 +0800
changeset 592935 17b5c00c74783f626a5f1e6d089a04747d91b2cc
parent 592895 e3638c5119ed03dcd6ad243f99b08251194e375b
child 592936 46efdecd7964a8cf4382b49f08a8fb654e57a6f3
push id63544
push userbmo:cam@mcc.id.au
push dateMon, 12 Jun 2017 23:35:12 +0000
reviewersbholley
bugs1370793
milestone56.0a1
Bug 1370793 - Part 1: Don't try to style unstyled children of elements with newly applied XBL bindings if in a display:none or unstyled subtree. r?bholley MozReview-Commit-ID: EFi2Vp19AQm
dom/xbl/nsXBLService.cpp
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
layout/style/ServoTypes.h
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -415,27 +415,17 @@ public:
   explicit AutoStyleNewChildren(Element* aElement) : mElement(aElement) { MOZ_ASSERT(mElement); }
   ~AutoStyleNewChildren()
   {
     nsIPresShell* presShell = mElement->OwnerDoc()->GetShell();
     if (!presShell || !presShell->DidInitialize()) {
       return;
     }
     if (ServoStyleSet* servoSet = presShell->StyleSet()->GetAsServo()) {
-      // In general the element is always styled by the time we're applying XBL
-      // bindings, because we need to style the element to know what the binding
-      // URI is. However, programmatic consumers of the XBL service (like the
-      // XML pretty printer) _can_ apply bindings without having styled the bound
-      // element. We could assert against this and require the callers manually
-      // resolve the style first, but it's easy enough to just handle here.
-      if (MOZ_UNLIKELY(!mElement->HasServoData())) {
-        servoSet->StyleNewSubtree(mElement);
-      } else {
-        servoSet->StyleNewChildren(mElement);
-      }
+      servoSet->StyleNewlyBoundElement(mElement);
     }
   }
 
 private:
   Element* mElement;
 };
 
 // This function loads a particular XBL file and installs all of the bindings
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -369,19 +369,22 @@ ServoStyleSet::PrepareAndTraverseSubtree
 
   const SnapshotTable& snapshots = Snapshots();
 
   bool isInitial = !aRoot->HasServoData();
   bool forReconstruct =
     aRestyleBehavior == TraversalRestyleBehavior::ForReconstruct;
   bool forAnimationOnly =
     aRestyleBehavior == TraversalRestyleBehavior::ForAnimationOnly;
+  bool forNewlyBoundElement =
+    aRestyleBehavior == TraversalRestyleBehavior::ForNewlyBoundElement;
   bool postTraversalRequired = Servo_TraverseSubtree(
     aRoot, mRawSet.get(), &snapshots, aRootBehavior, aRestyleBehavior);
-  MOZ_ASSERT(!(isInitial || forReconstruct) || !postTraversalRequired);
+  MOZ_ASSERT(!(isInitial || forReconstruct || forNewlyBoundElement) ||
+             !postTraversalRequired);
 
   // Don't need to trigger a second traversal if this restyle only needs
   // animation-only restyle.
   if (forAnimationOnly) {
     return postTraversalRequired;
   }
 
   auto root = const_cast<Element*>(aRoot);
@@ -951,16 +954,44 @@ ServoStyleSet::StyleNewChildren(Element*
   PrepareAndTraverseSubtree(aParent,
                             TraversalRootBehavior::UnstyledChildrenOnly,
                             TraversalRestyleBehavior::Normal);
   // We can't assert that Servo_TraverseSubtree returns false, since aParent
   // or some of its other children might have pending restyles.
 }
 
 void
+ServoStyleSet::StyleNewlyBoundElement(Element* aElement)
+{
+  PreTraverse();
+
+  // In general the element is always styled by the time we're applying XBL
+  // bindings, because we need to style the element to know what the binding
+  // URI is. However, programmatic consumers of the XBL service (like the
+  // XML pretty printer) _can_ apply bindings without having styled the bound
+  // element. We could assert against this and require the callers manually
+  // resolve the style first, but it's easy enough to just handle here.
+  //
+  // Also, when applying XBL bindings to elements within a display:none or
+  // unstyled subtree (for example, when <object> elements are wrapped to be
+  // exposed to JS), we need to tell the traversal that it is OK to
+  // skip restyling, rather than panic when trying to unwrap the styles
+  // it expects to have just computed.
+
+  TraversalRootBehavior rootBehavior =
+    MOZ_UNLIKELY(!aElement->HasServoData())
+      ? TraversalRootBehavior::Normal
+      : TraversalRootBehavior::UnstyledChildrenOnly;
+
+  PrepareAndTraverseSubtree(aElement,
+                            rootBehavior,
+                            TraversalRestyleBehavior::ForNewlyBoundElement);
+}
+
+void
 ServoStyleSet::StyleSubtreeForReconstruct(Element* aRoot)
 {
   PreTraverse(aRoot);
 
   DebugOnly<bool> postTraversalRequired =
     PrepareAndTraverseSubtree(aRoot,
                               TraversalRootBehavior::Normal,
                               TraversalRestyleBehavior::ForReconstruct);
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -301,16 +301,24 @@ public:
    * Like the above, but skips the root node, and only styles unstyled children.
    * When potentially appending multiple children, it's preferable to call
    * StyleNewChildren on the node rather than making multiple calls to
    * StyleNewSubtree on each child, since it allows for more parallelism.
    */
   void StyleNewChildren(dom::Element* aParent);
 
   /**
+   * Eagerly styles the children of an element that has just had an XBL
+   * binding applied to it.  Some XBL consumers attach bindings to elements
+   * that have not been styled yet, and in such cases, this will do the
+   * equivalent of StyleNewSubtree instead.
+   */
+  void StyleNewlyBoundElement(dom::Element* aElement);
+
+  /**
    * Like StyleNewSubtree, but in response to a request to reconstruct frames
    * for the given subtree, and so works on elements that already have
    * styles.  This will leave the subtree in a state just like after an initial
    * styling, i.e. with new styles, no change hints, and with the dirty
    * descendants bits cleared.  No comparison of old and new styles is done,
    * so no change hints will be processed.
    */
   void StyleSubtreeForReconstruct(dom::Element* aRoot);
--- a/layout/style/ServoTypes.h
+++ b/layout/style/ServoTypes.h
@@ -58,16 +58,20 @@ enum class TraversalRootBehavior {
 // or whether it should traverse in a mode that doesn't generate any change
 // hints, which is what's required when handling frame reconstruction.
 // The change hints in this case are unneeded, since the old frames have
 // already been destroyed.
 // Indicates how the Servo style system should perform.
 enum class TraversalRestyleBehavior {
   // Normal processing.
   Normal,
+  // Normal processing, but tolerant to calls to restyle elements in unstyled
+  // or display:none subtrees (which can occur when styling elements with
+  // newly applied XBL bindings).
+  ForNewlyBoundElement,
   // Traverses in a mode that doesn't generate any change hints, which is what's
   // required when handling frame reconstruction.  The change hints in this case
   // are unneeded, since the old frames have already been destroyed.
   ForReconstruct,
   // Processes animation-only restyle.
   ForAnimationOnly,
   // Traverses as normal mode but tries to update all CSS animations.
   ForCSSRuleChanges,