Bug 1380133 - Part 5: Call CalcStyleDifference with ServoStyleContexts instead of a FakeStyleContext wrapping a ServoComputedValues. r=emilio
☠☠ backed out by 0a99d8945fd9 ☠ ☠
authorCameron McCormack <cam@mcc.id.au>
Wed, 19 Jul 2017 17:50:35 +0800
changeset 418561 9a84b6988af9f5ed854927401acfae541de80d3f
parent 418560 cc720d72d02492f272cccdcda5ca969814120b52
child 418562 cf561cad85f14e30b4f073356d9291ff74dd6d99
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1380133
milestone56.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 1380133 - Part 5: Call CalcStyleDifference with ServoStyleContexts instead of a FakeStyleContext wrapping a ServoComputedValues. r=emilio MozReview-Commit-ID: 6JhMas1EiM7
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -393,28 +393,35 @@ Gecko_GetStyleContext(RawGeckoElementBor
 
 CSSPseudoElementType
 Gecko_GetImplementedPseudo(RawGeckoElementBorrowed aElement)
 {
   return aElement->GetPseudoElementType();
 }
 
 nsChangeHint
-Gecko_CalcStyleDifference(nsStyleContext* aOldStyleFromFrame,
+Gecko_CalcStyleDifference(const ServoStyleContext* aOldStyle,
                           const ServoStyleContext* aNewStyle,
+                          uint64_t aOldStyleBits,
                           bool* aAnyStyleChanged)
 {
-  MOZ_ASSERT(aOldStyleFromFrame);
+  MOZ_ASSERT(aOldStyle);
   MOZ_ASSERT(aNewStyle);
 
-  uint32_t equalStructs, samePointerStructs;
-  nsChangeHint result =
-    aOldStyleFromFrame->CalcStyleDifference(aNewStyle->ComputedValues(),
-                                            &equalStructs,
-                                            &samePointerStructs);
+  uint32_t relevantStructs = aOldStyleBits & NS_STYLE_INHERIT_MASK;
+
+  uint32_t equalStructs;
+  uint32_t samePointerStructs;  // unused
+  nsChangeHint result = const_cast<ServoStyleContext*>(aOldStyle)->
+    CalcStyleDifference(
+      const_cast<ServoStyleContext*>(aNewStyle),
+      &equalStructs,
+      &samePointerStructs,
+      relevantStructs);
+
   *aAnyStyleChanged = equalStructs != NS_STYLE_INHERIT_MASK;
   return result;
 }
 
 nsChangeHint
 Gecko_HintsHandledForDescendants(nsChangeHint aHint)
 {
   return aHint & ~NS_HintsNotHandledForDescendantsIn(aHint);
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -373,18 +373,19 @@ void Gecko_UnsetNodeFlags(RawGeckoNodeBo
 void Gecko_SetOwnerDocumentNeedsStyleFlush(RawGeckoElementBorrowed element);
 
 // Incremental restyle.
 // Also, we might want a ComputedValues to ComputedValues API for animations?
 // Not if we do them in Gecko...
 nsStyleContext* Gecko_GetStyleContext(RawGeckoElementBorrowed element,
                                       nsIAtom* aPseudoTagOrNull);
 mozilla::CSSPseudoElementType Gecko_GetImplementedPseudo(RawGeckoElementBorrowed element);
-nsChangeHint Gecko_CalcStyleDifference(nsStyleContext* old_style_from_frame,
+nsChangeHint Gecko_CalcStyleDifference(const mozilla::ServoStyleContext* old_style,
                                        const mozilla::ServoStyleContext* new_style,
+                                       uint64_t old_style_bits,
                                        bool* any_style_changed);
 nsChangeHint Gecko_HintsHandledForDescendants(nsChangeHint aHint);
 
 // Get an element snapshot for a given element from the table.
 const ServoElementSnapshot*
 Gecko_GetElementSnapshot(const mozilla::ServoElementSnapshotTable* table,
                          RawGeckoElementBorrowed element);
 
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -184,23 +184,31 @@ nsStyleContext::MoveTo(nsStyleContext* a
     styleIfVisited->mParent->AddChild(styleIfVisited);
   }
 }
 
 template<class StyleContextLike>
 nsChangeHint
 nsStyleContext::CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
                                             uint32_t* aEqualStructs,
-                                            uint32_t* aSamePointerStructs)
+                                            uint32_t* aSamePointerStructs,
+                                            uint32_t aRelevantStructs)
 {
   AUTO_PROFILER_LABEL("nsStyleContext::CalcStyleDifferenceInternal", 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;
+  }
+
   *aEqualStructs = 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
   // two style changes in succession, the second of which causes a real
   // style change, the PeekStyleData doesn't return null (implying that
@@ -237,20 +245,31 @@ nsStyleContext::CalcStyleDifferenceInter
 
   // Servo's optimization to stop the cascade when there are no style changes
   // that children need to be recascade for relies on comparing all of the
   // structs, not just those that are returned from PeekStyleData, although
   // if PeekStyleData does return null we still don't want to accumulate
   // any change hints for those structs.
   bool checkUnrequestedServoStructs = IsServo();
 
+  // For Gecko structs, we just defer to PeekStyleXXX.  But for Servo structs,
+  // we need to use the aRelevantStructs bitfield passed in to determine
+  // whether to return a struct or not, since this->mBits might not yet
+  // be correct (due to not calling ResolveSameStructsAs on it yet).
+#define PEEK(struct_)                                                         \
+  (IsGecko()                                                                  \
+   ? PeekStyle##struct_()                                                     \
+   : ((aRelevantStructs & NS_STYLE_INHERIT_BIT(struct_))                      \
+      ? AsServo()->ComputedValues()->GetStyle##struct_()                      \
+      : nullptr))
+
 #define EXPAND(...) __VA_ARGS__
 #define DO_STRUCT_DIFFERENCE_WITH_ARGS(struct_, extra_args_)                  \
   PR_BEGIN_MACRO                                                              \
-    const nsStyle##struct_* this##struct_ = PeekStyle##struct_();             \
+    const nsStyle##struct_* this##struct_ = PEEK(struct_);                    \
     bool unrequestedStruct;                                                   \
     if (this##struct_) {                                                      \
       unrequestedStruct = false;                                              \
       structsFound |= NS_STYLE_INHERIT_BIT(struct_);                          \
     } else if (checkUnrequestedServoStructs) {                                \
       this##struct_ =                                                         \
         AsServo()->ComputedValues()->GetStyle##struct_();                     \
       unrequestedStruct = true;                                               \
@@ -291,20 +310,20 @@ nsStyleContext::CalcStyleDifferenceInter
   DO_STRUCT_DIFFERENCE(Content);
   DO_STRUCT_DIFFERENCE(UserInterface);
   DO_STRUCT_DIFFERENCE(Visibility);
   DO_STRUCT_DIFFERENCE(Outline);
   DO_STRUCT_DIFFERENCE(TableBorder);
   DO_STRUCT_DIFFERENCE(Table);
   DO_STRUCT_DIFFERENCE(UIReset);
   DO_STRUCT_DIFFERENCE(Text);
-  DO_STRUCT_DIFFERENCE_WITH_ARGS(List, (, PeekStyleDisplay()));
+  DO_STRUCT_DIFFERENCE_WITH_ARGS(List, (, PEEK(Display)));
   DO_STRUCT_DIFFERENCE(SVGReset);
   DO_STRUCT_DIFFERENCE(SVG);
-  DO_STRUCT_DIFFERENCE_WITH_ARGS(Position, (, PeekStyleVisibility()));
+  DO_STRUCT_DIFFERENCE_WITH_ARGS(Position, (, PEEK(Visibility)));
   DO_STRUCT_DIFFERENCE(Font);
   DO_STRUCT_DIFFERENCE(Margin);
   DO_STRUCT_DIFFERENCE(Padding);
   DO_STRUCT_DIFFERENCE(Border);
   DO_STRUCT_DIFFERENCE(TextReset);
   DO_STRUCT_DIFFERENCE(Effects);
   DO_STRUCT_DIFFERENCE(Background);
   DO_STRUCT_DIFFERENCE(Color);
@@ -314,17 +333,17 @@ nsStyleContext::CalcStyleDifferenceInter
 #undef EXPAND
 
   MOZ_ASSERT(styleStructCount == nsStyleStructID_Length,
              "missing a call to DO_STRUCT_DIFFERENCE");
 
 #ifdef DEBUG
   #define STYLE_STRUCT(name_, callback_)                                      \
     MOZ_ASSERT(!!(structsFound & NS_STYLE_INHERIT_BIT(name_)) ==              \
-               !!PeekStyle##name_(),                                          \
+               !!PEEK(name_),                                                 \
                "PeekStyleData results must not change in the middle of "      \
                "difference calculation.");
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT
 #endif
 
   // We check for struct pointer equality here rather than as part of the
   // DO_STRUCT_DIFFERENCE calls, since those calls can result in structs
@@ -333,17 +352,17 @@ nsStyleContext::CalcStyleDifferenceInter
   // happen when the nsRuleNode::ComputeXXXData method looks up another
   // struct.)  This is important for callers in RestyleManager that
   // need to know the equality or not of the final set of cached struct
   // pointers.
   *aSamePointerStructs = 0;
 
 #define STYLE_STRUCT(name_, callback_)                                        \
   {                                                                           \
-    const nsStyle##name_* data = PeekStyle##name_();                          \
+    const nsStyle##name_* data = PEEK(name_);                                 \
     if (!data || data == aNewContext->ThreadsafeStyle##name_()) {             \
       *aSamePointerStructs |= NS_STYLE_INHERIT_BIT(name_);                    \
     }                                                                         \
   }
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
 
   // Note that we do not check whether this->RelevantLinkVisited() !=
@@ -364,35 +383,27 @@ nsStyleContext::CalcStyleDifferenceInter
   // call GetVisitedDependentColor.
   nsStyleContext *thisVis = GetStyleIfVisited(),
                 *otherVis = aNewContext->GetStyleIfVisited();
   if (!thisVis != !otherVis) {
     // One style context has a style-if-visited and the other doesn't.
     // Presume a difference.
     hint |= nsChangeHint_RepaintFrame;
   } else if (thisVis && !NS_IsHintSubset(nsChangeHint_RepaintFrame, hint)) {
-    // Bug 1364484: Update comments here and potentially remove the assertion
-    // below once we return a non-null visited context in CalcStyleDifference
-    // using Servo values.  The approach is becoming quite similar to Gecko.
-    // We'll handle visited style differently in servo. Assert against being
-    // in the parallel traversal to avoid static analysis hazards when calling
-    // StyleFoo() below.
-    MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal());
-
     // Both style contexts have a style-if-visited.
     bool change = false;
 
     // NB: Calling Peek on |this|, not |thisVis|, since callers may look
     // at a struct on |this| without looking at the same struct on
     // |thisVis| (including this function if we skip one of these checks
     // due to change being true already or due to the old style context
     // not having a style-if-visited), but not the other way around.
 #define STYLE_FIELD(name_) thisVisStruct->name_ != otherVisStruct->name_
 #define STYLE_STRUCT(name_, fields_)                                    \
-    if (!change && PeekStyle##name_()) {                                \
+    if (!change && PEEK(name_)) {                                       \
       const nsStyle##name_* thisVisStruct =                             \
         thisVis->ThreadsafeStyle##name_();                              \
       const nsStyle##name_* otherVisStruct =                            \
         otherVis->ThreadsafeStyle##name_();                             \
       if (MOZ_FOR_EACH_SEPARATED(STYLE_FIELD, (||), (), fields_)) {     \
         change = true;                                                  \
       }                                                                 \
     }
@@ -434,21 +445,23 @@ nsStyleContext::CalcStyleDifferenceInter
   MOZ_ASSERT(NS_IsHintSubset(hint, nsChangeHint_AllHints),
              "Added a new hint without bumping AllHints?");
   return hint & ~nsChangeHint_NeutralChange;
 }
 
 nsChangeHint
 nsStyleContext::CalcStyleDifference(nsStyleContext* aNewContext,
                                     uint32_t* aEqualStructs,
-                                    uint32_t* aSamePointerStructs)
+                                    uint32_t* aSamePointerStructs,
+                                    uint32_t aRelevantStructs)
 {
   return CalcStyleDifferenceInternal(aNewContext,
                                      aEqualStructs,
-                                     aSamePointerStructs);
+                                     aSamePointerStructs,
+                                     aRelevantStructs);
 }
 
 class MOZ_STACK_CLASS FakeStyleContext
 {
 public:
   explicit FakeStyleContext(const ServoComputedValues* aComputedValues)
     : mComputedValues(aComputedValues) {}
 
@@ -472,27 +485,16 @@ public:
   #undef STYLE_STRUCT
 
   const ServoComputedValues* ComputedValues() { return mComputedValues; }
 
 private:
   const ServoComputedValues* MOZ_NON_OWNING_REF mComputedValues;
 };
 
-nsChangeHint
-nsStyleContext::CalcStyleDifference(const ServoComputedValues* aNewComputedValues,
-                                    uint32_t* aEqualStructs,
-                                    uint32_t* aSamePointerStructs)
-{
-  FakeStyleContext newContext(aNewComputedValues);
-  return CalcStyleDifferenceInternal(&newContext,
-                                     aEqualStructs,
-                                     aSamePointerStructs);
-}
-
 namespace mozilla {
 
 void
 GeckoStyleContext::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.
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -248,47 +248,56 @@ 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.
    */
   nsChangeHint CalcStyleDifference(nsStyleContext* aNewContext,
                                    uint32_t* aEqualStructs,
-                                   uint32_t* aSamePointerStructs);
-
-  /**
-   * Like the above, but allows comparing ServoComputedValues instead of needing
-   * a full-fledged style context.
-   */
-  nsChangeHint CalcStyleDifference(const ServoComputedValues* aNewComputedValues,
-                                   uint32_t* aEqualStructs,
-                                   uint32_t* aSamePointerStructs);
+                                   uint32_t* aSamePointerStructs,
+                                   uint32_t aRelevantStructs =
+                                     kAllResolvedStructs);
 
 private:
   template<class StyleContextLike>
   nsChangeHint CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
                                            uint32_t* aEqualStructs,
-                                           uint32_t* aSamePointerStructs);
+                                           uint32_t* aSamePointerStructs,
+                                           uint32_t aRelevantStructs);
 
 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