Bug 1454251: Remove nsINode::eATTRIBUTE. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 15 Apr 2018 15:12:02 +0200
changeset 782395 1e8b955b13601faead9ed2c47aca8e5f5370342d
parent 782394 eab04ad2be68445843dc4beca11a574750af0286
push id106521
push userbmo:emilio@crisal.io
push dateSun, 15 Apr 2018 13:29:46 +0000
reviewersbz
bugs1454251
milestone61.0a1
Bug 1454251: Remove nsINode::eATTRIBUTE. r?bz MozReview-Commit-ID: 7HeUbcG6szy
dom/base/Attr.cpp
dom/base/Attr.h
dom/base/nsINode.cpp
dom/base/nsINode.h
dom/base/nsNodeUtils.cpp
dom/base/nsRange.cpp
dom/xslt/xpath/XPathResult.cpp
--- a/dom/base/Attr.cpp
+++ b/dom/base/Attr.cpp
@@ -257,17 +257,17 @@ Attr::SetTextContentInternal(const nsASt
                              ErrorResult& aError)
 {
   SetNodeValueInternal(aTextContent, aError);
 }
 
 bool
 Attr::IsNodeOfType(uint32_t aFlags) const
 {
-    return !(aFlags & ~eATTRIBUTE);
+    return false;
 }
 
 uint32_t
 Attr::GetChildCount() const
 {
   return 0;
 }
 
--- a/dom/base/Attr.h
+++ b/dom/base/Attr.h
@@ -35,16 +35,18 @@ class Attr final : public nsIAttribute,
 
 public:
   Attr(nsDOMAttributeMap* aAttrMap,
        already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
        const nsAString& aValue);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
