Bug 1188721 - Part 10: Stop clearing cached struct pointers in style context descendants. r?dbaron draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 23 Mar 2016 17:35:59 +1100
changeset 343760 29539b7116c6eb32b3652283d3dc717780837c46
parent 343759 fa51c699cd821d21adfc79cbf373356d39849426
child 343761 0ca5bf33a58b56537010599a4ed13435c976c7da
push id13680
push usercmccormack@mozilla.com
push dateWed, 23 Mar 2016 06:36:18 +0000
reviewersdbaron
bugs1188721
milestone48.0a1
Bug 1188721 - Part 10: Stop clearing cached struct pointers in style context descendants. r?dbaron MozReview-Commit-ID: 7px5ly23N2l
layout/base/RestyleManager.cpp
layout/base/RestyleManager.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -2526,17 +2526,16 @@ ElementRestyler::ElementRestyler(nsPresC
                                  nsStyleChangeList* aChangeList,
                                  nsChangeHint aHintsHandledByAncestors,
                                  RestyleTracker& aRestyleTracker,
                                  nsTArray<nsCSSSelector*>&
                                    aSelectorsForDescendants,
                                  TreeMatchContext& aTreeMatchContext,
                                  nsTArray<nsIContent*>&
                                    aVisibleKidsOfHiddenElement,
-                                 nsTArray<ContextToClear>& aContextsToClear,
                                  nsTArray<RefPtr<nsStyleContext>>&
                                    aSwappedStructOwners)
   : mPresContext(aPresContext)
   , mFrame(aFrame)
   , mParentContent(nullptr)
     // XXXldb Why does it make sense to use aParentContent?  (See
     // comment above assertion at start of ElementRestyler::Restyle.)
   , mContent(mFrame->GetContent() ? mFrame->GetContent() : mParentContent)
@@ -2544,17 +2543,16 @@ ElementRestyler::ElementRestyler(nsPresC
   , mHintsHandled(NS_SubtractHint(aHintsHandledByAncestors,
                   NS_HintsNotHandledForDescendantsIn(aHintsHandledByAncestors)))
   , mParentFrameHintsNotHandledForDescendants(nsChangeHint(0))
   , mHintsNotHandledForDescendants(nsChangeHint(0))
   , mRestyleTracker(aRestyleTracker)
   , mSelectorsForDescendants(aSelectorsForDescendants)
   , mTreeMatchContext(aTreeMatchContext)
   , mResolvedChild(nullptr)
-  , mContextsToClear(aContextsToClear)
   , mSwappedStructOwners(aSwappedStructOwners)
   , mIsRootOfRestyle(true)
 #ifdef ACCESSIBILITY
   , mDesiredA11yNotifications(eSendAllNotifications)
   , mKidsDesiredA11yNotifications(mDesiredA11yNotifications)
   , mOurA11yNotification(eDontNotify)
   , mVisibleKidsOfHiddenElement(aVisibleKidsOfHiddenElement)
 #endif
@@ -2578,17 +2576,16 @@ ElementRestyler::ElementRestyler(const E
                   NS_HintsNotHandledForDescendantsIn(aParentRestyler.mHintsHandled)))
   , mParentFrameHintsNotHandledForDescendants(
       aParentRestyler.mHintsNotHandledForDescendants)
   , mHintsNotHandledForDescendants(nsChangeHint(0))
   , mRestyleTracker(aParentRestyler.mRestyleTracker)
   , mSelectorsForDescendants(aParentRestyler.mSelectorsForDescendants)
   , mTreeMatchContext(aParentRestyler.mTreeMatchContext)
   , mResolvedChild(nullptr)
-  , mContextsToClear(aParentRestyler.mContextsToClear)
   , mSwappedStructOwners(aParentRestyler.mSwappedStructOwners)
   , mIsRootOfRestyle(false)
 #ifdef ACCESSIBILITY
   , mDesiredA11yNotifications(aParentRestyler.mKidsDesiredA11yNotifications)
   , mKidsDesiredA11yNotifications(mDesiredA11yNotifications)
   , mOurA11yNotification(eDontNotify)
   , mVisibleKidsOfHiddenElement(aParentRestyler.mVisibleKidsOfHiddenElement)
 #endif
