Bug 1337068 - Part 3: stylo: Add thread-safe version of nsStyleUtil::IsSignificantChild(); r=emilio draft
authorMatt Brubeck <mbrubeck@mozilla.com>
Mon, 20 Mar 2017 08:43:43 -0700
changeset 501610 d8a1746cbba03f2d8bcc722d54923e4c1c76da66
parent 501569 05bfa2831c0ba4a26fa72328ffe6a99aba9c356a
child 501611 749145496f104e5d1272a5052f146fd61da0473e
push id50046
push userbmo:mbrubeck@mozilla.com
push dateMon, 20 Mar 2017 16:47:20 +0000
reviewersemilio
bugs1337068
milestone55.0a1
Bug 1337068 - Part 3: stylo: Add thread-safe version of nsStyleUtil::IsSignificantChild(); r=emilio MozReview-Commit-ID: BZnAR0nROOS
dom/base/FragmentOrElement.cpp
dom/base/FragmentOrElement.h
dom/base/nsGenericDOMDataNode.cpp
dom/base/nsGenericDOMDataNode.h
dom/base/nsIContent.h
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsStyleUtil.cpp
layout/style/nsStyleUtil.h
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -2061,17 +2061,17 @@ FragmentOrElement::AppendText(const char
                              bool aNotify)
 {
   NS_ERROR("called FragmentOrElement::AppendText");
 
   return NS_ERROR_FAILURE;
 }
 
 bool
-FragmentOrElement::TextIsOnlyWhitespace()
+FragmentOrElement::TextIsOnlyWhitespace() const
 {
   return false;
 }
 
 bool
 FragmentOrElement::HasTextForTranslation()
 {
   return false;
--- a/dom/base/FragmentOrElement.h
+++ b/dom/base/FragmentOrElement.h
@@ -137,17 +137,17 @@ public:
                            bool aNotify) override;
   // Need to implement this here too to avoid hiding.
   nsresult SetText(const nsAString& aStr, bool aNotify)
   {
     return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
   }
   virtual nsresult AppendText(const char16_t* aBuffer, uint32_t aLength,
                               bool aNotify) override;
-  virtual bool TextIsOnlyWhitespace() override;
+  virtual bool TextIsOnlyWhitespace() const override;
   virtual bool HasTextForTranslation() override;
   virtual void AppendTextTo(nsAString& aResult) override;
   MOZ_MUST_USE
   virtual bool AppendTextTo(nsAString& aResult, const mozilla::fallible_t&) override;
   virtual nsIContent *GetBindingParent() const override;
   virtual nsXBLBinding *GetXBLBinding() const override;
   virtual void SetXBLBinding(nsXBLBinding* aBinding,
                              nsBindingManager* aOldBindingManager = nullptr) override;
--- a/dom/base/nsGenericDOMDataNode.cpp
+++ b/dom/base/nsGenericDOMDataNode.cpp
@@ -976,17 +976,36 @@ nsresult
 nsGenericDOMDataNode::AppendText(const char16_t* aBuffer,
                                  uint32_t aLength,
                                  bool aNotify)
 {
   return SetTextInternal(mText.GetLength(), 0, aBuffer, aLength, aNotify);
 }
 
 bool
-nsGenericDOMDataNode::TextIsOnlyWhitespace()
+nsGenericDOMDataNode::TextIsOnlyWhitespace() const
+{
+  bool result = TextIsOnlyWhitespaceInternal();
+  if (NS_IsMainThread()) {
+    nsGenericDOMDataNode* self = const_cast<nsGenericDOMDataNode*>(this);
+    if (!result) {
+      self->UnsetFlags(NS_TEXT_IS_ONLY_WHITESPACE);
+      self->SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE);
+      return false;
+    }
+
+    self->SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE | NS_TEXT_IS_ONLY_WHITESPACE);
+    return true;
+  } else {
+    return result;
+  }
+}
+
+bool
+nsGenericDOMDataNode::TextIsOnlyWhitespaceInternal() const
 {
   // FIXME: should this method take content language into account?
   if (mText.Is2b()) {
     // The fragment contains non-8bit characters and such characters
     // are never considered whitespace.
     return false;
   }
 
@@ -996,25 +1015,22 @@ nsGenericDOMDataNode::TextIsOnlyWhitespa
 
   const char* cp = mText.Get1b();
   const char* end = cp + mText.GetLength();
 
   while (cp < end) {
     char ch = *cp;
 
     if (!dom::IsSpaceCharacter(ch)) {
-      UnsetFlags(NS_TEXT_IS_ONLY_WHITESPACE);
-      SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE);
       return false;
     }
 
     ++cp;
   }
 
