Bug 1222745 - Restore eRestyleResult_StopWithStyleChange optimization for shared style contexts by comparing rule nodes for inherited style data changes. r=dbaron

dom/base/nsMappedAttributes.cpp

@@ -184,6 +184,14 @@ nsMappedAttributes::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsMappedAttributes::MightMapInheritedStyleData()
+{
+  // Just assume that we do, rather than adding checks to all of the different
+  // kinds of attribute mapping functions we have.
+  return true;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsMappedAttributes::List(FILE* out, int32_t aIndent) const

dom/base/nsMappedAttributes.h

@@ -74,6 +74,7 @@ public:
 
   // nsIStyleRule 
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif

dom/html/HTMLBodyElement.cpp

@@ -168,6 +168,12 @@ BodyRule::MapRuleInfoInto(nsRuleData* aData)
   }
 }
 
+/* virtual */ bool
+BodyRule::MightMapInheritedStyleData()
+{
+  return false;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 BodyRule::List(FILE* out, int32_t aIndent) const

dom/html/HTMLBodyElement.h

@@ -28,6 +28,7 @@ public:
 
   // nsIStyleRule interface
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif

layout/base/RestyleManager.cpp

@@ -3694,6 +3694,89 @@ ElementRestyler::CanReparentStyleContext(nsRestyleHint aRestyleHint)
          !mPresContext->StyleSet()->IsInRuleTreeReconstruct();
 }
 
+// Returns true iff any rule node that is an ancestor-or-self of the
+// two specified rule nodes, but which is not an ancestor of both,
+// has any inherited style data.  If false is returned, then we know
+// that a change from one rule node to the other must not result in
+// any change in inherited style data.
+static bool
+CommonInheritedStyleData(nsRuleNode* aRuleNode1, nsRuleNode* aRuleNode2)
+{
+  if (aRuleNode1 == aRuleNode2) {
+    return true;
+  }
+
+  nsRuleNode* n1 = aRuleNode1->GetParent();
+  nsRuleNode* n2 = aRuleNode2->GetParent();
+
+  if (n1 == n2) {
+    // aRuleNode1 and aRuleNode2 sharing a parent is a common case, e.g.
+    // when modifying a style="" attribute.  (We must null check GetRule()'s
+    // result since although we know the two parents are the same, it might
+    // be null, as in the case of the two rule nodes being roots of two
+    // different rule trees.)
+    if (aRuleNode1->GetRule() &&
+        aRuleNode1->GetRule()->MightMapInheritedStyleData()) {
+      return false;
+    }
+    if (aRuleNode2->GetRule() &&
+        aRuleNode2->GetRule()->MightMapInheritedStyleData()) {
+      return false;
+    }
+    return true;
+  }
+
+  // Compute the depths of aRuleNode1 and aRuleNode2.
+  int d1 = 0, d2 = 0;
+  while (n1) {
+    ++d1;
+    n1 = n1->GetParent();
+  }
+  while (n2) {
+    ++d2;
+    n2 = n2->GetParent();
+  }
+
+  // Make aRuleNode1 be the deeper node.
+  if (d2 > d1) {
+    std::swap(d1, d2);
+    std::swap(aRuleNode1, aRuleNode2);
+  }
+
+  // Check all of the rule nodes in the deeper branch until we reach
+  // the same depth as the shallower branch.
+  n1 = aRuleNode1;
+  n2 = aRuleNode2;
+  while (d1 > d2) {
+    nsIStyleRule* rule = n1->GetRule();
+    MOZ_ASSERT(rule, "non-root rule node should have a rule");
+    if (rule->MightMapInheritedStyleData()) {
+      return false;
+    }
+    n1 = n1->GetParent();
+    --d1;
+  }
+
+  // Check both branches simultaneously until we reach a common ancestor.
+  while (n1 != n2) {
+    MOZ_ASSERT(n1);
+    MOZ_ASSERT(n2);
+    // As above, we must null check GetRule()'s result since we won't find
+    // a common ancestor if the two rule nodes come from different rule trees,
+    // and thus we might reach the root (which has a null rule).
+    if (n1->GetRule() && n1->GetRule()->MightMapInheritedStyleData()) {
+      return false;
+    }
+    if (n2->GetRule() && n2->GetRule()->MightMapInheritedStyleData()) {
+      return false;
+    }
+    n1 = n1->GetParent();
+    n2 = n2->GetParent();
+  }
+
+  return true;
+}
+
 ElementRestyler::RestyleResult
 ElementRestyler::RestyleSelf(nsIFrame* aSelf,
                              nsRestyleHint aRestyleHint,
@@ -3957,14 +4040,24 @@ ElementRestyler::RestyleSelf(nsIFrame* aSelf,
       LOG_RESTYLE_CONTINUE("the old style context is shared");
       result = eRestyleResult_Continue;
 
-      // It's not safe to return eRestyleResult_StopWithStyleChange,
-      // as even though we might not have cached structs for inherited
-      // properties on oldContext (and thus our samePointerStructs
-      // check later will look OK), oldContext and newContext might
-      // represent different inherited style data, and some of the
-      // elements currently sharing oldContext might need to keep it
-      // rather than get restyled to use newContext.
-      canStopWithStyleChange = false;
+      // It is not safe to return eRestyleResult_StopWithStyleChange
+      // when oldContext is shared and newContext has different
+      // inherited style data, regardless of whether the oldContext has
+      // that inherited style data cached.  We can't simply rely on the
+      // samePointerStructs check later on, as the descendent style
+      // contexts just might not have had their inherited style data
+      // requested yet (which is possible for example if we flush style
+      // between resolving an initial style context for a frame and
+      // building its display list items).  Therefore we must compare
+      // the rule nodes of oldContext and newContext to see if the
+      // restyle results in new inherited style data.  If not, then
+      // we can continue assuming that eRestyleResult_StopWithStyleChange
+      // is safe.  Without this check, we could end up with style contexts
+      // shared between elements which should have different styles.
+      if (!CommonInheritedStyleData(oldContext->RuleNode(),
+                                    newContext->RuleNode())) {
+        canStopWithStyleChange = false;
+      }
     }
 
     // Look at some details of the new style context to see if it would

layout/style/AnimationCommon.cpp

@@ -397,6 +397,12 @@ AnimValuesStyleRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+AnimValuesStyleRule::MightMapInheritedStyleData()
+{
+  return mStyleBits & NS_STYLE_INHERITED_STRUCT_MASK;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 AnimValuesStyleRule::List(FILE* out, int32_t aIndent) const

layout/style/AnimationCommon.h

@@ -158,11 +158,15 @@ protected:
 class AnimValuesStyleRule final : public nsIStyleRule
 {
 public:
+  AnimValuesStyleRule()
+    : mStyleBits(0) {}
+
   // nsISupports implementation
   NS_DECL_ISUPPORTS
 
   // nsIStyleRule implementation
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
@@ -171,6 +175,8 @@ public:
   {
     PropertyValuePair v = { aProperty, aStartValue };
     mPropertyValuePairs.AppendElement(v);
+    mStyleBits |=
+      nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[aProperty]);
   }
 
   // Caller must fill in returned value.
@@ -178,6 +184,8 @@ public:
   {
     PropertyValuePair *p = mPropertyValuePairs.AppendElement();
     p->mProperty = aProperty;
+    mStyleBits |=
+      nsCachedStyleData::GetBitForSID(nsCSSProps::kSIDTable[aProperty]);
     return &p->mValue;
   }
 
@@ -198,6 +206,7 @@ private:
   ~AnimValuesStyleRule() {}
 
   InfallibleTArray<PropertyValuePair> mPropertyValuePairs;
+  uint32_t mStyleBits;
 };
 
 typedef InfallibleTArray<RefPtr<dom::Animation>> AnimationPtrArray;

layout/style/Declaration.cpp

@@ -29,6 +29,12 @@ ImportantStyleData::MapRuleInfoInto(nsRuleData* aRuleData)
   Declaration()->MapImportantRuleInfoInto(aRuleData);
 }
 
