Bug 1394586: Unconditionally clear the restyle flags on Element::BindToTree. r?bholley draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 29 Aug 2017 16:12:58 +0200
changeset 655093 2bd880fdd5f212a070fa6da5691b9850cc6cd54c
parent 655025 e3eeeb64640707c24c7e9a53f452c2bc14fbd150
child 728742 8bbe3f4fc20f8693d4f2eb6a47f595098f1fa9ee
push id76771
push userbmo:emilio@crisal.io
push dateTue, 29 Aug 2017 17:08:17 +0000
reviewersbholley
bugs1394586
milestone57.0a1
Bug 1394586: Unconditionally clear the restyle flags on Element::BindToTree. r?bholley The failure here is a <script> that document.writes a whole HTML document, which happens to have the Gecko style back-end. Of all the elements written there, there's an <img> element which triggers a load, which finishes and changes the state of the <img>, posting a restyle event to its Gecko-backed restyle manager. At that point, but before the refresh driver ticks, the element is unbound from the tree, keeping the restyle root bits from the Gecko back-end. Afterwards, the element is bound to a Servo-backed document, so ClearRestyleFlagsIfGecko does nothing (since the document pointer is already set, and points to a Servo-backed document). Furthermore, it's inserted on a display: none subtree, so we never reach it during the traversal to remove the flags. Other alternative fixes for this same issue would be: * Clearing the Gecko restyle flags in UnbindFromTree. Gecko doesn't seem to prevent restyle roots from being outside of the document (it just does nothing when finding them on ProcessPendingRestyles), so it doesn't seem a very reliable solution. * Reorder the flag clearing to be before setting the document in BindToTree. Seems somewhat risky, and for no good reason. MozReview-Commit-ID: B6iZKhGHc3A
dom/base/Element.cpp
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1602,29 +1602,35 @@ Element::BindToTree(nsIDocument* aDocume
     ClearSubtreeRootPointer();
 
     // Being added to a document.
     SetIsInDocument();
 
     // Unset this flag since we now really are in a document.
     UnsetFlags(NODE_FORCE_XBL_BINDINGS |
                // And clear the lazy frame construction bits.
-               NODE_NEEDS_FRAME | NODE_DESCENDANTS_NEED_FRAMES);
-    // And the restyle bits
-    UnsetRestyleFlagsIfGecko();
+               NODE_NEEDS_FRAME | NODE_DESCENDANTS_NEED_FRAMES |
+               // And the restyle bits. These shouldn't even get set if we came
+               // from a Servo-styled document, but they may be set if the
+               // element comes from a Gecko-backed document, see bug 1394586.
+               //
+               // TODO(emilio): We can remove this and assert we don't have any
+               // of them when we remove the old style system.
+               ELEMENT_ALL_RESTYLE_FLAGS);
   } else if (IsInShadowTree()) {
     // We're not in a document, but we did get inserted into a shadow tree.
     // Since we won't have any restyle data in the document's restyle trackers,
     // don't let us get inserted with restyle bits set incorrectly.
     //
     // Also clear all the other flags that are cleared above when we do get
     // inserted into a document.
-    UnsetFlags(NODE_FORCE_XBL_BINDINGS |
-               NODE_NEEDS_FRAME | NODE_DESCENDANTS_NEED_FRAMES);
-    UnsetRestyleFlagsIfGecko();
+    //
+    // See the comment about the restyle bits above, it also applies.
+    UnsetFlags(NODE_FORCE_XBL_BINDINGS | NODE_NEEDS_FRAME |
+               NODE_DESCENDANTS_NEED_FRAMES | ELEMENT_ALL_RESTYLE_FLAGS);
   } else {
     // If we're not in the doc and not in a shadow tree,
     // update our subtree pointer.
     SetSubtreeRootPointer(aParent->SubtreeRoot());
   }
 
   nsIDocument* composedDoc = GetComposedDoc();
   if (composedDoc) {