Bug 1146101 - Call ClearCachedInheritedStyleDataOnDescendants on more style contexts that had structs swapped out from them. r=dbaron a=abillings
authorCameron McCormack <cam@mcc.id.au>
Wed, 15 Apr 2015 08:13:45 +1000
changeset 239173 a95d4de3ea08e52104fbccfb478dd263a0f98a42
parent 239172 5f48890c23c1f4446e01014dd04dcb622695f3b5
child 239174 d60d4581a7e9908522d85f92bb4868fc0b12f65c
push id28584
push usercbook@mozilla.com
push dateWed, 15 Apr 2015 12:29:01 +0000
treeherdermozilla-central@b58b07945d30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, abillings
bugs1146101
milestone40.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 1146101 - Call ClearCachedInheritedStyleDataOnDescendants on more style contexts that had structs swapped out from them. r=dbaron a=abillings
layout/base/RestyleManager.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -2822,21 +2822,21 @@ ElementRestyler::Restyle(nsRestyleHint a
       LOG_RESTYLE("moving style context %p from old parent %p to new parent %p",
                   oldContext.get(), oldContext->GetParent(), newParent);
       // We keep strong references to the new parent around until the end
       // of the restyle, in case:
       //   (a) we swapped structs between the old and new parent,
       //   (b) some descendants of the old parent are not getting restyled
       //       (which is the reason for the existence of
       //       ClearCachedInheritedStyleDataOnDescendants),
-      //   (d) something under ProcessPendingRestyles (which notably is called
+      //   (c) something under ProcessPendingRestyles (which notably is called
       //       *before* ClearCachedInheritedStyleDataOnDescendants is called
       //       on the old context) causes the new parent to be destroyed, thus
       //       destroying its owned structs, and
-      //   (c) something under ProcessPendingRestyles then wants to use of those
+      //   (d) something under ProcessPendingRestyles then wants to use of those
       //       now destroyed structs (through the old parent's descendants).
       mSwappedStructOwners.AppendElement(newParent);
       oldContext->MoveTo(newParent);
     }
 
     // Send the accessibility notifications that RestyleChildren otherwise
     // would have sent.
     if (!(mHintsHandled & nsChangeHint_ReconstructFrame)) {
@@ -3386,16 +3386,25 @@ ElementRestyler::RestyleSelf(nsIFrame* a
           }
 #endif
         }
         LOG_RESTYLE("setting new style context");
         aSelf->SetStyleContext(newContext);
       }
     } else {
       LOG_RESTYLE("not setting new style context, since we'll reframe");
+      // We need to keep the new parent alive, in case it had structs
+      // swapped into it that our frame's style context still has cached.
+      // This is a similar scenario to the one described in the
+      // ElementRestyler::Restyle comment where we append to
+      // mSwappedStructOwners.
+      //
+      // We really only need to do this if we did swap structs on the
+      // parent, but we don't have that information here.
+      mSwappedStructOwners.AppendElement(newContext->GetParent());
     }
   }
   oldContext = nullptr;
 
   // do additional contexts
   // XXXbz might be able to avoid selector matching here in some
   // cases; won't worry about it for now.
   int32_t contextIndex = 0;
