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
--- 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,