Bug 1441729 - Remove aTextIsSignificant param from nsStyleUtil::IsSignificantChild and its friends. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Wed, 28 Feb 2018 12:54:01 +1100
changeset 762287 32a014591db37fae49e07ee3a1ca82cb84374983
parent 762286 a53aa027d562fbe1938e7bf69f9ef1b90f7b8b1a
child 762334 8f5bb4b94fed20068fa4ff06167b6f22a7666d1b
child 762372 c5525f66a2cc2195e2caeda3784571655b9040ba
push id101120
push userxquan@mozilla.com
push dateThu, 01 Mar 2018 23:52:32 +0000
reviewersemilio
bugs1441729
milestone60.0a1
Bug 1441729 - Remove aTextIsSignificant param from nsStyleUtil::IsSignificantChild and its friends. r?emilio MozReview-Commit-ID: CEZmAwcnglg
dom/base/nsObjectLoadingContent.cpp
layout/base/RestyleManager.cpp
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsStyleUtil.cpp
layout/style/nsStyleUtil.h
servo/components/style/gecko/wrapper.rs
--- a/dom/base/nsObjectLoadingContent.cpp
+++ b/dom/base/nsObjectLoadingContent.cpp
@@ -2964,17 +2964,17 @@ nsObjectLoadingContent::LoadFallback(Fal
   {
     for (nsIContent* child = thisContent->GetFirstChild(); child; ) {
       // When we advance to our next child, we don't want to traverse subtrees
       // under descendant <object> and <embed> elements; those will handle
       // those subtrees themselves if they end up falling back.
       bool skipChildDescendants = false;
       if (aType != eFallbackAlternate &&
           !child->IsHTMLElement(nsGkAtoms::param) &&
-          nsStyleUtil::IsSignificantChild(child, true, false)) {
+          nsStyleUtil::IsSignificantChild(child, false)) {
         aType = eFallbackAlternate;
       }
       if (thisIsObject) {
         if (auto embed = HTMLEmbedElement::FromContent(child)) {
           embed->StartObjectLoad(true, true);
           skipChildDescendants = true;
         } else if (auto object = HTMLObjectElement::FromContent(child)) {
           object->StartObjectLoad(true, true);
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -79,17 +79,17 @@ RestyleManager::ContentAppended(nsIConte
     bool wasEmpty = true; // :empty or :-moz-only-whitespace
     for (nsIContent* cur = container->GetFirstChild();
          cur != aFirstNewContent;
          cur = cur->GetNextSibling()) {
       // We don't know whether we're testing :empty or :-moz-only-whitespace,
       // so be conservative and assume :-moz-only-whitespace (i.e., make
       // IsSignificantChild less likely to be true, and thus make us more
       // likely to restyle).
-      if (nsStyleUtil::IsSignificantChild(cur, true, false)) {
+      if (nsStyleUtil::IsSignificantChild(cur, false)) {
         wasEmpty = false;
         break;
       }
     }
     if (wasEmpty) {
       RestyleForEmptyChange(container);
       return;
     }
@@ -225,17 +225,17 @@ HasAnySignificantSibling(Element* aConta
        child = child->GetNextSibling()) {
     if (child == aChild) {
       continue;
     }
     // We don't know whether we're testing :empty or :-moz-only-whitespace,
     // so be conservative and assume :-moz-only-whitespace (i.e., make
     // IsSignificantChild less likely to be true, and thus make us more
     // likely to restyle).
-    if (nsStyleUtil::IsSignificantChild(child, true, false)) {
+    if (nsStyleUtil::IsSignificantChild(child, false)) {
       return true;
     }
   }
 
   return false;
 }
 
 void
@@ -399,17 +399,17 @@ RestyleManager::ContentRemoved(nsINode* 
     bool isEmpty = true; // :empty or :-moz-only-whitespace
     for (nsIContent* child = container->GetFirstChild();
          child;
          child = child->GetNextSibling()) {
       // We don't know whether we're testing :empty or :-moz-only-whitespace,
       // so be conservative and assume :-moz-only-whitespace (i.e., make
       // IsSignificantChild less likely to be true, and thus make us more
       // likely to restyle).
-      if (nsStyleUtil::IsSignificantChild(child, true, false)) {
+      if (nsStyleUtil::IsSignificantChild(child, false)) {
         isEmpty = false;
         break;
       }
     }
     if (isEmpty) {
       RestyleForEmptyChange(container);
       return;
     }
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -130,21 +130,20 @@ AssertIsMainThreadOrServoLangFontPrefsCa
 
 
 /*
  * Does this child count as significant for selector matching?
  *
  * See nsStyleUtil::IsSignificantChild for details.
  */
 bool
-Gecko_IsSignificantChild(RawGeckoNodeBorrowed aNode, bool aTextIsSignificant,
+Gecko_IsSignificantChild(RawGeckoNodeBorrowed aNode,
                          bool aWhitespaceIsSignificant)
 {
   return nsStyleUtil::ThreadSafeIsSignificantChild(aNode->AsContent(),
-                                                   aTextIsSignificant,
                                                    aWhitespaceIsSignificant);
 }
 
 RawGeckoNodeBorrowedOrNull
 Gecko_GetLastChild(RawGeckoNodeBorrowed aNode)
 {
   return aNode->GetLastChild();
 }
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -148,19 +148,17 @@ struct FontSizePrefs
 };
 
 struct MediumFeaturesChangedResult {
   bool mAffectsDocumentRules;
   bool mAffectsNonDocumentRules;
   bool mUsesViewportUnits;
 };
 
-bool Gecko_IsSignificantChild(RawGeckoNodeBorrowed node,
-                              bool text_is_significant,
-                              bool whitespace_is_significant);
+bool Gecko_IsSignificantChild(RawGeckoNodeBorrowed node, bool whitespace_is_significant);
 RawGeckoNodeBorrowedOrNull Gecko_GetLastChild(RawGeckoNodeBorrowed node);
 RawGeckoNodeBorrowedOrNull Gecko_GetFlattenedTreeParentNode(RawGeckoNodeBorrowed node);
 RawGeckoElementBorrowedOrNull Gecko_GetBeforeOrAfterPseudo(RawGeckoElementBorrowed element, bool is_before);
 nsTArray<nsIContent*>* Gecko_GetAnonymousContentForElement(RawGeckoElementBorrowed element);
 void Gecko_DestroyAnonymousContentList(nsTArray<nsIContent*>* anon_content);
 
 void Gecko_ServoStyleContext_Init(mozilla::ServoStyleContext* context,
                                   ServoStyleContextBorrowedOrNull parent_context,
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1216,21 +1216,19 @@ static inline bool ActiveHoverQuirkMatch
     }
   }
 
   return true;
 }
 
 
 static inline bool
-IsSignificantChild(nsIContent* aChild, bool aTextIsSignificant,
-                   bool aWhitespaceIsSignificant)
+IsSignificantChild(nsIContent* aChild, bool aWhitespaceIsSignificant)
 {
-  return nsStyleUtil::IsSignificantChild(aChild, aTextIsSignificant,
-                                         aWhitespaceIsSignificant);
+  return nsStyleUtil::IsSignificantChild(aChild, aWhitespaceIsSignificant);
 }
 
 // This function is to be called once we have fetched a value for an attribute
 // whose namespace and name match those of aAttrSelector.  This function
 // performs comparisons on the value only, based on aAttrSelector->mFunction.
 static bool AttrMatchesValue(const nsAttrSelector* aAttrSelector,
                                const nsString& aValue, bool isHTML)
 {
@@ -1355,17 +1353,17 @@ checkGenericEmptyMatches(Element* aEleme
   nsIContent *child = nullptr;
 
   if (aTreeMatchContext.mForStyling)
     aElement->SetFlags(NODE_HAS_EMPTY_SELECTOR);
 
   // stop at first non-comment (and non-whitespace for :-moz-only-whitespace)
   // node
   for (child = aElement->GetFirstChild();
-       child && !IsSignificantChild(child, true, isWhitespaceSignificant);
+       child && !IsSignificantChild(child, isWhitespaceSignificant);
        child = child->GetNextSibling());
   return (child == nullptr);
 }
 
 static const EventStates sPseudoClassStates[] = {
 #define CSS_PSEUDO_CLASS(_name, _value, _flags, _pref) \
   EventStates(),
 #define CSS_STATE_PSEUDO_CLASS(_name, _value, _flags, _pref, _states) \
@@ -1627,17 +1625,17 @@ static bool SelectorMatches(Element* aEl
         nsIContent *firstNode = nullptr;
         nsIContent *parent = aElement->GetParent();
         if (parent) {
           if (aTreeMatchContext.mForStyling)
             parent->SetFlags(NODE_HAS_EDGE_CHILD_SELECTOR);
 
           // stop at first non-comment and non-whitespace node
           for (firstNode = parent->GetFirstChild();
-               firstNode && !IsSignificantChild(firstNode, true, false);
+               firstNode && !IsSignificantChild(firstNode, false);
                firstNode = firstNode->GetNextSibling());
         }
         if (aElement != firstNode) {
           return false;
         }
       }
       break;
 
@@ -1652,17 +1650,17 @@ static bool SelectorMatches(Element* aEl
         nsIContent *lastNode = nullptr;
         nsIContent *parent = aElement->GetParent();
         if (parent) {
           if (aTreeMatchContext.mForStyling)
             parent->SetFlags(NODE_HAS_EDGE_CHILD_SELECTOR);
 
           // stop at first non-comment and non-whitespace node
           for (lastNode = parent->GetLastChild();
-               lastNode && !IsSignificantChild(lastNode, true, false);
+               lastNode && !IsSignificantChild(lastNode, false);
                lastNode = lastNode->GetPreviousSibling());
         }
         if (aElement != lastNode) {
           return false;
         }
       }
       break;
 
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -736,50 +736,43 @@ 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(nsIContent* aChild,
                                 bool aWhitespaceIsSignificant)
 {
-  NS_ASSERTION(!aWhitespaceIsSignificant || aTextIsSignificant,
-               "Nonsensical arguments");
-
   bool isText = aChild->IsNodeOfType(nsINode::eTEXT);
 
   if (!isText && !aChild->IsNodeOfType(nsINode::eCOMMENT) &&
       !aChild->IsNodeOfType(nsINode::ePROCESSING_INSTRUCTION)) {
     return true;
   }
 
-  return aTextIsSignificant && isText && aChild->TextLength() != 0 &&
+  return isText && aChild->TextLength() != 0 &&
          (aWhitespaceIsSignificant ||
           !aChild->TextIsOnlyWhitespace());
 }
 
 /* static */ bool
 nsStyleUtil::ThreadSafeIsSignificantChild(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) &&
       !aChild->IsNodeOfType(nsINode::ePROCESSING_INSTRUCTION)) {
     return true;
   }
 
-  return aTextIsSignificant && isText && aChild->TextLength() != 0 &&
+  return isText && aChild->TextLength() != 0 &&
          (aWhitespaceIsSignificant ||
           !aChild->ThreadSafeTextIsOnlyWhitespace());
 }
 
 // For a replaced element whose concrete object size is no larger than the
 // element's content-box, this method checks whether the given
 // "object-position" coordinate might cause overflow in its dimension.
 static bool
--- a/layout/style/nsStyleUtil.h
+++ b/layout/style/nsStyleUtil.h
@@ -144,24 +144,22 @@ 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);
+                                 bool aWhitespaceIsSignificant);
 
   /*
    * Thread-safe version of IsSignificantChild()
    */
   static bool ThreadSafeIsSignificantChild(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).
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -251,17 +251,17 @@ impl<'ln> GeckoNode<'ln> {
 
         // NOTE(emilio): If this call is too expensive, we could manually
         // inline more aggressively.
         unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).map(GeckoNode) }
     }
 
     #[inline]
     fn contains_non_whitespace_content(&self) -> bool {
-        unsafe { Gecko_IsSignificantChild(self.0, true, false) }
+        unsafe { Gecko_IsSignificantChild(self.0, false) }
     }
 }
 
 impl<'ln> NodeInfo for GeckoNode<'ln> {
     #[inline]
     fn is_element(&self) -> bool {
         self.get_bool_flag(nsINode_BooleanFlag::NodeIsElement)
     }
@@ -1928,17 +1928,17 @@ impl<'le> ::selectors::Element for Gecko
 
         unsafe {
             Gecko_IsRootElement(self.0)
         }
     }
 
     fn is_empty(&self) -> bool {
         !self.as_node().dom_children().any(|child| unsafe {
-            Gecko_IsSignificantChild(child.0, true, true)
+            Gecko_IsSignificantChild(child.0, true)
         })
     }
 
     #[inline]
     fn local_name(&self) -> &WeakAtom {
         unsafe {
             WeakAtom::new(self.as_node().node_info().mInner.mName)
         }