@@ -2626,17 +2623,16 @@ ElementRestyler::ElementRestyler(ParentC
   , mParentFrameHintsNotHandledForDescendants(
       // assume the worst
       nsChangeHint_Hints_NotHandledForDescendants)
   , mHintsNotHandledForDescendants(nsChangeHint(0))
   , mRestyleTracker(aParentRestyler.mRestyleTracker)
   , mSelectorsForDescendants(aParentRestyler.mSelectorsForDescendants)
   , mTreeMatchContext(aParentRestyler.mTreeMatchContext)
   , mResolvedChild(nullptr)
-  , mContextsToClear(aParentRestyler.mContextsToClear)
   , mSwappedStructOwners(aParentRestyler.mSwappedStructOwners)
   , mIsRootOfRestyle(false)
 #ifdef ACCESSIBILITY
   , mDesiredA11yNotifications(aParentRestyler.mDesiredA11yNotifications)
   , mKidsDesiredA11yNotifications(mDesiredA11yNotifications)
   , mOurA11yNotification(eDontNotify)
   , mVisibleKidsOfHiddenElement(aParentRestyler.mVisibleKidsOfHiddenElement)
 #endif
@@ -2650,33 +2646,31 @@ ElementRestyler::ElementRestyler(nsPresC
                                  nsIContent* aContent,
                                  nsStyleChangeList* aChangeList,
                                  nsChangeHint aHintsHandledByAncestors,
                                  RestyleTracker& aRestyleTracker,
                                  nsTArray<nsCSSSelector*>& aSelectorsForDescendants,
                                  TreeMatchContext& aTreeMatchContext,
                                  nsTArray<nsIContent*>&
                                    aVisibleKidsOfHiddenElement,
-                                 nsTArray<ContextToClear>& aContextsToClear,
                                  nsTArray<RefPtr<nsStyleContext>>&
                                    aSwappedStructOwners)
   : mPresContext(aPresContext)
   , mFrame(nullptr)
   , mParentContent(nullptr)
   , mContent(aContent)
   , mChangeList(aChangeList)
   , mHintsHandled(NS_SubtractHint(aHintsHandledByAncestors,
                   NS_HintsNotHandledForDescendantsIn(aHintsHandledByAncestors)))
   , mParentFrameHintsNotHandledForDescendants(nsChangeHint(0))
   , mHintsNotHandledForDescendants(nsChangeHint(0))
   , mRestyleTracker(aRestyleTracker)
   , mSelectorsForDescendants(aSelectorsForDescendants)
   , mTreeMatchContext(aTreeMatchContext)
   , mResolvedChild(nullptr)
-  , mContextsToClear(aContextsToClear)
   , mSwappedStructOwners(aSwappedStructOwners)
   , mIsRootOfRestyle(true)
 #ifdef ACCESSIBILITY
   , mDesiredA11yNotifications(eSendAllNotifications)
   , mKidsDesiredA11yNotifications(mDesiredA11yNotifications)
   , mOurA11yNotification(eDontNotify)
   , mVisibleKidsOfHiddenElement(aVisibleKidsOfHiddenElement)
 #endif
@@ -3394,33 +3388,16 @@ ElementRestyler::Restyle(nsRestyleHint a
   // It's also important to check mHintsHandled since we use
   // mFrame->StyleContext(), which is out of date if mHintsHandled
   // has a ReconstructFrame hint.  Using an out of date style
   // context could trigger assertions about mismatched rule trees.
   if (!(mHintsHandled & nsChangeHint_ReconstructFrame)) {
     RestyleChildren(childRestyleHint);
   }
 
-  if (oldContext && !oldContext->HasSingleReference()) {
-    // If we swapped some structs out of oldContext in the RestyleSelf call
-    // and after the RestyleChildren call we still have other strong references
-    // to it, we need to make ensure its descendants don't cache any of the
-    // structs that were swapped out.
-    //
-    // Much of the time we will not get in here; we do for example when the
-    // style context is shared with a later IB split sibling (which we won't
-    // restyle until a bit later) or if other code is holding a strong reference
-    // to the style context (as is done by nsTransformedTextRun objects, which
-    // can be referenced by a text frame's mTextRun longer than the frame's
-    // mStyleContext).
-    ContextToClear* toClear = mContextsToClear.AppendElement();
-    toClear->mStyleContext = Move(oldContext);
-    toClear->mStructs = 0;
-  }
-
   mRestyleTracker.AddRestyleRootsIfAwaitingRestyle(descendants);
 }
 
 /**
  * Depending on the details of the frame we are restyling or its old style
  * context, we may or may not be able to stop restyling after this frame if
  * we find we had no style changes.
  *
@@ -4304,17 +4281,17 @@ ElementRestyler::RestyleChildrenOfDispla
     nsIFrame::ChildListIterator lists(aParentFrame);
     for ( ; !lists.IsDone(); lists.Next()) {
       for (nsIFrame* f : lists.CurrentList()) {
         if (nsContentUtils::ContentIsDescendantOf(f->GetContent(), mContent) &&
             !f->GetPrevContinuation()) {
           if (!(f->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) {
             ComputeStyleChangeFor(f, mChangeList, aMinHint, aRestyleTracker,
                                   aRestyleHint, aRestyleHintData,
-                                  mContextsToClear, mSwappedStructOwners);
+                                  mSwappedStructOwners);
           }
         }
       }
     }
   }
   if (!(mHintsHandled & nsChangeHint_ReconstructFrame)) {
     SendAccessibilityNotifications();
   }
@@ -4322,18 +4299,16 @@ ElementRestyler::RestyleChildrenOfDispla
 
 void
 ElementRestyler::ComputeStyleChangeFor(nsIFrame*          aFrame,
                                        nsStyleChangeList* aChangeList,
                                        nsChangeHint       aMinChange,
                                        RestyleTracker&    aRestyleTracker,
                                        nsRestyleHint      aRestyleHint,
                                        const RestyleHintData& aRestyleHintData,
-                                       nsTArray<ContextToClear>&
-                                         aContextsToClear,
                                        nsTArray<RefPtr<nsStyleContext>>&
                                          aSwappedStructOwners)
 {
   nsIContent* content = aFrame->GetContent();
   nsAutoCString localDescriptor;
   if (profiler_is_active() && content) {
     std::string elemDesc = ToString(*content);
     localDescriptor.Assign(elemDesc.c_str());
@@ -4393,17 +4368,17 @@ ElementRestyler::ComputeStyleChangeFor(n
       }
 
       // Inner loop over next-in-flows of the current frame
       ElementRestyler restyler(presContext, cont, aChangeList,
                                aMinChange, aRestyleTracker,
                                selectorsForDescendants,
                                treeMatchContext,
                                visibleKidsOfHiddenElement,
-                               aContextsToClear, aSwappedStructOwners);
+                               aSwappedStructOwners);
 
       restyler.Restyle(aRestyleHint);
 
       if (restyler.HintsHandledForFrame() & nsChangeHint_ReconstructFrame) {
         // If it's going to cause a framechange, then don't bother
         // with the continuations or ib-split siblings since they'll be
         // clobbered by the frame reconstruct anyway.
         NS_ASSERTION(!cont->GetPrevContinuation(),
@@ -4776,51 +4751,34 @@ ElementRestyler::SendAccessibilityNotifi
                                          childContent->GetNextSibling());
       }
       mVisibleKidsOfHiddenElement.Clear();
     }
   }
 #endif
 }
 
-static void
-ClearCachedInheritedStyleDataOnDescendants(
-    nsTArray<ElementRestyler::ContextToClear>& aContextsToClear)
-{
-  for (size_t i = 0; i < aContextsToClear.Length(); i++) {
-    auto& entry = aContextsToClear[i];
-    if (!entry.mStyleContext->HasSingleReference()) {
-      entry.mStyleContext->ClearCachedInheritedStyleDataOnDescendants(
-          entry.mStructs);
-    }
-    entry.mStyleContext = nullptr;
-  }
-}
-
 void
 RestyleManager::ComputeAndProcessStyleChange(nsIFrame*              aFrame,
                                              nsChangeHint           aMinChange,
                                              RestyleTracker&        aRestyleTracker,
                                              nsRestyleHint          aRestyleHint,
                                              const RestyleHintData& aRestyleHintData)
 {
   MOZ_ASSERT(mReframingStyleContexts, "should have rsc");
   nsStyleChangeList changeList;
-  nsTArray<ElementRestyler::ContextToClear> contextsToClear;
 
   // swappedStructOwners needs to be kept alive until after
-  // ProcessRestyledFrames and ClearCachedInheritedStyleDataOnDescendants
-  // calls; see comment in ElementRestyler::Restyle.
+  // ProcessRestyledFrames; see comment in ElementRestyler::Restyle.
   nsTArray<RefPtr<nsStyleContext>> swappedStructOwners;
   ElementRestyler::ComputeStyleChangeFor(aFrame, &changeList, aMinChange,
                                          aRestyleTracker, aRestyleHint,
                                          aRestyleHintData,
-                                         contextsToClear, swappedStructOwners);
+                                         swappedStructOwners);
   ProcessRestyledFrames(changeList);
-  ClearCachedInheritedStyleDataOnDescendants(contextsToClear);
 }
 
 void
 RestyleManager::ComputeAndProcessStyleChange(nsStyleContext*        aNewContext,
                                              Element*               aElement,
                                              nsChangeHint           aMinChange,
                                              RestyleTracker&        aRestyleTracker,
                                              nsRestyleHint          aRestyleHint,
@@ -4836,32 +4794,28 @@ RestyleManager::ComputeAndProcessStyleCh
                                     frame->PresContext()->Document());
   nsIContent* parent = aElement->GetParent();
   Element* parentElement =
     parent && parent->IsElement() ? parent->AsElement() : nullptr;
   treeMatchContext.InitAncestors(parentElement);
 
   nsTArray<nsCSSSelector*> selectorsForDescendants;
   nsTArray<nsIContent*> visibleKidsOfHiddenElement;
-  nsTArray<ElementRestyler::ContextToClear> contextsToClear;
 
   // swappedStructOwners needs to be kept alive until after
-  // ProcessRestyledFrames and ClearCachedInheritedStyleDataOnDescendants
-  // calls; see comment in ElementRestyler::Restyle.
+  // ProcessRestyledFrames; see comment in ElementRestyler::Restyle.
   nsTArray<RefPtr<nsStyleContext>> swappedStructOwners;
   nsStyleChangeList changeList;
   ElementRestyler r(frame->PresContext(), aElement, &changeList, aMinChange,
                     aRestyleTracker, selectorsForDescendants, treeMatchContext,
-                    visibleKidsOfHiddenElement, contextsToClear,
-                    swappedStructOwners);
+                    visibleKidsOfHiddenElement, swappedStructOwners);
   r.RestyleChildrenOfDisplayContentsElement(frame, aNewContext, aMinChange,
                                             aRestyleTracker,
                                             aRestyleHint, aRestyleHintData);
   ProcessRestyledFrames(changeList);
-  ClearCachedInheritedStyleDataOnDescendants(contextsToClear);
 }
 
 nsStyleSet*
 RestyleManager::StyleSet() const
 {
   MOZ_ASSERT(mPresContext->StyleSet()->IsGecko(),
              "RestyleManager should only be used with a Gecko-flavored "
              "style backend");
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -587,31 +587,25 @@ private:
  * An ElementRestyler is created for *each* element in a subtree that we
  * recompute styles for.
  */
 class ElementRestyler final
 {
 public:
   typedef mozilla::dom::Element Element;
 
-  struct ContextToClear {
-    RefPtr<nsStyleContext> mStyleContext;
-    uint32_t mStructs;
-  };
-
   // Construct for the root of the subtree that we're restyling.
   ElementRestyler(nsPresContext* aPresContext,
                   nsIFrame* aFrame,
                   nsStyleChangeList* aChangeList,
                   nsChangeHint aHintsHandledByAncestors,
                   RestyleTracker& aRestyleTracker,
                   nsTArray<nsCSSSelector*>& aSelectorsForDescendants,
                   TreeMatchContext& aTreeMatchContext,
                   nsTArray<nsIContent*>& aVisibleKidsOfHiddenElement,
-                  nsTArray<ContextToClear>& aContextsToClear,
                   nsTArray<RefPtr<nsStyleContext>>& aSwappedStructOwners);
 
   // Construct for an element whose parent is being restyled.
   enum ConstructorFlags {
     FOR_OUT_OF_FLOW_CHILD = 1<<0
   };
   ElementRestyler(const ElementRestyler& aParentRestyler,
                   nsIFrame* aFrame,
@@ -632,17 +626,16 @@ public:
   ElementRestyler(nsPresContext* aPresContext,
                   nsIContent* aContent,
                   nsStyleChangeList* aChangeList,
                   nsChangeHint aHintsHandledByAncestors,
                   RestyleTracker& aRestyleTracker,
                   nsTArray<nsCSSSelector*>& aSelectorsForDescendants,
                   TreeMatchContext& aTreeMatchContext,
                   nsTArray<nsIContent*>& aVisibleKidsOfHiddenElement,
-                  nsTArray<ContextToClear>& aContextsToClear,
                   nsTArray<RefPtr<nsStyleContext>>& aSwappedStructOwners);
 
   /**
    * Restyle our frame's element and its subtree.
    *
    * Use eRestyle_Self for the aRestyleHint argument to mean
    * "reresolve our style context but not kids", use eRestyle_Subtree
    * to mean "reresolve our style context and kids", and use
@@ -677,17 +670,16 @@ public:
    * based on the resulting style changes, plus aMinChange applied to aFrame.
    */
   static void ComputeStyleChangeFor(nsIFrame*          aFrame,
                                     nsStyleChangeList* aChangeList,
                                     nsChangeHint       aMinChange,
                                     RestyleTracker&    aRestyleTracker,
                                     nsRestyleHint      aRestyleHint,
                                     const RestyleHintData& aRestyleHintData,
-                                    nsTArray<ContextToClear>& aContextsToClear,
                                     nsTArray<RefPtr<nsStyleContext>>&
                                       aSwappedStructOwners);
 
 #ifdef RESTYLE_LOGGING
   bool ShouldLogRestyle() {
     return RestyleManager::ShouldLogRestyle(mPresContext);
   }
 #endif
@@ -877,20 +869,16 @@ private:
   nsChangeHint mHintsHandled;
   // See nsStyleContext::CalcStyleDifference
   nsChangeHint mParentFrameHintsNotHandledForDescendants;
   nsChangeHint mHintsNotHandledForDescendants;
   RestyleTracker& mRestyleTracker;
   nsTArray<nsCSSSelector*>& mSelectorsForDescendants;
   TreeMatchContext& mTreeMatchContext;
   nsIFrame* mResolvedChild; // child that provides our parent style context
-  // Array of style context subtrees in which we need to clear out cached
-  // structs at the end of the restyle (after change hints have been
-  // processed).
-  nsTArray<ContextToClear>& mContextsToClear;
   // Style contexts that had old structs swapped into it and which should
   // stay alive until the end of the restyle.  (See comment in
   // ElementRestyler::Restyle.)
   nsTArray<RefPtr<nsStyleContext>>& mSwappedStructOwners;
   // Whether this is the root of the restyle.
   bool mIsRootOfRestyle;
 
 #ifdef ACCESSIBILITY