Oilpan: move custom element objects to the heap.

Source/core/dom/custom/CustomElementUpgradeCandidateMap.h

Index: Source/core/dom/custom/CustomElementUpgradeCandidateMap.h
diff --git a/Source/core/dom/custom/CustomElementUpgradeCandidateMap.h b/Source/core/dom/custom/CustomElementUpgradeCandidateMap.h
index f68c50b65be76cea7412a881ba3ae7c256da5742..80e343be9b1f445c49ab3aad83a3c39c32e3bed5 100644
--- a/Source/core/dom/custom/CustomElementUpgradeCandidateMap.h
+++ b/Source/core/dom/custom/CustomElementUpgradeCandidateMap.h
@@ -42,31 +42,35 @@ namespace WebCore {
 
 class Element;
 
-class CustomElementUpgradeCandidateMap : CustomElementObserver {
+class CustomElementUpgradeCandidateMap FINAL : public CustomElementObserver {
     WTF_MAKE_NONCOPYABLE(CustomElementUpgradeCandidateMap);
 public:
-    CustomElementUpgradeCandidateMap() { }
-    ~CustomElementUpgradeCandidateMap();
+    static PassOwnPtrWillBeRawPtr<CustomElementUpgradeCandidateMap> create();
+    virtual ~CustomElementUpgradeCandidateMap();
 
     // API for CustomElementRegistrationContext to save and take candidates
 
-    typedef ListHashSet<Element*> ElementSet;
+    typedef WillBeHeapListHashSet<RawPtrWillBeMember<Element> > ElementSet;
 
     void add(const CustomElementDescriptor&, Element*);
     void remove(Element*);
     ElementSet takeUpgradeCandidatesFor(const CustomElementDescriptor&);
 
+    virtual void trace(Visitor*) OVERRIDE;
+
 private:
+    CustomElementUpgradeCandidateMap() { }
+
     virtual void elementWasDestroyed(Element*) OVERRIDE;
     void removeCommon(Element*);
 
     virtual void elementDidFinishParsingChildren(Element*) OVERRIDE;
     void moveToEnd(Element*);
 
-    typedef HashMap<Element*, CustomElementDescriptor> UpgradeCandidateMap;
+    typedef WillBeHeapHashMap<RawPtrWillBeMember<Element>, CustomElementDescriptor> UpgradeCandidateMap;

[haraken, 2014/05/22 08:24:10]

Shouldn't the key Element be a weak member?

[Mads Ager (chromium), 2014/05/22 08:33:27]

Will the m_upgradeCandidates map always end up empty eventually? Otherwise this
looks like it could leak.

The global map from element to custom element observer is:

HashMap<WeakMember<Element>, Member<CustomElementObserver> >

which means that the observer is alive as long as the Element stays alive. Now,
if the CustomElementUpgradeCandidateMap which is a CustomElementObserver keeps
the element alive, we will never collect these.

     UpgradeCandidateMap m_upgradeCandidates;
 
-    typedef HashMap<CustomElementDescriptor, ElementSet> UnresolvedDefinitionMap;
+    typedef WillBeHeapHashMap<CustomElementDescriptor, ElementSet> UnresolvedDefinitionMap;

[haraken, 2014/05/22 08:24:10]

This looks OK. The ElementSet can be a hash set of strong members, because the
entries of the hash map are explicitly cleared.

[wibling-chromium, 2014/05/22 09:06:05]

I cannot find where this is explicitly cleared? The elementWasDestroyed method
is only called when the observed element is destroyed which will not happen as
long as we have this hash map with a member pointing to the element.
Generally speaking shouldn't we have purely weak members in observers to ensure
they don't keep the observee alive?

[sof, 2014/05/25 16:30:01]

Whether or not to use weak references in ElementSet is the same issue as the use
of a weak or strong Element reference for the key in UpgradeCandidateMap.

If we do have to, then we'll have to work out what to use instead of
ListHashSets of weak members :)

     UnresolvedDefinitionMap m_unresolvedDefinitions;
 };