Have GeolocationServiceImpl refer to RFHI via a weak pointer

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 9a8810b3aa9a45c2ec8424f30fbbff6800fe5421..edddbc940f08e5f7552b5126d1f7891588a92449 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -1696,11 +1696,17 @@ void RenderFrameHostImpl::RegisterMojoServices() {
     // TODO(creis): Bind process ID here so that GeolocationServiceImpl
     // can perform permissions checks once site isolation is complete.
     // crbug.com/426384
+    // NOTE: At shutdown, there is no guaranteed ordering between destruction of
+    // this object and destruction of any GeolocationServicesImpls created via
+    // the below service registry, the reason being that the destruction of the
+    // latter is triggered by receiving a message that the pipe was closed from
+    // the renderer side. Hence, supply the reference to this object as a weak
+    // pointer.

[Michael van Ouwerkerk, 2016/02/18 09:37:57]

Nit: maybe also refer to crbug.com/556749 in this comment.

[blundell, 2016/02/26 12:34:12]

I think the comment is sufficient to explain the issue as-is, but happy to
follow up if you feel strongly.

     GetServiceRegistry()->AddService<GeolocationService>(
         base::Bind(&GeolocationServiceContext::CreateService,
                    base::Unretained(geolocation_service_context),
                    base::Bind(&RenderFrameHostImpl::DidUseGeolocationPermission,
-                              base::Unretained(this))));
+                              weak_ptr_factory_.GetWeakPtr())));

[ncarter (slow), 2016/02/19 00:44:36]

The RenderFrameHostImpl owns the ServiceRegistry, so I am a bit surprised to
learn that that this callback, which we hand to the ServiceRegistry, can be
invoked after we've destroyed the ServiceRegistry.

So what's actually going on here? Are we sure this isn't a bug in the
ServiceRegistry?

[ncarter (slow), 2016/02/19 23:09:44]

I looked at this a little more, and am inferring that it's not the
ServiceRegistry that is outliving the RFH, but the inner callback here, which is
retained by the GeolocationServiceImpl, which is what actually winds up
outliving the RFH.

In that case the WeakPtr here seems fine -- so lgtm in the sense that this fixes
a UaF.

I do wish there was a way to actively kill off the services that so that they
couldn't outlive their associated RenderFrameHost or WebContents, and breed this
kind of UaF possibility. Without a live RFH we really ought not to be allowing
any kind of IPCs in (see also: https://codereview.chromium.org/1534933002/ and
its associated bugs)

[blundell, 2016/02/26 12:32:50]

Correct.
https://codereview.chromium.org/1411063007 is related, although I don't think it
would be a complete silver bullet.

   }
 
   WakeLockServiceContext* wake_lock_service_context =