Bug 1214857. Store the document-is-HTML state directly in nsContentList instead of refetching from the node being matched. r=smaug

dom/base/nsContentList.cpp

@@ -198,7 +198,8 @@ NS_GetContentList(nsINode* aRootNode,
   NS_ASSERTION(aRootNode, "content list has to have a root");
 
   nsRefPtr<nsContentList> list;
-  nsContentListKey hashKey(aRootNode, aMatchNameSpaceId, aTagname);
+  nsContentListKey hashKey(aRootNode, aMatchNameSpaceId, aTagname,
+                           aRootNode->OwnerDoc()->IsHTMLDocument());
   uint32_t recentlyUsedCacheIndex = RecentlyUsedCacheIndex(hashKey);
   nsContentList* cachedList = sRecentlyUsedContentLists[recentlyUsedCacheIndex];
   if (cachedList && cachedList->MatchesKey(hashKey)) {
@@ -398,7 +399,8 @@ nsContentList::nsContentList(nsINode* aRootNode,
     mData(nullptr),
     mState(LIST_DIRTY),
     mDeep(aDeep),
-    mFuncMayDependOnAttr(false)
+    mFuncMayDependOnAttr(false),
+    mIsHTMLDocument(aRootNode->OwnerDoc()->IsHTMLDocument())
 {
   NS_ASSERTION(mRootNode, "Must have root");
   if (nsGkAtoms::_asterisk == mHTMLMatchAtom) {
@@ -438,7 +440,8 @@ nsContentList::nsContentList(nsINode* aRootNode,
     mState(LIST_DIRTY),
     mMatchAll(false),
     mDeep(aDeep),
-    mFuncMayDependOnAttr(aFuncMayDependOnAttr)
+    mFuncMayDependOnAttr(aFuncMayDependOnAttr),
+    mIsHTMLDocument(false)
 {
   NS_ASSERTION(mRootNode, "Must have root");
   mRootNode->AddMutationObserver(this);
@@ -839,7 +842,7 @@ nsContentList::Match(Element *aElement)
   if (!mXMLMatchAtom)
     return false;
 
-  mozilla::dom::NodeInfo *ni = aElement->NodeInfo();
+  NodeInfo *ni = aElement->NodeInfo();
 
   bool wildcard = mMatchNameSpaceId == kNameSpaceID_Wildcard ||
                   mMatchNameSpaceId == kNameSpaceID_Unknown;
@@ -850,9 +853,9 @@ nsContentList::Match(Element *aElement)
   if (toReturn)
     return toReturn;
 
-  bool matchHTML = aElement->GetNameSpaceID() == kNameSpaceID_XHTML &&
-    aElement->OwnerDoc()->IsHTMLDocument();
- 
+  bool matchHTML =
+    mIsHTMLDocument && aElement->GetNameSpaceID() == kNameSpaceID_XHTML;
+
   if (wildcard) {
     return matchHTML ? ni->Equals(mHTMLMatchAtom) :
                        ni->Equals(mXMLMatchAtom);
@@ -957,7 +960,7 @@ nsContentList::RemoveFromHashtable()
   }
   
   nsDependentAtomString str(mXMLMatchAtom);
-  nsContentListKey key(mRootNode, mMatchNameSpaceId, str);
+  nsContentListKey key(mRootNode, mMatchNameSpaceId, str, mIsHTMLDocument);
   uint32_t recentlyUsedCacheIndex = RecentlyUsedCacheIndex(key);
   if (sRecentlyUsedContentLists[recentlyUsedCacheIndex] == this) {
     sRecentlyUsedContentLists[recentlyUsedCacheIndex] = nullptr;

dom/base/nsContentList.h

@@ -142,14 +142,21 @@ private:
  */
 struct nsContentListKey
 {
+  // We have to take an aIsHTMLDocument arg for two reasons:
+  // 1) We don't want to include nsIDocument.h in this header.
+  // 2) We need to do that to make nsContentList::RemoveFromHashtable
+  //    work, because by the time it's called the document of the
+  //    list's root node might have changed.
   nsContentListKey(nsINode* aRootNode,
                    int32_t aMatchNameSpaceId,
-                   const nsAString& aTagname)
+                   const nsAString& aTagname,
+                   bool aIsHTMLDocument)
     : mRootNode(aRootNode),
       mMatchNameSpaceId(aMatchNameSpaceId),
       mTagname(aTagname),
+      mIsHTMLDocument(aIsHTMLDocument),
       mHash(mozilla::AddToHash(mozilla::HashString(aTagname), mRootNode,
-                               mMatchNameSpaceId))
+                               mMatchNameSpaceId, mIsHTMLDocument))
   {
   }
 
@@ -157,6 +164,7 @@ struct nsContentListKey
     : mRootNode(aContentListKey.mRootNode),
       mMatchNameSpaceId(aContentListKey.mMatchNameSpaceId),
       mTagname(aContentListKey.mTagname),
+      mIsHTMLDocument(aContentListKey.mIsHTMLDocument),
       mHash(aContentListKey.mHash)
   {
   }
@@ -169,6 +177,7 @@ struct nsContentListKey
   nsINode* const mRootNode; // Weak ref
   const int32_t mMatchNameSpaceId;
   const nsAString& mTagname;
+  bool mIsHTMLDocument;
   const uint32_t mHash;
 };
 
@@ -319,13 +328,15 @@ public:
   {
     // The root node is most commonly the same: the document.  And the
     // most common namespace id is kNameSpaceID_Unknown.  So check the
-    // string first.
+    // string first.  Cases in which whether our root's ownerDocument
+    // is HTML changes are extremely rare, so check those last.
     NS_PRECONDITION(mXMLMatchAtom,
                     "How did we get here with a null match atom on our list?");
     return
       mXMLMatchAtom->Equals(aKey.mTagname) &&
       mRootNode == aKey.mRootNode &&
-      mMatchNameSpaceId == aKey.mMatchNameSpaceId;
+      mMatchNameSpaceId == aKey.mMatchNameSpaceId &&
+      mIsHTMLDocument == aKey.mIsHTMLDocument;
   }
 
   /**
@@ -446,6 +457,12 @@ protected:
    * Whether we actually need to flush to get our state correct.
    */
   uint8_t mFlushesNeeded : 1;
+  /**
+   * Whether the ownerDocument of our root node at list creation time was an
+   * HTML document.  Only needed when we're doing a namespace/atom match, not
+   * when doing function matching, always false otherwise.
+   */
+  uint8_t mIsHTMLDocument : 1;
 
 #ifdef DEBUG_CONTENT_LIST
   void AssertInSync();

testing/web-platform/meta/MANIFEST.json

@@ -29219,6 +29219,12 @@
     "deleted": [],
     "items": {
       "testharness": {
+        "dom/nodes/Element-getElementsByTagName-change-document-HTMLNess.html": [
+          {
+            "path": "dom/nodes/Element-getElementsByTagName-change-document-HTMLNess.html",
+            "url": "/dom/nodes/Element-getElementsByTagName-change-document-HTMLNess.html"
+          }
+        ],
         "webaudio/the-audio-api/the-audioparam-interface/retrospective-setValueAtTime.html": [
           {
             "path": "webaudio/the-audio-api/the-audioparam-interface/retrospective-setValueAtTime.html",

testing/web-platform/tests/dom/nodes/Element-getElementsByTagName-change-document-HTMLNess-iframe.xml

@@ -0,0 +1 @@
+<root/>

testing/web-platform/tests/dom/nodes/Element-getElementsByTagName-change-document-HTMLNess.html

@@ -0,0 +1,50 @@
+<!doctype html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<iframe src="Element-getElementsByTagName-change-document-HTMLNess-iframe.xml"></iframe>
+<script>
+  onload = function() {
+    var parent = document.createElement("div");
+    var child1 = document.createElementNS("http://www.w3.org/1999/xhtml", "a");
+    child1.textContent = "xhtml:a";
+    var child2 = document.createElementNS("http://www.w3.org/1999/xhtml", "A");
+    child2.textContent = "xhtml:A";
+    var child3 = document.createElementNS("", "a");
+    child3.textContent = "a";
+    var child4 = document.createElementNS("", "A");
+    child4.textContent = "A";
+
+    parent.appendChild(child1);
+    parent.appendChild(child2);
+    parent.appendChild(child3);
+    parent.appendChild(child4);
+
+    var list = parent.getElementsByTagName("A");
+    assert_array_equals(list, [child1, child4],
+      "In an HTML document, should lowercase the tagname passed in for HTML " +
+      "elements only");
+
+    frames[0].document.documentElement.appendChild(parent);
+    assert_array_equals(list, [child1, child4],
+      "After changing document, should still be lowercasing for HTML");
+
+    assert_array_equals(parent.getElementsByTagName("A"),
+                        [child2, child4],
+      "New list with same root and argument should not be lowercasing now");
+
+    // Now reinsert all those nodes into the parent, to blow away caches.
+    parent.appendChild(child1);
+    parent.appendChild(child2);
+    parent.appendChild(child3);
+    parent.appendChild(child4);
+    assert_array_equals(list, [child1, child4],
+      "After blowing away caches, should still have the same list");
+
+    assert_array_equals(parent.getElementsByTagName("A"),
+                        [child2, child4],
+      "New list with same root and argument should still not be lowercasing");
+    done();
+  }
+</script>