Bug 1350441 - Clear servo data when tearing down frames for DestroyFramesFor. r=heycam
authorBobby Holley <bobbyholley@gmail.com>
Thu, 23 Mar 2017 17:16:00 -0700
changeset 350387 d09572f941c52f2f0b64ba7c7089208d91f47e8a
parent 350386 abcdc4570f03657167ba46025677211b27e71444
child 350388 d124e2cc2e68b329550ad77569da4d790b003400
push id39780
push userbholley@mozilla.com
push dateWed, 29 Mar 2017 16:27:31 +0000
treeherderautoland@d09572f941c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1350441
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1350441 - Clear servo data when tearing down frames for DestroyFramesFor. r=heycam MozReview-Commit-ID: DWwu8FqSjdj
layout/base/nsCSSFrameConstructor.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -8406,16 +8406,40 @@ nsCSSFrameConstructor::ContentRemoved(ns
   NS_PRECONDITION(mUpdateCount != 0,
                   "Should be in an update while destroying frames");
 
   *aDidReconstruct = false;
   if (aDestroyedFramesFor) {
     *aDestroyedFramesFor = aChild;
   }
 
+  // We're destroying our frame(s). This normally happens either when the content
+  // is being removed from the DOM (in which case we'll drop all Servo data in
+  // UnbindFromTree), or when we're recreating frames (usually in response to
+  // having retrieved a ReconstructFrame change hint after restyling). In both of
+  // those cases, there are no pending restyles we need to worry about.
+  //
+  // However, there is also the (rare) DestroyFramesFor path, in which we tear
+  // down (and usually recreate) the frames for a subtree. In this case, leaving
+  // the style data on the elements is problematic for our invariants, because
+  // there might be pending restyles in the subtree. If we simply leave them as-is,
+  // the subsequent traversal when recreating frames will generate a bunch of bogus
+  // change hints to update frames that no longer exist.
+  //
+  // So the two obvious options are to (1) process all pending restyles and take all
+  // the change hints before destroying the frames, or (2) drop all the style data.
+  // We chose the latter, since that matches the performance characteristics of the
+  // old Gecko style system.
+  //
+  // That said, it's almost certainly possible to optimize this if it turns out to be
+  // hot. It's just not a priority at the moment.
+  if (aFlags == REMOVE_DESTROY_FRAMES && aChild->IsElement() && aChild->IsStyledByServo()) {
+    ServoRestyleManager::ClearServoDataFromSubtree(aChild->AsElement());
+  }
+
   if (aChild->IsHTMLElement(nsGkAtoms::body) ||
       (!aContainer && aChild->IsElement())) {
     // This might be the element we propagated viewport scrollbar
     // styles from.  Recompute those.
     mPresShell->GetPresContext()->UpdateViewportScrollbarStylesOverride();
   }
 
   // XXXldb Do we need to re-resolve style to handle the CSS2 + combinator and