+/* virtual */ bool
+ImportantStyleData::MightMapInheritedStyleData()
+{
+  return Declaration()->MapsImportantInheritedStyleData();
+}
+
 #ifdef DEBUG
 /* virtual */ void
 ImportantStyleData::List(FILE* out, int32_t aIndent) const
@@ -88,13 +94,35 @@ NS_IMPL_RELEASE(Declaration)
 /* virtual */ void
 Declaration::MapRuleInfoInto(nsRuleData* aRuleData)
 {
-  MOZ_ASSERT(mData, "called while expanded");
+  MOZ_ASSERT(mData, "must call only while compressed");
   mData->MapRuleInfoInto(aRuleData);
   if (mVariables) {
     mVariables->MapRuleInfoInto(aRuleData);
   }
 }
 
+/* virtual */ bool
+Declaration::MightMapInheritedStyleData()
+{
+  MOZ_ASSERT(mData, "must call only while compressed");
+  if (mVariables && mVariables->Count() != 0) {
+    return true;
+  }
+  return mData->HasInheritedStyleData();
+}
+
+bool
+Declaration::MapsImportantInheritedStyleData() const
+{
+  MOZ_ASSERT(mData, "must call only while compressed");
+  MOZ_ASSERT(HasImportantData(), "must only be called for Declarations with "
+                                 "important data");
+  if (mImportantVariables && mImportantVariables->Count() != 0) {
+    return true;
+  }
+  return mImportantData ? mImportantData->HasInheritedStyleData() : false;
+}
+
 void
 Declaration::ValueAppended(nsCSSProperty aProperty)
 {

layout/style/Declaration.h

@@ -59,6 +59,7 @@ public:
 
   // nsIStyleRule interface
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
@@ -101,6 +102,7 @@ public:
 
   // nsIStyleRule implementation
   virtual void MapRuleInfoInto(nsRuleData *aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
@@ -232,6 +234,8 @@ public:
     }
   }
 
