Oilpan: move custom element objects to the heap.

Source/core/dom/custom/CustomElementUpgradeCandidateMap.h

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 24 matching lines @@
 #include "core/dom/custom/CustomElementDescriptorHash.h"
 #include "core/dom/custom/CustomElementObserver.h"
 #include "wtf/HashMap.h"
 #include "wtf/ListHashSet.h"
 #include "wtf/Noncopyable.h"
 
 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;
 };
 
 }
 
 #endif // CustomElementUpgradeCandidateMap_h