-  SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE | NS_TEXT_IS_ONLY_WHITESPACE);
   return true;
 }
 
 bool
 nsGenericDOMDataNode::HasTextForTranslation()
 {
   if (NodeType() != nsIDOMNode::TEXT_NODE &&
       NodeType() != nsIDOMNode::CDATA_SECTION_NODE) {
--- a/dom/base/nsGenericDOMDataNode.h
+++ b/dom/base/nsGenericDOMDataNode.h
@@ -130,17 +130,18 @@ public:
                            bool aNotify) override;
   // Need to implement this here too to avoid hiding.
   nsresult SetText(const nsAString& aStr, bool aNotify)
   {
     return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
   }
   virtual nsresult AppendText(const char16_t* aBuffer, uint32_t aLength,
                               bool aNotify) override;
-  virtual bool TextIsOnlyWhitespace() override;
+  virtual bool TextIsOnlyWhitespace() const override;
+  bool TextIsOnlyWhitespaceInternal() const;
   virtual bool HasTextForTranslation() override;
   virtual void AppendTextTo(nsAString& aResult) override;
   MOZ_MUST_USE
   virtual bool AppendTextTo(nsAString& aResult,
                             const mozilla::fallible_t&) override;
   virtual void SaveSubtreeState() override;
 
 #ifdef DEBUG
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@@ -551,17 +551,17 @@ public:
   {
     return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
   }
 
   /**
    * Query method to see if the frame is nothing but whitespace
    * NOTE: Always returns false for elements
    */
-  virtual bool TextIsOnlyWhitespace() = 0;
+  virtual bool TextIsOnlyWhitespace() const = 0;
 
   /**
    * Method to see if the text node contains data that is useful
    * for a translation: i.e., it consists of more than just whitespace,
    * digits and punctuation.
    * NOTE: Always returns false for elements.
    */
   virtual bool HasTextForTranslation() = 0;
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1774,30 +1774,30 @@ static bool SelectorMatches(Element* aEl
       if (!checkGenericEmptyMatches(aElement, aTreeMatchContext, false)) {
         return false;
       }
       break;
 
     case CSSPseudoClassType::mozEmptyExceptChildrenWithLocalname:
       {
         NS_ASSERTION(pseudoClass->u.mString, "Must have string!");
-        nsIContent *child = nullptr;
+        const nsIContent* child = nullptr;
         int32_t index = -1;
 
         if (aTreeMatchContext.mForStyling)
           // FIXME:  This isn't sufficient to handle:
           //   :-moz-empty-except-children-with-localname() + E
           //   :-moz-empty-except-children-with-localname() ~ E
           // because we don't know to restyle the grandparent of the
           // inserted/removed element (as in bug 534804 for :empty).
           aElement->SetFlags(NODE_HAS_SLOW_SELECTOR);
         do {
           child = aElement->GetChildAt(++index);
         } while (child &&
-                  (!IsSignificantChild(child, true, false) ||
+                  (!nsStyleUtil::IsSignificantChild(child, true, false) ||
                   (child->GetNameSpaceID() == aElement->GetNameSpaceID() &&
                     child->NodeInfo()->NameAtom()->Equals(nsDependentString(pseudoClass->u.mString)))));
         if (child != nullptr) {
           return false;
         }
       }
       break;
 
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -718,17 +718,17 @@ nsStyleUtil::ColorComponentToFloat(uint8
   if (FloatToColorComponent(rounded) != aAlpha) {
     // Use three decimal places.
     rounded = NS_roundf(float(aAlpha) * 1000.0f / 255.0f) / 1000.0f;
   }
   return rounded;
 }
 
 /* static */ bool
-nsStyleUtil::IsSignificantChild(nsIContent* aChild, bool aTextIsSignificant,
+nsStyleUtil::IsSignificantChild(const nsIContent* aChild, bool aTextIsSignificant,
                                 bool aWhitespaceIsSignificant)
 {
   NS_ASSERTION(!aWhitespaceIsSignificant || aTextIsSignificant,
                "Nonsensical arguments");
 
   bool isText = aChild->IsNodeOfType(nsINode::eTEXT);
 
   if (!isText && !aChild->IsNodeOfType(nsINode::eCOMMENT) &&
--- a/layout/style/nsStyleUtil.h
+++ b/layout/style/nsStyleUtil.h
@@ -130,19 +130,19 @@ public:
    *
    * Should be used only by serialization code.
    */
   static float ColorComponentToFloat(uint8_t aAlpha);
 
   /*
    * Does this child count as significant for selector matching?
    */
-  static bool IsSignificantChild(nsIContent* aChild,
-                                   bool aTextIsSignificant,
-                                   bool aWhitespaceIsSignificant);
+  static bool IsSignificantChild(const nsIContent* aChild,
+                                 bool aTextIsSignificant,
+                                 bool aWhitespaceIsSignificant);
   /**
    * Returns true if our object-fit & object-position properties might cause
    * a replaced element's contents to overflow its content-box (requiring
    * clipping), or false if we can be sure that this won't happen.
    *
    * This lets us optimize by skipping clipping when we can tell it's
    * unnecessary (particularly with the default values of these properties).
    *