Bug 898329 patch 4: Avoid using newContext outside of what will be in RestyleManager::RestyleSelf. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Tue, 30 Jul 2013 17:36:11 -0700
changeset 152949 b1f2ea65f84646f92ec7da356defd9a26052acc2
parent 152948 c500d0a58eac561cafb50697a544b8b3c46228f3
child 152950 54617e0ac4537af0438cee5f90b308f3bc49769a
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs898329
milestone25.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 898329 patch 4: Avoid using newContext outside of what will be in RestyleManager::RestyleSelf. r=heycam This replaces newContext with mFrame->StyleContext(), which is a valid replacement since all of the replacements are inside a test that we don't have a framechange hint. If we have a framechange hint, then mFrame still has its old style context.
layout/base/RestyleManager.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -2340,16 +2340,20 @@ ElementRestyler::Restyle(nsRestyleHint a
                                      GetDocElementContainingBlock();
       undisplayedParent = nullptr;
     } else {
       checkUndisplayed = !!mFrame->GetContent();
       undisplayedParent = mFrame->GetContent();
     }
     if (checkUndisplayed &&
         // No need to do this if we're planning to reframe already.
+        // 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.
         !(mHintsHandled & nsChangeHint_ReconstructFrame)) {
       UndisplayedNode* undisplayed =
         frameConstructor->GetAllUndisplayedContentIn(undisplayedParent);
       for (TreeMatchContext::AutoAncestorPusher
              pushAncestor(undisplayed, mTreeMatchContext,
                           undisplayedParent ? undisplayedParent->AsElement()
                                             : nullptr);
            undisplayed; undisplayed = undisplayed->mNext) {
@@ -2377,21 +2381,22 @@ ElementRestyler::Restyle(nsRestyleHint a
                                            &undisplayedRestyleData)) {
           thisChildHint =
             nsRestyleHint(thisChildHint | undisplayedRestyleData.mRestyleHint);
         }
         nsRefPtr<nsStyleContext> undisplayedContext;
         if (thisChildHint) {
           undisplayedContext =
             styleSet->ResolveStyleFor(undisplayed->mContent->AsElement(),
-                                      newContext,
+                                      mFrame->StyleContext(),
                                       mTreeMatchContext);
         } else {
           undisplayedContext =
-            styleSet->ReparentStyleContext(undisplayed->mStyle, newContext,
+            styleSet->ReparentStyleContext(undisplayed->mStyle,
+                                           mFrame->StyleContext(),
                                            undisplayed->mContent->AsElement());
         }
         if (undisplayedContext) {
           const nsStyleDisplay* display = undisplayedContext->StyleDisplay();
           if (display->mDisplay != NS_STYLE_DISPLAY_NONE) {
             NS_ASSERTION(undisplayed->mContent,
                          "Must have undisplayed content");
             mChangeList->AppendChange(nullptr, undisplayed->mContent,
@@ -2404,64 +2409,69 @@ ElementRestyler::Restyle(nsRestyleHint a
           }
         }
       }
     }
 
     // Check whether we might need to create a new ::before frame.
     // There's no need to do this if we're planning to reframe already
     // or if we're not forcing restyles on kids.
+    // 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) &&
         childRestyleHint) {
       // Make sure not to do this for pseudo-frames or frames that
       // can't have generated content.
       if (!pseudoTag &&
           ((mFrame->GetStateBits() & NS_FRAME_MAY_HAVE_GENERATED_CONTENT) ||
            // Our content insertion frame might have gotten flagged
            (mFrame->GetContentInsertionFrame()->GetStateBits() &
             NS_FRAME_MAY_HAVE_GENERATED_CONTENT))) {
         // Check for a new :before pseudo and an existing :before
         // frame, but only if the frame is the first continuation.
         nsIFrame* prevContinuation = mFrame->GetPrevContinuation();
         if (!prevContinuation) {
           // Checking for a :before frame is cheaper than getting the
           // :before style context.
           if (!nsLayoutUtils::GetBeforeFrame(mFrame) &&
-              nsLayoutUtils::HasPseudoStyle(mFrame->GetContent(), newContext,
+              nsLayoutUtils::HasPseudoStyle(mFrame->GetContent(),
+                                            mFrame->StyleContext(),
                                             nsCSSPseudoElements::ePseudo_before,
                                             mPresContext)) {
             // Have to create the new :before frame
             NS_UpdateHint(mHintsHandled, nsChangeHint_ReconstructFrame);
             mChangeList->AppendChange(mFrame, mContent,
                                       nsChangeHint_ReconstructFrame);
           }
         }
       }
     }
 
     // Check whether we might need to create a new ::after frame.
-    // There's no need to do this if we're planning to reframe already
-    // or if we're not forcing restyles on kids.
+    // See comments above regarding :before.
     if (!(mHintsHandled & nsChangeHint_ReconstructFrame) &&
         childRestyleHint) {
       // Make sure not to do this for pseudo-frames or frames that
       // can't have generated content.
       if (!pseudoTag &&
           ((mFrame->GetStateBits() & NS_FRAME_MAY_HAVE_GENERATED_CONTENT) ||
            // Our content insertion frame might have gotten flagged
            (mFrame->GetContentInsertionFrame()->GetStateBits() &
             NS_FRAME_MAY_HAVE_GENERATED_CONTENT))) {
         // Check for new :after content, but only if the frame is the
         // last continuation.
         nsIFrame* nextContinuation = mFrame->GetNextContinuation();
 
         if (!nextContinuation) {
           // Getting the :after frame is more expensive than getting the pseudo
           // context, so get the pseudo context first.
-          if (nsLayoutUtils::HasPseudoStyle(mFrame->GetContent(), newContext,
+          if (nsLayoutUtils::HasPseudoStyle(mFrame->GetContent(),
+                                            mFrame->StyleContext(),
                                             nsCSSPseudoElements::ePseudo_after,
                                             mPresContext) &&
               !nsLayoutUtils::GetAfterFrame(mFrame)) {
             // have to create the new :after frame
             NS_UpdateHint(mHintsHandled, nsChangeHint_ReconstructFrame);
             mChangeList->AppendChange(mFrame, mContent,
                                       nsChangeHint_ReconstructFrame);
           }
@@ -2469,27 +2479,31 @@ ElementRestyler::Restyle(nsRestyleHint a
       }
     }
 
     // There is no need to waste time crawling into a frame's children
     // on a frame change.  The act of reconstructing frames will force
     // new style contexts to be resolved on all of this frame's
     // descendants anyway, so we want to avoid wasting time processing
     // style contexts that we're just going to throw away anyway. - dwh
+    // It's also important to check mHintsHandled since reresolving the
+    // kids would use mFrame->StyleContext(), which is out of date if
+    // mHintsHandled has a ReconstructFrame hint; doing this could trigger
+    // assertions about mismatched rule trees.
     if (!(mHintsHandled & nsChangeHint_ReconstructFrame)) {
 
 #ifdef ACCESSIBILITY
       // Notify a11y for primary frame only if it's a root frame of visibility
       // changes or its parent frame was hidden while it stays visible and
       // it is not inside a {ib} split or is the first frame of {ib} split.
       if (nsIPresShell::IsAccessibilityActive() &&
           !mFrame->GetPrevContinuation() &&
           !nsLayoutUtils::FrameIsNonFirstInIBSplit(mFrame)) {
         if (mDesiredA11yNotifications == eSendAllNotifications) {
-          bool isFrameVisible = newContext->StyleVisibility()->IsVisible();
+          bool isFrameVisible = mFrame->StyleVisibility()->IsVisible();
           if (isFrameVisible != mWasFrameVisible) {
             if (isFrameVisible) {
               // Notify a11y the element (perhaps with its children) was shown.
               // We don't fall into this case if this element gets or stays shown
               // while its parent becomes hidden.
               mKidsDesiredA11yNotifications = eSkipNotifications;
               mOurA11yNotification = eNotifyShown;
             } else {
@@ -2498,17 +2512,17 @@ ElementRestyler::Restyle(nsRestyleHint a
               // visible children then we should notify a11y about that as if
               // they were inserted into tree. Notify a11y this element was
               // hidden.
               mKidsDesiredA11yNotifications = eNotifyIfShown;
               mOurA11yNotification = eNotifyHidden;
             }
           }
         } else if (mDesiredA11yNotifications == eNotifyIfShown &&
-                   newContext->StyleVisibility()->IsVisible()) {
+                   mFrame->StyleVisibility()->IsVisible()) {
           // Notify a11y that element stayed visible while its parent was
           // hidden.
           mVisibleKidsOfHiddenElement.AppendElement(mFrame->GetContent());
           mKidsDesiredA11yNotifications = eSkipNotifications;
         }
       }
 #endif