Oilpan: Move all supplements of Page, Document, and WorkerClients to the managed heap.

Source/modules/geolocation/GeolocationController.h

Index: Source/modules/geolocation/GeolocationController.h
diff --git a/Source/modules/geolocation/GeolocationController.h b/Source/modules/geolocation/GeolocationController.h
index 118d042a6af54d4f582d8d175395ac7092f1140b..c868ba16d95e6b2f5823a41f9e313a33b3cf426c 100644
--- a/Source/modules/geolocation/GeolocationController.h
+++ b/Source/modules/geolocation/GeolocationController.h
@@ -42,12 +42,13 @@ class GeolocationError;
 class GeolocationPosition;
 class Page;
 
-class GeolocationController FINAL : public Supplement<Page>, public PageLifecycleObserver {
+class GeolocationController FINAL : public NoBaseWillBeGarbageCollectedFinalized<GeolocationController>, public WillBeHeapSupplement<Page>, public PageLifecycleObserver {
+    WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(GeolocationController);
     WTF_MAKE_NONCOPYABLE(GeolocationController);
 public:
     virtual ~GeolocationController();
 
-    static PassOwnPtr<GeolocationController> create(Page&, GeolocationClient*);
+    static PassOwnPtrWillBeRawPtr<GeolocationController> create(Page&, GeolocationClient*);
 
     void addObserver(Geolocation*, bool enableHighAccuracy);
     void removeObserver(Geolocation*);
@@ -68,9 +69,9 @@ public:
     virtual void pageVisibilityChanged() OVERRIDE;
 
     static const char* supplementName();
-    static GeolocationController* from(Page* page) { return static_cast<GeolocationController*>(Supplement<Page>::from(page, supplementName())); }
+    static GeolocationController* from(Page* page) { return static_cast<GeolocationController*>(WillBeHeapSupplement<Page>::from(page, supplementName())); }
 
-    virtual void trace(Visitor*) OVERRIDE { };
+    virtual void trace(Visitor*) OVERRIDE;
 
     virtual void willBeDestroyed() OVERRIDE;
 
@@ -83,8 +84,8 @@ private:
     GeolocationClient* m_client;

[haraken, 2014/04/30 02:41:22]

Ditto. I wonder how it's guaranteed that the client does not outlive the
GeolocationController.

[zerny-chromium, 2014/04/30 08:29:34]

The opposite needs to hold. The client should always outlive the controller
since the controller has a raw pointer to the client. The client appears to be a
proxy (eg GeolocationClientProxy) which points to a 'web' client (eg
WebGeolocationClient). I'm not sure about the lifetime of those two, but they
are unaffected by this CL.

[haraken, 2014/04/30 11:18:08]

This raw pointer happens to be safe because WebViewImpl owns both Page and
GeolocationClient, and thus they die together.

However, "they are unaffected by this CL" is not true, because this CL changes
the lifetime of the Page. Previously the Page was dead promptly when the OwnPtr
to the Page is gone. After this CL, the Page does not die until the next GC is
triggered. Thus it's possible that the client can outlive the Page.

In general, we need to call m_client->willBeDestroyed() when
Page::willBeDestroyed() is called and clear out the m_client. See what
Page::willBeDestroyed() is doing for clients of the Page. (Of course, if there
is nothing to do in m_client->willBeDestroyed(), we don't need to call it.)

I think Mads is familiar with this issue since he addressed the client issues
when moving Page to the heap.

[haraken, 2014/04/30 11:19:58]

Note: I haven't yet checked if other m_client pointers in this CL are safe.

[zerny-chromium, 2014/04/30 11:46:53]

Page is already moved. This CL does not change its lifetime.

[haraken, 2014/04/30 12:02:08]

Sorry, I was misunderstanding! The supplementable objects are already moved to
the heap, so this CL doesn't change any lifetime. Thus the CL doesn't produce
any new issues.

     bool m_hasClientForTest;
 
-    RefPtrWillBePersistent<GeolocationPosition> m_lastPosition;
-    typedef WillBePersistentHeapHashSet<RefPtrWillBeMember<Geolocation> > ObserversSet;
+    RefPtrWillBeMember<GeolocationPosition> m_lastPosition;
+    typedef WillBeHeapHashSet<RefPtrWillBeMember<Geolocation> > ObserversSet;
     // All observers; both those requesting high accuracy and those not.
     ObserversSet m_observers;
     ObserversSet m_highAccuracyObservers;