+  NS_IMPL_FROMNODE_HELPER(Attr, IsAttr())
+
   // nsINode interface
   virtual void GetTextContentInternal(nsAString& aTextContent,
                                       OOMReporter& aError) override;
   virtual void SetTextContentInternal(const nsAString& aTextContent,
                                       nsIPrincipal* aSubjectPrincipal,
                                       ErrorResult& aError) override;
   virtual void GetNodeValueInternal(nsAString& aNodeValue) override;
   virtual void SetNodeValueInternal(const nsAString& aNodeValue,
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -408,30 +408,28 @@ nsINode::GetSelectionRootContent(nsIPres
   return content;
 }
 
 nsINodeList*
 nsINode::ChildNodes()
 {
   nsSlots* slots = Slots();
   if (!slots->mChildNodes) {
-    // Check |!IsElement()| first to catch the common case
-    // without virtual call |IsNodeOfType|
-    slots->mChildNodes = !IsElement() && IsNodeOfType(nsINode::eATTRIBUTE) ?
-                           new nsAttrChildContentList(this) :
-                           new nsParentNodeChildContentList(this);
+    slots->mChildNodes = IsAttr()
+      ? new nsAttrChildContentList(this)
+      : new nsParentNodeChildContentList(this);
   }
 
   return slots->mChildNodes;
 }
 
 void
 nsINode::InvalidateChildNodes()
 {
-  MOZ_ASSERT(IsElement() || !IsNodeOfType(nsINode::eATTRIBUTE));
+  MOZ_ASSERT(!IsAttr());
 
   nsSlots* slots = GetExistingSlots();
   if (!slots || !slots->mChildNodes) {
     return;
   }
 
   auto childNodes =
     static_cast<nsParentNodeChildContentList*>(slots->mChildNodes.get());
@@ -736,32 +734,31 @@ nsINode::CompareDocumentPosition(nsINode
   }
   if (GetNextSibling() == &aOtherNode) {
     MOZ_ASSERT(GetParentNode() == aOtherNode.GetParentNode());
     return NodeBinding::DOCUMENT_POSITION_FOLLOWING;
   }
 
   AutoTArray<const nsINode*, 32> parents1, parents2;
 
-  const nsINode *node1 = &aOtherNode, *node2 = this;
+  const nsINode* node1 = &aOtherNode;
+  const nsINode* node2 = this;
 
   // Check if either node is an attribute
-  const Attr* attr1 = nullptr;
-  if (node1->IsNodeOfType(nsINode::eATTRIBUTE)) {
-    attr1 = static_cast<const Attr*>(node1);
+  const Attr* attr1 = Attr::FromNode(node1);
+  if (attr1) {
     const Element* elem = attr1->GetElement();
     // If there is an owner element add the attribute
     // to the chain and walk up to the element
     if (elem) {
       node1 = elem;
       parents1.AppendElement(attr1);
     }
   }
-  if (node2->IsNodeOfType(nsINode::eATTRIBUTE)) {
-    const Attr* attr2 = static_cast<const Attr*>(node2);
+  if (auto* attr2 = Attr::FromNode(node2)) {
     const Element* elem = attr2->GetElement();
     if (elem == node1 && attr1) {
       // Both nodes are attributes on the same element.
       // Compare position between the attributes.
 
       uint32_t i;
       const nsAttrName* attrName;
       for (i = 0; (attrName = elem->GetAttrNameAt(i)); ++i) {
@@ -1290,17 +1287,17 @@ CheckForOutdatedParent(nsINode* aParent,
   }
 }
 
 nsresult
 nsINode::doInsertChildAt(nsIContent* aKid, uint32_t aIndex,
                          bool aNotify, nsAttrAndChildArray& aChildArray)
 {
   MOZ_ASSERT(!aKid->GetParentNode(), "Inserting node that already has parent");
-  MOZ_ASSERT(!IsNodeOfType(nsINode::eATTRIBUTE));
+  MOZ_ASSERT(!IsAttr());
 
   // The id-handling code, and in the future possibly other code, need to
   // react to unexpected attribute changes.
   nsMutationGuard::DidMutate();
 
   // Do this before checking the child-count since this could cause mutations
   nsIDocument* doc = GetUncomposedDoc();
   mozAutoDocUpdate updateBatch(GetComposedDoc(), UPDATE_CONTENT_MODEL, aNotify);
@@ -1644,17 +1641,17 @@ nsINode::doRemoveChildAt(uint32_t aIndex
   // NOTE: This function must not trigger any calls to
   // nsIDocument::GetRootElement() calls until *after* it has removed aKid from
   // aChildArray. Any calls before then could potentially restore a stale
   // value for our cached root element, per note in
   // nsDocument::RemoveChildNode().
   MOZ_ASSERT(aKid && aKid->GetParentNode() == this &&
              aKid == GetChildAt_Deprecated(aIndex) &&
              ComputeIndexOf(aKid) == (int32_t)aIndex, "Bogus aKid");
-  MOZ_ASSERT(!IsNodeOfType(nsINode::eATTRIBUTE));
+  MOZ_ASSERT(!IsAttr());
 
   nsMutationGuard::DidMutate();
   mozAutoDocUpdate updateBatch(GetComposedDoc(), UPDATE_CONTENT_MODEL, aNotify);
 
   nsIContent* previousSibling = aKid->GetPreviousSibling();
 
   if (GetFirstChild() == aKid) {
     mFirstChild = aKid->GetNextSibling();
--- a/dom/base/nsINode.h
+++ b/dom/base/nsINode.h
@@ -407,18 +407,16 @@ public:
 #endif
 
   virtual ~nsINode();
 
   /**
    * Bit-flags to pass (or'ed together) to IsNodeOfType()
    */
   enum {
-    /** nsIAttribute nodes */
-    eATTRIBUTE           = 1 << 2,
     /** form control elements */
     eHTML_FORM_CONTROL   = 1 << 6,
     /** animation elements */
     eANIMATION           = 1 << 10,
     /** filter elements that implement SVGFilterPrimitiveStandardAttributes */
     eFILTER              = 1 << 11,
     /** SVGGeometryElement */
     eSHAPE               = 1 << 12
@@ -576,16 +574,24 @@ public:
   /**
    * Return whether the node is a Comment node.
    */
   bool IsComment() const
   {
     return NodeType() == COMMENT_NODE;
   }
 
+  /**
+   * Return whether the node is an Attr node.
+   */
+  bool IsAttr() const
+  {
+    return NodeType() == ATTRIBUTE_NODE;
+  }
+
   virtual nsIDOMNode* AsDOMNode() = 0;
 
   /**
    * Return if this node has any children.
    */
   bool HasChildren() const { return !!mFirstChild; }
 
   /**
--- a/dom/base/nsNodeUtils.cpp
+++ b/dom/base/nsNodeUtils.cpp
@@ -608,17 +608,17 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNod
     bool ok = aNodesWithProperties->AppendObject(aNode);
     MOZ_RELEASE_ASSERT(ok, "Out of memory");
     if (aClone) {
       ok = aNodesWithProperties->AppendObject(clone);
       MOZ_RELEASE_ASSERT(ok, "Out of memory");
     }
   }
 
-  if (aDeep && (!aClone || !aNode->IsNodeOfType(nsINode::eATTRIBUTE))) {
+  if (aDeep && (!aClone || !aNode->IsAttr())) {
     // aNode's children.
     for (nsIContent* cloneChild = aNode->GetFirstChild();
          cloneChild;
          cloneChild = cloneChild->GetNextSibling()) {
       nsCOMPtr<nsINode> child =
         CloneAndAdopt(cloneChild, aClone, true, nodeInfoManager,
                       aReparentScope, aNodesWithProperties, clone,
                       aError);
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -978,17 +978,17 @@ nsRange::DoSetRange(const RawRangeBounda
                   (aStart.Container()->IsContent() &&
                    aEnd.Container()->IsContent() &&
                    aRoot ==
                     static_cast<nsIContent*>(aStart.Container())->GetBindingParent() &&
                    aRoot ==
                     static_cast<nsIContent*>(aEnd.Container())->GetBindingParent()) ||
                   (!aRoot->GetParentNode() &&
                    (aRoot->IsDocument() ||
-                    aRoot->IsNodeOfType(nsINode::eATTRIBUTE) ||
+                    aRoot->IsAttr() ||
                     aRoot->IsDocumentFragment() ||
                      /*For backward compatibility*/
                     aRoot->IsContent())),
                   "Bad root");
   if (mRoot != aRoot) {
     if (mRoot) {
       mRoot->RemoveMutationObserver(this);
     }
--- a/dom/xslt/xpath/XPathResult.cpp
+++ b/dom/xslt/xpath/XPathResult.cpp
@@ -244,20 +244,18 @@ XPathResult::Invalidate(const nsIContent
         // If context node is in anonymous content, changes to
         // non-anonymous content need to invalidate the XPathResult. If
         // the changes are happening in a different anonymous trees, no
         // invalidation should happen.
         nsIContent* ctxBindingParent = nullptr;
         if (contextNode->IsContent()) {
             ctxBindingParent =
               contextNode->AsContent()->GetBindingParent();
-        } else if (contextNode->IsNodeOfType(nsINode::eATTRIBUTE)) {
-            Element* parent =
-              static_cast<Attr*>(contextNode.get())->GetElement();
-            if (parent) {
+        } else if (auto* attr = Attr::FromNode(contextNode)) {
+            if (Element* parent = attr->GetElement()) {
                 ctxBindingParent = parent->GetBindingParent();
             }
         }
         if (ctxBindingParent != aChangeRoot->GetBindingParent()) {
           return;
         }
     }