Bug 1188721 - Part 7: Stop swapping structs between style contexts when restyling. r?dbaron
MozReview-Commit-ID: CySThmmVyVK
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -3244,34 +3244,30 @@ ElementRestyler::Restyle(nsRestyleHint a
// eRestyle_ForceDescendants flag down too.
nsRestyleHint childRestyleHint =
nsRestyleHint(aRestyleHint & (eRestyle_SomeDescendants |
eRestyle_Subtree |
eRestyle_ForceDescendants));
RefPtr<nsStyleContext> oldContext = mFrame->StyleContext();
- nsTArray<SwapInstruction> swaps;
-
// TEMPORARY (until bug 918064): Call RestyleSelf for each
// continuation or block-in-inline sibling.
// We must make a single decision on how to process this frame and
// its descendants, yet RestyleSelf might return different RestyleResult
// values for the different same-style continuations. |result| is our
// overall decision.
RestyleResult result = RestyleResult(0);
- uint32_t swappedStructs = 0;
nsRestyleHint thisRestyleHint = aRestyleHint;
bool haveMoreContinuations = false;
for (nsIFrame* f = mFrame; f; ) {
- RestyleResult thisResult =
- RestyleSelf(f, thisRestyleHint, &swappedStructs, swaps);
+ RestyleResult thisResult = RestyleSelf(f, thisRestyleHint);
if (thisResult != eRestyleResult_Stop) {
// Calls to RestyleSelf for later same-style continuations must not
// return eRestyleResult_Stop, so pass eRestyle_Force in to them.
thisRestyleHint = nsRestyleHint(thisRestyleHint | eRestyle_Force);
if (result == eRestyleResult_Stop) {
// We received eRestyleResult_Stop for earlier same-style
@@ -3357,19 +3353,16 @@ ElementRestyler::Restyle(nsRestyleHint a
return;
}
if (result == eRestyleResult_StopWithStyleChange &&
!(mHintsHandled & nsChangeHint_ReconstructFrame)) {
MOZ_ASSERT(mFrame->StyleContext() != oldContext,
"eRestyleResult_StopWithStyleChange should only be returned "
"if we got a new style context or we will reconstruct");
- MOZ_ASSERT(swappedStructs == 0,
- "should have ensured we didn't swap structs when "
- "returning eRestyleResult_StopWithStyleChange");
// We need to ensure that all of the frames that inherit their style
// from oldContext are able to be moved across to newContext.
// MoveStyleContextsForChildren will check for certain conditions
// to ensure it is safe to move all of the relevant child style
// contexts to newContext. If these conditions fail, it will
// return false, and we'll have to continue restyling.
const bool canStop = MoveStyleContextsForChildren(oldContext);
@@ -3383,35 +3376,19 @@ ElementRestyler::Restyle(nsRestyleHint a
}
mRestyleTracker.AddRestyleRootsIfAwaitingRestyle(descendants);
if (aRestyleHint & eRestyle_SomeDescendants) {
ConditionallyRestyleChildren();
}
return;
}
-
- // Turns out we couldn't stop restyling here. Process the struct
- // swaps that RestyleSelf would've done had we not returned
- // eRestyleResult_StopWithStyleChange.
- for (SwapInstruction& swap : swaps) {
- LOG_RESTYLE("swapping style structs between %p and %p",
- swap.mOldContext.get(), swap.mNewContext.get());
- swap.mOldContext->SwapStyleData(swap.mNewContext, swap.mStructsToSwap);
- swappedStructs |= swap.mStructsToSwap;
- }
- swaps.Clear();
- }
-
- if (!swappedStructs) {
- // If we swapped any structs from the old context, then we need to keep
- // it alive until after the RestyleChildren call so that we can fix up
- // its descendants' cached structs.
- oldContext = nullptr;
- }
+ }
+
+ oldContext = nullptr;
if (result == eRestyleResult_ContinueAndForceDescendants) {
childRestyleHint =
nsRestyleHint(childRestyleHint | eRestyle_ForceDescendants);
}
// No need to do this if we're planning to reframe already.
// It's also important to check mHintsHandled since we use
@@ -3437,17 +3414,17 @@ ElementRestyler::Restyle(nsRestyleHint a
//
// Also, we don't want this style context to get any more uses by being
// returned from nsStyleContext::FindChildWithRules, so we add the
// NS_STYLE_INELIGIBLE_FOR_SHARING bit to it.
oldContext->SetIneligibleForSharing();
ContextToClear* toClear = mContextsToClear.AppendElement();
toClear->mStyleContext = Move(oldContext);
- toClear->mStructs = swappedStructs;
+ 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
@@ -3764,20 +3741,17 @@ CommonInheritedStyleData(nsRuleNode* aRu
n1 = n1->GetParent();
n2 = n2->GetParent();
}
return true;
}
ElementRestyler::RestyleResult
-ElementRestyler::RestyleSelf(nsIFrame* aSelf,
- nsRestyleHint aRestyleHint,
- uint32_t* aSwappedStructs,
- nsTArray<SwapInstruction>& aSwaps)
+ElementRestyler::RestyleSelf(nsIFrame* aSelf, nsRestyleHint aRestyleHint)
{
MOZ_ASSERT(!(aRestyleHint & eRestyle_LaterSiblings),
"eRestyle_LaterSiblings must not be part of aRestyleHint");
// XXXldb get new context from prev-in-flow if possible, to avoid
// duplication. (Or should we just let |GetContext| handle that?)
// Getting the hint would be nice too, but that's harder.
@@ -4138,54 +4112,16 @@ ElementRestyler::RestyleSelf(nsIFrame* a
// which is important to maintain various invariants about
// frame types matching their style contexts.
// Note that this check even makes sense if we didn't call
// CaptureChange because of copyFromContinuation being true,
// since we'll have copied the existing context from the
// previous continuation, so newContext == oldContext.
if (result != eRestyleResult_Stop) {
- if (copyFromContinuation) {
- LOG_RESTYLE("not swapping style structs, since we copied from a "
- "continuation");
- } else if (oldContext->IsShared() && newContext->IsShared()) {
- LOG_RESTYLE("not swapping style structs, since both old and contexts "
- "are shared");
- } else if (oldContext->IsShared()) {
- LOG_RESTYLE("not swapping style structs, since the old context is "
- "shared");
- } else if (newContext->IsShared()) {
- LOG_RESTYLE("not swapping style structs, since the new context is "
- "shared");
- } else {
- if (result == eRestyleResult_StopWithStyleChange) {
- LOG_RESTYLE("recording a style struct swap between %p and %p to "
- "do if eRestyleResult_StopWithStyleChange fails",
- oldContext.get(), newContext.get());
- SwapInstruction* swap = aSwaps.AppendElement();
- swap->mOldContext = oldContext;
- swap->mNewContext = newContext;
- swap->mStructsToSwap = equalStructs;
- } else {
- LOG_RESTYLE("swapping style structs between %p and %p",
- oldContext.get(), newContext.get());
- oldContext->SwapStyleData(newContext, equalStructs);
- *aSwappedStructs |= equalStructs;
- }
-#ifdef RESTYLE_LOGGING
- uint32_t structs = RestyleManager::StructsToLog() & equalStructs;
- if (structs) {
- LOG_RESTYLE_INDENT();
- LOG_RESTYLE("old style context now has: %s",
- oldContext->GetCachedStyleDataAsString(structs).get());
- LOG_RESTYLE("new style context now has: %s",
- newContext->GetCachedStyleDataAsString(structs).get());
- }
-#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
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -711,30 +711,20 @@ private:
// continue restyling children
eRestyleResult_Continue,
// continue restyling children with eRestyle_ForceDescendants set
eRestyleResult_ContinueAndForceDescendants
};
- struct SwapInstruction
- {
- RefPtr<nsStyleContext> mOldContext;
- RefPtr<nsStyleContext> mNewContext;
- uint32_t mStructsToSwap;
- };
-
/**
* First half of Restyle().
*/
- RestyleResult RestyleSelf(nsIFrame* aSelf,
- nsRestyleHint aRestyleHint,
- uint32_t* aSwappedStructs,
- nsTArray<SwapInstruction>& aSwaps);
+ RestyleResult RestyleSelf(nsIFrame* aSelf, nsRestyleHint aRestyleHint);
/**
* Restyle the children of this frame (and, in turn, their children).
*
* Second half of Restyle().
*/
void RestyleChildren(nsRestyleHint aChildRestyleHint);