Bug 1380133 - Part 5: Call CalcStyleDifference with ServoStyleContexts instead of a FakeStyleContext wrapping a ServoComputedValues. r=emilio draft
authorCameron McCormack <cam@mcc.id.au>
Fri, 21 Jul 2017 11:42:44 +0800
changeset 612844 88576e17cd3867acdf8862dab42fd4b1e73bc755
parent 612843 b77d33b5823eca4a57e568881c00959ddecf9089
child 612845 55e3be75bb4325de6431e636ef815bc870bcad94
push id69614
push userbmo:cam@mcc.id.au
push dateFri, 21 Jul 2017 03:44:21 +0000
reviewersemilio
bugs1380133
milestone56.0a1
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 | 21 ++++++++++----- layout/style/ServoBindings.h | 3 ++- layout/style/nsStyleContext.cpp | 58 +++++++++++++++++++++-------------------- layout/style/nsStyleContext.h | 29 ++++++++++++++------- 4 files changed, 65 insertions(+), 46 deletions(-)
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_) != nullptr),                                      \
                "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