+  bool MapsImportantInheritedStyleData() const;
+
   /**
    * Attempt to replace the value for |aProperty| stored in this
    * declaration with the matching value from |aFromBlock|.

layout/style/nsCSSDataBlock.h

@@ -15,6 +15,7 @@
 #include "nsCSSProps.h"
 #include "nsCSSPropertySet.h"
 #include "nsCSSValue.h"
+#include "nsStyleStruct.h"
 #include "imgRequestProxy.h"
 
 struct nsRuleData;
@@ -92,6 +93,11 @@ public:
     bool HasDefaultBorderImageOutset() const;
     bool HasDefaultBorderImageRepeat() const;
 
+    bool HasInheritedStyleData() const
+    {
+      return mStyleBits & NS_STYLE_INHERITED_STRUCT_MASK;
+    }
+
 private:
     void* operator new(size_t aBaseSize, uint32_t aNumProps) {
         MOZ_ASSERT(aBaseSize == sizeof(nsCSSCompressedDataBlock),

layout/style/nsHTMLStyleSheet.cpp

@@ -51,6 +51,12 @@ nsHTMLStyleSheet::HTMLColorRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsHTMLStyleSheet::HTMLColorRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsHTMLStyleSheet::HTMLColorRule::List(FILE* out, int32_t aIndent) const
@@ -90,6 +96,12 @@ nsHTMLStyleSheet::TableTHRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsHTMLStyleSheet::TableTHRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 /* virtual */ void
 nsHTMLStyleSheet::TableQuirkColorRule::MapRuleInfoInto(nsRuleData* aRuleData)
 {
@@ -103,6 +115,12 @@ nsHTMLStyleSheet::TableQuirkColorRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsHTMLStyleSheet::TableQuirkColorRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 
 NS_IMPL_ISUPPORTS(nsHTMLStyleSheet::LangRule, nsIStyleRule)
 
@@ -117,6 +135,12 @@ nsHTMLStyleSheet::LangRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsHTMLStyleSheet::LangRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsHTMLStyleSheet::LangRule::List(FILE* out, int32_t aIndent) const

layout/style/nsHTMLStyleSheet.h

@@ -84,6 +84,7 @@ private:
 
     // nsIStyleRule interface
     virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+    virtual bool MightMapInheritedStyleData() override;
   #ifdef DEBUG
     virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
   #endif
@@ -106,6 +107,7 @@ private:
 
     // nsIStyleRule interface
     virtual void MapRuleInfoInto(nsRuleData* aRuleData) override = 0;
+    virtual bool MightMapInheritedStyleData() override = 0;
   #ifdef DEBUG
     virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
   #endif
@@ -119,6 +121,7 @@ private:
     TableTHRule() {}
 
     virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+    virtual bool MightMapInheritedStyleData() override;
   };
 
   // Rule to handle quirk table colors
@@ -127,6 +130,7 @@ private:
     TableQuirkColorRule() {}
 
     virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+    virtual bool MightMapInheritedStyleData() override;
   };
 
 public: // for mLangRuleTable structures only
@@ -145,6 +149,7 @@ public: // for mLangRuleTable structures only
 
     // nsIStyleRule interface
     virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+    virtual bool MightMapInheritedStyleData() override;
   #ifdef DEBUG
     virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
   #endif

layout/style/nsIStyleRule.h

@@ -71,6 +71,12 @@ public:
    */
   virtual void MapRuleInfoInto(nsRuleData* aRuleData)=0;
 
+  /**
+   * Returns whether this style rule has any style data for inherited
+   * properties.
+   */
+  virtual bool MightMapInheritedStyleData() = 0;
+
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const = 0;
 #endif

layout/style/nsStyleSet.cpp

@@ -54,6 +54,12 @@ nsEmptyStyleRule::MapRuleInfoInto(nsRuleData* aRuleData)
 {
 }
 
+/* virtual */ bool
+nsEmptyStyleRule::MightMapInheritedStyleData()
+{
+  return false;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsEmptyStyleRule::List(FILE* out, int32_t aIndent) const
@@ -107,6 +113,12 @@ nsInitialStyleRule::MapRuleInfoInto(nsRuleData* aRuleData)
   }
 }
 
+/* virtual */ bool
+nsInitialStyleRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsInitialStyleRule::List(FILE* out, int32_t aIndent) const
@@ -132,6 +144,12 @@ nsDisableTextZoomStyleRule::MapRuleInfoInto(nsRuleData* aRuleData)
     value->SetNoneValue();
 }
 
+/* virtual */ bool
+nsDisableTextZoomStyleRule::MightMapInheritedStyleData()
+{
+  return true;
+}
+
 #ifdef DEBUG
 /* virtual */ void
 nsDisableTextZoomStyleRule::List(FILE* out, int32_t aIndent) const

layout/style/nsStyleSet.h

@@ -50,6 +50,7 @@ private:
 public:
   NS_DECL_ISUPPORTS
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
@@ -63,6 +64,7 @@ private:
 public:
   NS_DECL_ISUPPORTS
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
@@ -76,6 +78,7 @@ private:
 public:
   NS_DECL_ISUPPORTS
   virtual void MapRuleInfoInto(nsRuleData* aRuleData) override;
+  virtual bool MightMapInheritedStyleData() override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif