Oilpan: move LiveNodeList collections to the heap.

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 231cd5588662207ca7e845add15c9a715c86605f..8017c9a7f79d589852f3d7e1d9a272acc05eabb0 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -509,8 +509,10 @@ Document::Document(const DocumentInit& initializer, DocumentClassFlags documentC
     initSecurityContext(initializer);
     initDNSPrefetch();
 
+#if !ENABLE(OILPAN)
     for (unsigned i = 0; i < WTF_ARRAY_LENGTH(m_nodeListCounts); i++)
         m_nodeListCounts[i] = 0;
+#endif
 
     InspectorCounters::incrementCounter(InspectorCounters::DocumentCounter);
 
@@ -589,17 +591,17 @@ Document::~Document()
     m_fetcher.clear();
 #endif
 
+#if !ENABLE(OILPAN)

[Mads Ager (chromium), 2014/05/15 10:54:24]

Remove this line and line 592? :)

     // We must call clearRareData() here since a Document class inherits TreeScope
     // as well as Node. See a comment on TreeScope.h for the reason.
-#if !ENABLE(OILPAN)
     if (hasRareData())
         clearRareData();
-#endif
 
-    ASSERT(!m_listsInvalidatedAtDocument.size());
+    ASSERT(m_listsInvalidatedAtDocument.isEmpty());
 
     for (unsigned i = 0; i < WTF_ARRAY_LENGTH(m_nodeListCounts); i++)
         ASSERT(!m_nodeListCounts[i]);
+#endif
 
     setClient(0);
 
@@ -3699,20 +3701,42 @@ void Document::setCSSTarget(Element* n)
 
 void Document::registerNodeList(LiveNodeListBase* list)
 {
+#if ENABLE(OILPAN)
+    m_nodeLists[list->invalidationType()].add(list);
+#else
     m_nodeListCounts[list->invalidationType()]++;
+#endif
     if (list->isRootedAtDocument())
         m_listsInvalidatedAtDocument.add(list);
 }
 
 void Document::unregisterNodeList(LiveNodeListBase* list)
 {
+#if ENABLE(OILPAN)
+    ASSERT(m_nodeLists[list->invalidationType()].contains(WeakMember<LiveNodeListBase>(list)));

[Erik Corry, 2014/05/15 12:34:52]

I think you can write contains(list) instead of
contains(WeakMember<LiveNodeListBase>(list))
At least it works in clang.  Is there some compiler that won't eat that?

There are 5 instances of this pattern.

[sof, 2014/05/15 22:15:57]

Yes, the explicit WeakMember<>s aren't needed, removed them.

The const_cast<>s are needed though, but I'd like to avoid using them. However,
if I change m_nodeLists[i]'s key to be WeakMember<const LiveNodeListBase>, the
isAlive() method in effect isn't adjusting the object pointer as required for
this mixin & it crashes when determining if the weak reference is alive.

I haven't pinned down if this is due to the 'const' getting in the way when
deriving what ObjectAliveTrait to use; will resume the chase tomorrow.

[sof, 2014/05/16 06:12:28]

That is the problem,

  NeedsAdjustAndMark<const LiveNodeListBase>::value == false
  NeedsAdjustAndMark<LiveNodeListBase>::value == true

By redundantly adding "const"s to the underlying type conversion check +
virtually inheriting GarbageCollectedMixin throughout almost fixes it -- 2-3
instances of inaccessible bases compilation errors to contend with.

Don't have a solution for handling those wrt WTF::IsSubclass<> yet (short of
opening up the inheritance to public.)

+    m_nodeLists[list->invalidationType()].remove(WeakMember<LiveNodeListBase>(list));
+#else
     m_nodeListCounts[list->invalidationType()]--;
+#endif
     if (list->isRootedAtDocument()) {
         ASSERT(m_listsInvalidatedAtDocument.contains(list));
         m_listsInvalidatedAtDocument.remove(list);
     }
 }
 
+#if ENABLE(OILPAN)
+void Document::incrementNodeListWithIdNameCacheCount(const LiveNodeListBase* list)
+{
+    m_nodeLists[InvalidateOnIdNameAttrChange].add(WeakMember<LiveNodeListBase>(const_cast<LiveNodeListBase*>(list)));
+}
+
+void Document::decrementNodeListWithIdNameCacheCount(const LiveNodeListBase* list)
+{
+    ASSERT(m_nodeLists[InvalidateOnIdNameAttrChange].contains(WeakMember<LiveNodeListBase>(const_cast<LiveNodeListBase*>(list))));
+    m_nodeLists[InvalidateOnIdNameAttrChange].remove(WeakMember<LiveNodeListBase>(const_cast<LiveNodeListBase*>(list)));
+}
+#endif
+
 void Document::attachNodeIterator(NodeIterator* ni)
 {
     m_nodeIterators.add(ni);
@@ -4516,63 +4540,63 @@ bool Document::hasSVGRootNode() const
     return isSVGSVGElement(documentElement());
 }
 
-PassRefPtr<HTMLCollection> Document::ensureCachedCollection(CollectionType type)
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::ensureCachedCollection(CollectionType type)
 {
     return ensureRareData().ensureNodeLists().addCache<HTMLCollection>(*this, type);
 }
 
-PassRefPtr<HTMLCollection> Document::images()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::images()
 {
     return ensureCachedCollection(DocImages);
 }
 
-PassRefPtr<HTMLCollection> Document::applets()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::applets()
 {
     return ensureCachedCollection(DocApplets);
 }
 
-PassRefPtr<HTMLCollection> Document::embeds()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::embeds()
 {
     return ensureCachedCollection(DocEmbeds);
 }
 
-PassRefPtr<HTMLCollection> Document::scripts()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::scripts()
 {
     return ensureCachedCollection(DocScripts);
 }
 
-PassRefPtr<HTMLCollection> Document::links()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::links()
 {
     return ensureCachedCollection(DocLinks);
 }
 
-PassRefPtr<HTMLCollection> Document::forms()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::forms()
 {
     return ensureCachedCollection(DocForms);
 }
 
-PassRefPtr<HTMLCollection> Document::anchors()
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::anchors()
 {
     return ensureCachedCollection(DocAnchors);
 }
 
-PassRefPtr<HTMLAllCollection> Document::allForBinding()
+PassRefPtrWillBeRawPtr<HTMLAllCollection> Document::allForBinding()
 {
     UseCounter::count(*this, UseCounter::DocumentAll);
     return all();
 }
 
-PassRefPtr<HTMLAllCollection> Document::all()
+PassRefPtrWillBeRawPtr<HTMLAllCollection> Document::all()
 {
     return ensureRareData().ensureNodeLists().addCache<HTMLAllCollection>(*this, DocAll);
 }
 
-PassRefPtr<HTMLCollection> Document::windowNamedItems(const AtomicString& name)
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::windowNamedItems(const AtomicString& name)
 {
     return ensureRareData().ensureNodeLists().addCache<HTMLNameCollection>(*this, WindowNamedItems, name);
 }
 
-PassRefPtr<HTMLCollection> Document::documentNamedItems(const AtomicString& name)
+PassRefPtrWillBeRawPtr<HTMLCollection> Document::documentNamedItems(const AtomicString& name)
 {
     return ensureRareData().ensureNodeLists().addCache<HTMLNameCollection>(*this, DocumentNamedItems, name);
 }
@@ -5650,6 +5674,21 @@ bool Document::hasFocus() const
     return false;
 }
 
