Bug 1146101 - Call ClearCachedInheritedStyleDataOnDescendants on more style contexts that had structs swapped out from them. r=dbaron, a=sledru
authorCameron McCormack <cam@mcc.id.au>
Wed, 15 Apr 2015 08:13:45 +1000
changeset 258479 baa8222aaafd
parent 258478 4f36d5aff5cf
child 258480 01e0d4e09b6d
push id4678
push userryanvm@gmail.com
push date2015-04-15 13:36 +0000
treeherdermozilla-beta@e487ace8d7f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, sledru
bugs1146101
milestone38.0
Bug 1146101 - Call ClearCachedInheritedStyleDataOnDescendants on more style contexts that had structs swapped out from them. r=dbaron, a=sledru
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)) {
@@ -3379,16 +3379,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;
@@ -4029,27 +4038,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,
@@ -4064,31 +4071,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)