Backed out changeset 9a84b6988af9 (bug 1380133)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 20 Jul 2017 15:39:58 +0200
changeset 418591 0a99d8945fd9876a90e1619a33ae3637cb8817e4
parent 418590 d986a76643602972c0024a93f07a88b7fa685ce8
child 418592 8e48ee4ce7d4e8ab0a57668ea334fadd5dff31a6
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)
bugs1380133
milestone56.0a1
backs out9a84b6988af9f5ed854927401acfae541de80d3f
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
Backed out changeset 9a84b6988af9 (bug 1380133)
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,35 +393,28 @@ Gecko_GetStyleContext(RawGeckoElementBor
 
 CSSPseudoElementType
 Gecko_GetImplementedPseudo(RawGeckoElementBorrowed aElement)
 {
   return aElement->GetPseudoElementType();
 }
 
 nsChangeHint
-Gecko_CalcStyleDifference(const ServoStyleContext* aOldStyle,
+Gecko_CalcStyleDifference(nsStyleContext* aOldStyleFromFrame,
                           const ServoStyleContext* aNewStyle,
-                          uint64_t aOldStyleBits,
                           bool* aAnyStyleChanged)
 {
-  MOZ_ASSERT(aOldStyle);
+  MOZ_ASSERT(aOldStyleFromFrame);
   MOZ_ASSERT(aNewStyle);
 
-  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);
-
+  uint32_t equalStructs, samePointerStructs;
+  nsChangeHint result =
+    aOldStyleFromFrame->CalcStyleDifference(aNewStyle->ComputedValues(),
+                                            &equalStructs,
+                                            &samePointerStructs);
   *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,19 +373,18 @@ 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(const mozilla::ServoStyleContext* old_style,
+nsChangeHint Gecko_CalcStyleDifference(nsStyleContext* old_style_from_frame,
                                        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,31 +184,23 @@ nsStyleContext::MoveTo(nsStyleContext* a
     styleIfVisited->mParent->AddChild(styleIfVisited);
   }
 }
 
 template<class StyleContextLike>
 nsChangeHint
 nsStyleContext::CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
                                             uint32_t* aEqualStructs,
-                                            uint32_t* aSamePointerStructs,
-                                            uint32_t aRelevantStructs)
+                                            uint32_t* aSamePointerStructs)
 {
   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
@@ -245,31 +237,20 @@ 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_ = PEEK(struct_);                    \
+    const nsStyle##struct_* this##struct_ = PeekStyle##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;                                               \
@@ -310,20 +291,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, (, PEEK(Display)));
+  DO_STRUCT_DIFFERENCE_WITH_ARGS(List, (, PeekStyleDisplay()));
   DO_STRUCT_DIFFERENCE(SVGReset);
   DO_STRUCT_DIFFERENCE(SVG);
-  DO_STRUCT_DIFFERENCE_WITH_ARGS(Position, (, PEEK(Visibility)));
+  DO_STRUCT_DIFFERENCE_WITH_ARGS(Position, (, PeekStyleVisibility()));
   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);
@@ -333,17 +314,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_)) ==              \
-               !!PEEK(name_),                                                 \
+               !!PeekStyle##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
@@ -352,17 +333,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 = PEEK(name_);                                 \
+    const nsStyle##name_* data = PeekStyle##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() !=
@@ -383,27 +364,35 @@ 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 && PEEK(name_)) {                                       \
+    if (!change && PeekStyle##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;                                                  \
       }                                                                 \
     }
@@ -445,23 +434,21 @@ 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 aRelevantStructs)
+                                    uint32_t* aSamePointerStructs)
 {
   return CalcStyleDifferenceInternal(aNewContext,
                                      aEqualStructs,
-                                     aSamePointerStructs,
-                                     aRelevantStructs);
+                                     aSamePointerStructs);
 }
 
 class MOZ_STACK_CLASS FakeStyleContext
 {
 public:
   explicit FakeStyleContext(const ServoComputedValues* aComputedValues)
     : mComputedValues(aComputedValues) {}
 
@@ -485,16 +472,27 @@ 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,56 +248,47 @@ 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,
-                                   uint32_t aRelevantStructs =
-                                     kAllResolvedStructs);
+                                   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);
 
 private:
   template<class StyleContextLike>
   nsChangeHint CalcStyleDifferenceInternal(StyleContextLike* aNewContext,
                                            uint32_t* aEqualStructs,
-                                           uint32_t* aSamePointerStructs,
-                                           uint32_t aRelevantStructs);
+                                           uint32_t* aSamePointerStructs);
 
 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