+#if ENABLE(OILPAN)
+template<unsigned type>
+bool shouldInvalidateNodeListCachesForAttr(const HeapHashSet<WeakMember<LiveNodeListBase> > nodeLists[], const QualifiedName& attrName)
+{
+    if (!nodeLists[type].isEmpty() && LiveNodeListBase::shouldInvalidateTypeOnAttributeChange(static_cast<NodeListInvalidationType>(type), attrName))
+        return true;
+    return shouldInvalidateNodeListCachesForAttr<type + 1>(nodeLists, attrName);
+}
+
+template<>
+bool shouldInvalidateNodeListCachesForAttr<numNodeListInvalidationTypes>(const HeapHashSet<WeakMember<LiveNodeListBase> >[], const QualifiedName&)
+{
+    return false;
+}
+#else
 template<unsigned type>
 bool shouldInvalidateNodeListCachesForAttr(const unsigned nodeListCounts[], const QualifiedName& attrName)
 {
@@ -5663,14 +5702,24 @@ bool shouldInvalidateNodeListCachesForAttr<numNodeListInvalidationTypes>(const u
 {
     return false;
 }
+#endif
 
 bool Document::shouldInvalidateNodeListCaches(const QualifiedName* attrName) const
 {
-    if (attrName)
+    if (attrName) {
+#if ENABLE(OILPAN)
+        return shouldInvalidateNodeListCachesForAttr<DoNotInvalidateOnAttributeChanges + 1>(m_nodeLists, *attrName);

[Erik Corry, 2014/05/15 12:34:52]

Looks like all these templated functions are just a wildly complicated way to
write

for (unsigned i = DoNotInvalidateOnAttributeChanges + 1; i <
numNodeListInvalidationTypes; ++i) {
    if (!nodeLists[i].isEmpty() && shouldInvalidateTypeOnAttributeChange(i,
attrName))
        return true;
}
return false;

Probably you need to add some casts.  The templated version causes us to
generate a function per NodeListInvalidationType and recurse instead of
iterating.

[sof, 2014/05/15 22:15:57]

Alright, took that on here as well, removing the template expansion over the
enum for a simpler loop.

+#else
         return shouldInvalidateNodeListCachesForAttr<DoNotInvalidateOnAttributeChanges + 1>(m_nodeListCounts, *attrName);
+#endif
+    }
 
     for (int type = 0; type < numNodeListInvalidationTypes; type++) {
+#if ENABLE(OILPAN)
+        if (!m_nodeLists[type].isEmpty())
+#else
         if (m_nodeListCounts[type])
+#endif
             return true;
     }
 
@@ -5679,8 +5728,8 @@ bool Document::shouldInvalidateNodeListCaches(const QualifiedName* attrName) con
 
 void Document::invalidateNodeListCaches(const QualifiedName* attrName)
 {
-    HashSet<LiveNodeListBase*>::iterator end = m_listsInvalidatedAtDocument.end();
-    for (HashSet<LiveNodeListBase*>::iterator it = m_listsInvalidatedAtDocument.begin(); it != end; ++it)
+    WillBeHeapHashSet<RawPtrWillBeWeakMember<LiveNodeListBase> >::iterator end = m_listsInvalidatedAtDocument.end();
+    for (WillBeHeapHashSet<RawPtrWillBeWeakMember<LiveNodeListBase> >::iterator it = m_listsInvalidatedAtDocument.begin(); it != end; ++it)
         (*it)->invalidateCacheForAttribute(attrName);
 }
 
@@ -5716,6 +5765,11 @@ void Document::trace(Visitor* visitor)
     visitor->trace(m_titleElement);
     visitor->trace(m_currentScriptStack);
     visitor->trace(m_transformSourceDocument);
+    visitor->trace(m_listsInvalidatedAtDocument);
+#if ENABLE(OILPAN)
+    for (int i = 0; i < numNodeListInvalidationTypes; i++)
+        visitor->trace(m_nodeLists[i]);
+#endif
     visitor->trace(m_cssCanvasElements);
     visitor->trace(m_topLayerElements);
     visitor->trace(m_elemSheet);