Oilpan: Prepare to move ImageLoader and its subclasses to Oilpan heap.

Source/core/loader/ImageLoader.h

Index: Source/core/loader/ImageLoader.h
diff --git a/Source/core/loader/ImageLoader.h b/Source/core/loader/ImageLoader.h
index 1e0cdc426af7bc28729f3a8821b069c885551461..9ab71e6b77e164952294926d71122f5ff50858a4 100644
--- a/Source/core/loader/ImageLoader.h
+++ b/Source/core/loader/ImageLoader.h
@@ -55,10 +55,11 @@ class RenderImageResource;
 template<typename T> class EventSender;
 typedef EventSender<ImageLoader> ImageEventSender;
 
-class ImageLoader : public ImageResourceClient {
+class ImageLoader : public NoBaseWillBeGarbageCollectedFinalized<ImageLoader>, public ImageResourceClient {
 public:
     explicit ImageLoader(Element*);
     virtual ~ImageLoader();
+    void trace(Visitor*);
 
     enum LoadType {
         LoadNormally,
@@ -133,8 +134,12 @@ private:
 
     void willRemoveClient(ImageLoaderClient&);
 
-    Element* m_element;
+    RawPtrWillBeMember<Element> m_element;
     ResourcePtr<ImageResource> m_image;
+    // FIXME: Oilpan: We might be able to remove this Persistent hack when
+    // ImageResourceClient is traceable.
+    GC_PLUGIN_IGNORE("http://crbug.com/353083")
+    RefPtrWillBePersistent<Element> m_keepAlive;
 #if ENABLE(OILPAN)
     class ImageLoaderClientRemover {
     public:
@@ -146,6 +151,9 @@ private:
         ImageLoaderClient& m_client;
     };
     friend class ImageLoaderClientRemover;
+    // Oilpan: This ImageLoader object must outlive its clients because they
+    // need to call ImageLoader::willRemoveClient before they die.
+    GC_PLUGIN_IGNORE("http://crbug.com/353083")
     PersistentHeapHashMap<WeakMember<ImageLoaderClient>, OwnPtr<ImageLoaderClientRemover> > m_clients;

[haraken, 2014/06/10 04:15:13]

I'm curious why you need to make this map a persistent. Even if you don't make
this map a persistent, it will be guaranteed that the ImageLoader is alive when
~ImageLoaderClientRemover touches the ImageLoader.

(1) An ImageLoaderClient and an ImageLoader become unreachable.
(2) A GC is triggered.
(3) The ImageLoaderClient is removed from the map by oilpan's weak processing.
~ImageLoaderClientRemover is called. It can touche the ImageLoader, because the
ImageLoader is not yet swept.
(4) The ImageLoader is swept.

[tkent, 2014/06/10 05:32:46]

Though I don't remember the precise mechanism, we need to make this
Persistent-in-GarbageCollectedFinalized<> in this case.  Making this Member<>
causes crash in ImageBitmapTest.ImageBitmapLiveResourcePriority.
Please see modules/webdatabse/DatabaseSync.h. 
modules/webdatabase/DatabseContext.h has non-persistent weak HashMap.  It's ok
because DatabaseCloser doesn't touch the DatabaseContext.

I guess removing weak HashMap entries is done in the weak processing, but
finalizing weak HashMap entries is done in the sweep stage?

[haraken, 2014/06/10 05:54:10]

Given that ImageLoaderClientRemover is off heap, ~ImageLoaderClientRemover
should be called promptly when the OwnPtr is removed from the hash map in the
weak processing phase (which happens before the sweeping phase).

Help me understand: Does the ImageLoaderClient have a Member back to the
ImageLoader? (It was not clear from a quick grep.) If the ImageLoaderClient has
a Member back to the ImageLoader, this change makes sense because the Member
guarantees that the ImageLoader is destructed in the next GC of the GC that
destructed the ImageLoaderClient.

Would it be possible to make this hash map a static hash map? Unlike 
DatabaseContext, I don't see any threading issue here.

 #else
     HashSet<ImageLoaderClient*> m_clients;