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

layout/base/GeckoRestyleManager.cpp

@@ -967,21 +967,10 @@ GeckoRestyleManager::ReparentStyleContext(nsIFrame* aFrame)
       }
 #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);
@@ -1033,23 +1022,9 @@ GeckoRestyleManager::ReparentStyleContext(nsIFrame* aFrame)
                                                  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);

layout/base/ServoRestyleManager.cpp

@@ -213,7 +213,7 @@ ServoRestyleManager::ProcessPostTraversal(Element* aElement,
       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

layout/style/nsStyleContext.cpp

@@ -179,36 +179,6 @@ nsStyleContext::FinishConstruction(bool aSkipParentDisplayBasedStyleFixup)
   #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");
@@ -1331,6 +1301,34 @@ nsStyleContext::CalcStyleDifference(const ServoComputedValues* aNewComputedValue
                                      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
+  if (mSource.IsServoComputedValues()) {
+    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)
 {

layout/style/nsStyleContext.h

@@ -386,14 +386,6 @@ public:
   #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
@@ -435,6 +427,12 @@ private:
                                            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().