@@ -4047,27 +4056,25 @@ void
 RestyleManager::ComputeAndProcessStyleChange(nsIFrame*          aFrame,
                                              nsChangeHint       aMinChange,
                                              RestyleTracker&    aRestyleTracker,
                                              nsRestyleHint      aRestyleHint)
 {
   MOZ_ASSERT(mReframingStyleContexts, "should have rsc");
   nsStyleChangeList changeList;
   nsTArray<ElementRestyler::ContextToClear> contextsToClear;
-  {
-    // swappedStructOwners needs to be kept alive until after
-    // ProcessRestyledFrames; see comment in ElementRestyler::Restyle.
-    // (Destroying it before the ClearCachedInheritedStyleDataOnDescendants call
-    // helps minimize the work done by that function.)
-    nsTArray<nsRefPtr<nsStyleContext>> swappedStructOwners;
-    ElementRestyler::ComputeStyleChangeFor(aFrame, &changeList, aMinChange,
-                                           aRestyleTracker, aRestyleHint,
-                                           contextsToClear, swappedStructOwners);
-    ProcessRestyledFrames(changeList);
-  }
+
+  // swappedStructOwners needs to be kept alive until after
+  // ProcessRestyledFrames and ClearCachedInheritedStyleDataOnDescendants
+  // calls; see comment in ElementRestyler::Restyle.
+  nsTArray<nsRefPtr<nsStyleContext>> swappedStructOwners;
+  ElementRestyler::ComputeStyleChangeFor(aFrame, &changeList, aMinChange,
+                                         aRestyleTracker, aRestyleHint,
+                                         contextsToClear, swappedStructOwners);
+  ProcessRestyledFrames(changeList);
   ClearCachedInheritedStyleDataOnDescendants(contextsToClear);
 }
 
 void
 RestyleManager::ComputeAndProcessStyleChange(nsStyleContext*    aNewContext,
                                              Element*           aElement,
                                              nsChangeHint       aMinChange,
                                              RestyleTracker&    aRestyleTracker,
@@ -4082,31 +4089,29 @@ RestyleManager::ComputeAndProcessStyleCh
                                     nsRuleWalker::eRelevantLinkUnvisited,
                                     frame->PresContext()->Document());
   nsIContent* parent = aElement->GetParent();
   Element* parentElement =
     parent && parent->IsElement() ? parent->AsElement() : nullptr;
   treeMatchContext.InitAncestors(parentElement);
   nsTArray<nsIContent*> visibleKidsOfHiddenElement;
   nsTArray<ElementRestyler::ContextToClear> contextsToClear;
-  {
-    // swappedStructOwners needs to be kept alive until after
-    // ProcessRestyledFrames; see comment in ElementRestyler::Restyle.
-    // (Destroying it before the ClearCachedInheritedStyleDataOnDescendants call
-    // helps minimize the work done by that function.)
-    nsTArray<nsRefPtr<nsStyleContext>> swappedStructOwners;
-    nsStyleChangeList changeList;
-    ElementRestyler r(frame->PresContext(), aElement, &changeList, aMinChange,
-                      aRestyleTracker, treeMatchContext,
-                      visibleKidsOfHiddenElement, contextsToClear,
-                      swappedStructOwners);
-    r.RestyleChildrenOfDisplayContentsElement(frame, aNewContext, aMinChange,
-                                              aRestyleTracker, aRestyleHint);
-    ProcessRestyledFrames(changeList);
-  }
+
+  // swappedStructOwners needs to be kept alive until after
+  // ProcessRestyledFrames and ClearCachedInheritedStyleDataOnDescendants
+  // calls; see comment in ElementRestyler::Restyle.
+  nsTArray<nsRefPtr<nsStyleContext>> swappedStructOwners;
+  nsStyleChangeList changeList;
+  ElementRestyler r(frame->PresContext(), aElement, &changeList, aMinChange,
+                    aRestyleTracker, treeMatchContext,
+                    visibleKidsOfHiddenElement, contextsToClear,
+                    swappedStructOwners);
+  r.RestyleChildrenOfDisplayContentsElement(frame, aNewContext, aMinChange,
+                                            aRestyleTracker, aRestyleHint);
+  ProcessRestyledFrames(changeList);
   ClearCachedInheritedStyleDataOnDescendants(contextsToClear);
 }
 
 AutoDisplayContentsAncestorPusher::AutoDisplayContentsAncestorPusher(
   TreeMatchContext& aTreeMatchContext, nsPresContext* aPresContext,
   nsIContent* aParent)
   : mTreeMatchContext(aTreeMatchContext)
   , mPresContext(aPresContext)