Bug 1407246 - Split out Variables struct difference calculation. r?emilio draft
authorCameron McCormack <cam@mcc.id.au>
Thu, 12 Oct 2017 09:12:30 +0800
changeset 678940 e926cb078f53e43084257052fef7e583a5f743a1
parent 678939 3359ba885adac92e8a0ed41ee53adf703b495c6e
child 735469 d241db2ef8ae9b4cc0abe085b8e327e921508d04
push id84074
push userbmo:cam@mcc.id.au
push dateThu, 12 Oct 2017 02:42:09 +0000
reviewersemilio
bugs1407246
milestone58.0a1
Bug 1407246 - Split out Variables struct difference calculation. r?emilio MozReview-Commit-ID: CtWtG3zkD1D
layout/generic/nsFrame.cpp
layout/style/ServoBindings.cpp
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
layout/tables/nsTableFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -10227,17 +10227,18 @@ nsIFrame::UpdateStyleOfOwnedChildFrame(
   //
   // 2) Content can change stylesheets that change the styles of pseudos, and
   //    extensions can add/remove stylesheets that change the styles of
   //    anonymous boxes directly.
   uint32_t equalStructs, samePointerStructs; // Not used, actually.
   nsChangeHint childHint = aChildFrame->StyleContext()->CalcStyleDifference(
     aNewStyleContext,
     &equalStructs,
-    &samePointerStructs);
+    &samePointerStructs,
+    /* aIgnoreVariables = */ true);
 
   // CalcStyleDifference will handle caching structs on the new style context,
   // but only if we're not on a style worker thread.
   MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal(),
              "if we can get in here from style worker threads, then we need "
              "a ResolveSameStructsAs call to ensure structs are cached on "
              "aNewStyleContext");
 
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -396,17 +396,17 @@ Gecko_CalcStyleDifference(ServoStyleCont
 
   uint32_t equalStructs;
   uint32_t samePointerStructs;  // unused
   nsChangeHint result = const_cast<ServoStyleContext*>(aOldStyle)->
     CalcStyleDifference(
       const_cast<ServoStyleContext*>(aNewStyle),
       &equalStructs,
       &samePointerStructs,
-      NS_STYLE_INHERIT_MASK);
+      /* aIgnoreVariables = */ true);
 
   *aAnyStyleChanged = equalStructs != NS_STYLE_INHERIT_MASK;
 
   const uint32_t kInheritStructsMask =
     NS_STYLE_INHERIT_MASK & ~NS_STYLE_RESET_STRUCT_MASK;
 
   *aOnlyResetStructsChanged =
     (equalStructs & kInheritStructsMask) == kInheritStructsMask;
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -101,29 +101,25 @@ nsStyleContext::nsStyleContext(nsAtom* a
                 "Number of items in dependency table doesn't match IDs");
 #endif
 }
 
 nsChangeHint
 nsStyleContext::CalcStyleDifference(nsStyleContext* aNewContext,
                                     uint32_t* aEqualStructs,
                                     uint32_t* aSamePointerStructs,
-                                    uint32_t aRelevantStructs)
+                                    bool aIgnoreVariables)
 {
   AUTO_PROFILER_LABEL("nsStyleContext::CalcStyleDifference", CSS);
 
   static_assert(nsStyleStructID_Length <= 32,
                 "aEqualStructs is not big enough");
 
-  MOZ_ASSERT(aRelevantStructs == kAllResolvedStructs || IsServo(),
-             "aRelevantStructs must be kAllResolvedStructs for Gecko contexts");
-
-  if (aRelevantStructs == kAllResolvedStructs) {
-    aRelevantStructs = mBits & NS_STYLE_INHERIT_MASK;
-  }
+  MOZ_ASSERT(!aIgnoreVariables || IsServo(),
+             "aIgnoreVariables must be false for Gecko contexts");
 
   *aEqualStructs = 0;
   *aSamePointerStructs = 0;
 
   nsChangeHint hint = nsChangeHint(0);
   NS_ENSURE_TRUE(aNewContext, hint);
   // We must always ensure that we populate the structs on the new style
   // context that are filled in on the old context, so that if we get
@@ -153,17 +149,18 @@ nsStyleContext::CalcStyleDifference(nsSt
         *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
       } else if (thisVariables->mVariables == otherVariables->mVariables) {
         *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
       }
     } else {
       *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
     }
   } else {
-    if (Servo_ComputedValues_EqualCustomProperties(
+    if (aIgnoreVariables ||
+        Servo_ComputedValues_EqualCustomProperties(
           AsServo()->ComputedData(),
           aNewContext->ComputedData())) {
       *aEqualStructs |= NS_STYLE_INHERIT_BIT(Variables);
     }
   }
 
   DebugOnly<int> styleStructCount = 1;  // count Variables already
 
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -236,49 +236,38 @@ public:
    *
    * Perhaps this shouldn't be a public nsStyleContext API.
    */
   #define STYLE_STRUCT(name_, checkdata_cb_)  \
     inline const nsStyle##name_ * PeekStyle##name_();
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT
 
-  // Value that can be passed as CalcStyleDifference's aRelevantStructs
-  // argument to indicate that all structs that are currently resolved on the
-  // old style context should be compared.  This is only relevant for
-  // ServoStyleContexts.
-  enum { kAllResolvedStructs = 0xffffffff };
-  static_assert(kAllResolvedStructs != NS_STYLE_INHERIT_MASK,
-                "uint32_t not big enough for special kAllResolvedStructs value");
-
   /**
    * 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"
    * hints) also apply to all descendants.
    *
    * aEqualStructs must not be null.  Into it will be stored a bitfield
    * representing which structs were compared to be non-equal.
    *
-   * aRelevantStructs must be kAllResolvedStructs for GeckoStyleContexts.
-   * For ServoStyleContexts, it controls which structs will be compared.
-   * This is needed because in some cases, we can't rely on mBits in the
-   * old style context to accurately reflect which are the relevant
-   * structs to be compared.
+   * aIgnoreVariables indicates whether to skip comparing the Variables
+   * struct.  This must only be true for Servo style contexts.  When
+   * true, the Variables bit in aEqualStructs will be set.
    */
   nsChangeHint CalcStyleDifference(nsStyleContext* aNewContext,
                                    uint32_t* aEqualStructs,
                                    uint32_t* aSamePointerStructs,
-                                   uint32_t aRelevantStructs =
-                                     kAllResolvedStructs);
+				   bool aIgnoreVariables = false);
 
 public:
   /**
    * 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
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -8082,17 +8082,18 @@ nsTableFrame::UpdateStyleOfOwnedAnonBoxe
   //
   // NOTE(emilio): We can't use the ChangesHandledFor optimization (and we
   // assert against that), because the table wrapper is up in the frame tree
   // compared to the owner frame.
   uint32_t equalStructs, samePointerStructs; // Not used, actually.
   nsChangeHint wrapperHint = aWrapperFrame->StyleContext()->CalcStyleDifference(
     newContext,
     &equalStructs,
-    &samePointerStructs);
+    &samePointerStructs,
+    /* aIgnoreVariables = */ true);
 
   // CalcStyleDifference will handle caching structs on the new style context,
   // but only if we're not on a style worker thread.
   MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal(),
              "if we can get in here from style worker threads, then we need "
              "a ResolveSameStructsAs call to ensure structs are cached on "
              "aNewStyleContext");