Get rid of unnecessary isChildNodeList() virtual call in clearChildNodeListCache()

Source/core/dom/NodeListsNodeData.h

 /*
  * Copyright (C) 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2008 David Smith <catfish.man@gmail.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 21 matching lines @@
 #include "wtf/text/StringHash.h"
 
 namespace blink {
 
 class NodeListsNodeData FINAL : public NoBaseWillBeGarbageCollectedFinalized<NodeListsNodeData> {
     WTF_MAKE_NONCOPYABLE(NodeListsNodeData);
     WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
 public:
     void clearChildNodeListCache()
     {
-        if (m_childNodeList && m_childNodeList->isChildNodeList())
+        if (m_childNodeList) {
+            // This can only be called from ContainerNode so casting to ChildNodeList

[adamk, 2014/07/24 22:06:20]

This seems a bit unsafe, since there's nothing (except maybe an ASSERT in
toChildNodeList) stopping this from getting called on a Node.

Is this a performance win?

[Inactive, 2014/07/25 00:09:50]

Yes, there is an assertion in the casting function but I understand your
concern. Before I give up on this, I'd like to make a counter proposal:
- Remove the NodeListsNodeData::clearChildNodeListCache() method
- Add a NodeList* NodeListsNodeData::childNodeList() getter instead
- From ContainerNode::invalidateNodeListCachesInAncestors(), call
toChildNodeList(nodeLists->childNodeList())->invalidateCache()

Since we are in a ContainerNode method, the container node would look safer.
Loosing the clearChildNodeListCache() method does not seem important as there is
only 1 caller.

Maybe even better, the getter could even look like:
ChildNodeList* childNodeList(ContainerNode&);
// similarly to ensureChildNodeList(ContainerNode&) below.

What do you think?
I have just run Dromaeo but it did not show any perf win. I introduced this
virtual function call a while back and was not entirely happy with it, which is
why I was revisiting it.

+            // is safe.
             toChildNodeList(m_childNodeList)->invalidateCache();
+        }
     }
 
     PassRefPtrWillBeRawPtr<ChildNodeList> ensureChildNodeList(ContainerNode& node)
     {
         if (m_childNodeList)
             return toChildNodeList(m_childNodeList);
         RefPtrWillBeRawPtr<ChildNodeList> list = ChildNodeList::create(node);
         m_childNodeList = list.get();
         return list.release();
     }
@@ skipping 178 matching lines @@
     if ((m_childNodeList ? 1 : 0) + m_atomicNameCaches.size() + m_tagCollectionCacheNS.size() != 1)
         return false;
     ownerNode.clearNodeLists();
     return true;
 }
 #endif
 
 } // namespace blink
 
 #endif // NodeListsNodeData_h