Oilpan: detach GeolocationController on being destroyed as a supplement.

Oilpan: detach GeolocationController on being destroyed as a supplement.

If the LocalFrame supplementable signals destruction, detach from the
inspector agent and clear out the GeolocationClient reference after
having relayed the destruction notification.

Not doing both risks the inspector agent object accessing a what's
now effectively a dead object (with a dead client reference; for safety
we clear that out also.)

R=haraken@chromium.org
BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176692

[sof, 2014-06-20 20:23:33]

Please take a look.

[haraken, 2014-06-21 02:19:53]

LGTM

https://codereview.chromium.org/337053006/diff/1/Source/modules/geolocation/G...
File Source/modules/geolocation/GeolocationController.cpp (right):

https://codereview.chromium.org/337053006/diff/1/Source/modules/geolocation/G...
Source/modules/geolocation/GeolocationController.cpp:92:
m_inspectorAgent->RemoveController(this);

keishi@: I think you need to remove this code when moving the InspectorAgent to
the heap. I think you can remove the code by making
GeolocationInspectorAgent::m_controllers a hash set of weak members.

https://codereview.chromium.org/337053006/diff/1/Source/modules/geolocation/G...
Source/modules/geolocation/GeolocationController.cpp:102: // FIXME: Oilpan: Once
GeolocationClient is on-heap m_client should be a strong

We might want to consider moving GeolocationClient to the heap. Once the
GeolocationInspectorAgent and the GeolocationClient are moved to the heap, we
can make the destructor empty.

https://codereview.chromium.org/337053006/diff/1/Source/modules/geolocation/G...
File Source/modules/geolocation/GeolocationController.h (right):

https://codereview.chromium.org/337053006/diff/1/Source/modules/geolocation/G...
Source/modules/geolocation/GeolocationController.h:94:
GeolocationInspectorAgent* m_inspectorAgent;

keishi@: You're moving the GeolocatoinInspectorAgent to the heap in
https://codereview.chromium.org/307943002/. This means that:

- You need to change m_inspectorAgent a Member.
- You need to remove the code that touches m_inspectorAgent from
GeolocatoinInspectorAgent's destructor. (Doesn't ASAN detect the error?)

[haraken, 2014-06-21 02:20:09]

+keishi

[sof, 2014-06-21 12:07:28]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-06-21 12:07:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/337053006/1

[commit-bot: I haz the power, 2014-06-21 13:32:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-06-21 14:13:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-21 14:13:59]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[sof, 2014-06-21 20:27:31]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-06-21 20:27:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/337053006/1

[commit-bot: I haz the power, 2014-06-21 21:04:05]

Change committed as 176692

[wibling-chromium, 2014-06-23 07:29:10]

lgtm