Bug 1302054 - Part 1: Avoid computing style differences when we just want to ensure structs are cached on the new context. r=dbaron
MozReview-Commit-ID: DLhHcCD4GQS
* * *
[mq]: x
MozReview-Commit-ID: 7XOgQYu9kiW
--- a/layout/base/GeckoRestyleManager.cpp
+++ b/layout/base/GeckoRestyleManager.cpp
@@ -944,31 +944,20 @@ GeckoRestyleManager::ReparentStyleContex
// nsTransitionManager::ConsiderInitiatingTransition.
#if 0
if (!copyFromContinuation) {
TryInitiatingTransition(mPresContext, aFrame->GetContent(),
oldContext, &newContext);
}
#endif
- // Make sure to call CalcStyleDifference so that the new context ends
- // up resolving all the structs the old context resolved.
+ // Ensure the new context ends up resolving all the structs the old
+ // context resolved.
if (!copyFromContinuation) {
- uint32_t equalStructs;
- uint32_t samePointerStructs;
- DebugOnly<nsChangeHint> styleChange =
- oldContext->CalcStyleDifference(newContext, nsChangeHint(0),
- &equalStructs,
- &samePointerStructs);
- // The style change is always 0 because we have the same rulenode and
- // CalcStyleDifference optimizes us away. That's OK, though:
- // reparenting should never trigger a frame reconstruct, and whenever
- // it's happening we already plan to reflow and repaint the frames.
- NS_ASSERTION(!(styleChange & nsChangeHint_ReconstructFrame),
- "Our frame tree is likely to be bogus!");
+ newContext->EnsureSameStructsCached(oldContext);
}
aFrame->SetStyleContext(newContext);
nsIFrame::ChildListIterator lists(aFrame);
for (; !lists.IsDone(); lists.Next()) {
for (nsIFrame* child : lists.CurrentList()) {
// only do frames that are in flow
@@ -1010,33 +999,19 @@ GeckoRestyleManager::ReparentStyleContex
(oldExtraContext = aFrame->GetAdditionalStyleContext(contextIndex));
++contextIndex) {
RefPtr<nsStyleContext> newExtraContext;
newExtraContext = StyleSet()->
ReparentStyleContext(oldExtraContext,
newContext, nullptr);
if (newExtraContext) {
if (newExtraContext != oldExtraContext) {
- // Make sure to call CalcStyleDifference so that the new
- // context ends up resolving all the structs the old context
- // resolved.
- uint32_t equalStructs;
- uint32_t samePointerStructs;
- DebugOnly<nsChangeHint> styleChange =
- oldExtraContext->CalcStyleDifference(newExtraContext,
- nsChangeHint(0),
- &equalStructs,
- &samePointerStructs);
- // The style change is always 0 because we have the same
- // rulenode and CalcStyleDifference optimizes us away. That's
- // OK, though: reparenting should never trigger a frame
- // reconstruct, and whenever it's happening we already plan to
- // reflow and repaint the frames.
- NS_ASSERTION(!(styleChange & nsChangeHint_ReconstructFrame),
- "Our frame tree is likely to be bogus!");
+ // Ensure the new context ends up resolving all the structs the old
+ // context resolved.
+ newContext->EnsureSameStructsCached(oldContext);
}
aFrame->SetAdditionalStyleContext(contextIndex, newExtraContext);
}
}
#ifdef DEBUG
DebugVerifyStyleTree(aFrame);
#endif
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -215,17 +215,17 @@ ServoRestyleManager::ProcessPostTraversa
RefPtr<nsStyleContext> newContext = nullptr;
if (recreateContext) {
MOZ_ASSERT(styleFrame || displayContentsNode);
newContext =
aStyleSet->GetContext(computedValues.forget(), aParentContext, nullptr,
CSSPseudoElementType::NotPseudo, aElement);
- newContext->EnsureStructsForServo(oldStyleContext);
+ newContext->EnsureSameStructsCached(oldStyleContext);
// XXX This could not always work as expected: there are kinds of content
// with the first split and the last sharing style, but others not. We
// should handle those properly.
// XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
// right, but not other cases where we happen to have different styles on
// different continuations... (e.g. first-line).
for (nsIFrame* f = styleFrame; f;
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -174,46 +174,16 @@ nsStyleContext::FinishConstruction(bool
}
#define eStyleStruct_LastItem (nsStyleStructID_Length - 1)
NS_ASSERTION(NS_STYLE_INHERIT_MASK & NS_STYLE_INHERIT_BIT(LastItem),
"NS_STYLE_INHERIT_MASK must be bigger, and other bits shifted");
#undef eStyleStruct_LastItem
}
-void
-nsStyleContext::EnsureStructsForServo(const nsStyleContext* aOldContext)
-{
- MOZ_ASSERT(aOldContext);
- MOZ_ASSERT(mSource.IsServoComputedValues() &&
- aOldContext->mSource.IsServoComputedValues());
-
- // NOTE(emilio): We could do better here. We only call Style##name_() because
- // we need to run FinishStyle, but otherwise this is only a bitwise or.
- //
- // We could reduce the FFI traffic we do only doing it for structs that have
- // non-trivial FinishStyle.
-#define STYLE_STRUCT(name_, checkdata_cb) \
- if (aOldContext->mBits & NS_STYLE_INHERIT_BIT(name_)) { \
- mozilla::Unused << Style##name_(); \
- }
-
-#include "nsStyleStructList.h"
-
-#undef STYLE_STRUCT
-
-#ifdef DEBUG
- auto oldMask = aOldContext->mBits & NS_STYLE_INHERIT_MASK;
- auto newMask = mBits & NS_STYLE_INHERIT_MASK;
- MOZ_ASSERT((oldMask & newMask) == oldMask,
- "Should have at least as many structs computed as the "
- "old context!");
-#endif
-}
-
nsStyleContext::~nsStyleContext()
{
NS_ASSERTION((nullptr == mChild) && (nullptr == mEmptyChild), "destructing context with children");
MOZ_ASSERT_IF(mSource.IsServoComputedValues(), !mCachedResetData);
#ifdef DEBUG
if (mSource.IsServoComputedValues()) {
MOZ_ASSERT(!mCachedResetData,
@@ -1326,16 +1296,42 @@ nsStyleContext::CalcStyleDifference(cons
{
FakeStyleContext newContext(aNewComputedValues);
return CalcStyleDifferenceInternal(&newContext,
aParentHintsNotHandledForDescendants,
aEqualStructs,
aSamePointerStructs);
}
+void
+nsStyleContext::EnsureSameStructsCached(nsStyleContext* aOldContext)
+{
+ // NOTE(emilio): We could do better here for stylo, where we only call
+ // Style##name_() because we need to run FinishStyle, but otherwise this
+ // is only a bitwise or.
+ //
+ // We could reduce the FFI traffic we do only doing it for structs that have
+ // non-trivial FinishStyle.
+
+#define STYLE_STRUCT(name_, checkdata_cb_) \
+ if (aOldContext->PeekStyle##name_()) { \
+ Style##name_(); \
+ }
+#include "nsStyleStructList.h"
+#undef STYLE_STRUCT
+
+#ifdef DEBUG
+ auto oldMask = aOldContext->mBits & NS_STYLE_INHERIT_MASK;
+ auto newMask = mBits & NS_STYLE_INHERIT_MASK;
+ MOZ_ASSERT((oldMask & newMask) == oldMask,
+ "Should have at least as many structs computed as the "
+ "old context!");
+#endif
+}
+
#ifdef DEBUG
void nsStyleContext::List(FILE* out, int32_t aIndent, bool aListDescendants)
{
nsAutoCString str;
// Indent
int32_t ix;
for (ix = aIndent; --ix >= 0; ) {
str.AppendLiteral(" ");
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -382,24 +382,16 @@ public:
#define STYLE_STRUCT(name_, checkdata_cb_) \
const nsStyle##name_ * PeekStyle##name_() { \
return DoGetStyle##name_<false>(); \
}
#include "nsStyleStructList.h"
#undef STYLE_STRUCT
/**
- * Ensures that this context's computed struct list is at least the old
- * context's.
- *
- * aOldContext must not be null.
- */
- void EnsureStructsForServo(const nsStyleContext* aOldContext);
-
- /**
* Compute the style changes needed during restyling when this style
* context is being replaced by aNewContext. (This is nonsymmetric since
* we optimize by skipping comparison for styles that have never been
* requested.)
*
* This method returns a change hint (see nsChangeHint.h). All change
* hints apply to the frame and its later continuations or ib-split
* siblings. Most (all of those except the "NotHandledForDescendants"
@@ -431,16 +423,22 @@ private:
template<class StyleContextLike>
nsChangeHint CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
nsChangeHint aParentHintsNotHandledForDescendants,
uint32_t* aEqualStructs,
uint32_t* aSamePointerStructs);
public:
/**
+ * Ensures the same structs are cached on this style context as would be
+ * done if we called aOther->CalcDifference(this).
+ */
+ void EnsureSameStructsCached(nsStyleContext* aOldContext);
+
+ /**
* Get a color that depends on link-visitedness using this and
* this->GetStyleIfVisited().
*
* @param aField A pointer to a member variable in a style struct.
* The member variable and its style struct must have
* been listed in nsCSSVisitedDependentPropList.h.
*/
template<typename T, typename S>