Dispatch geolocation IPCs on the UI thread. Aside from simplifying the code to avoid a lot of threa…

android_webview/native/aw_geolocation_permission_context.cc

Index: android_webview/native/aw_geolocation_permission_context.cc
===================================================================
--- android_webview/native/aw_geolocation_permission_context.cc	(revision 269041)
+++ android_webview/native/aw_geolocation_permission_context.cc	(working copy)
@@ -16,18 +16,13 @@
 AwGeolocationPermissionContext::~AwGeolocationPermissionContext() {
 }
 
-void
-AwGeolocationPermissionContext::RequestGeolocationPermissionOnUIThread(
-    int render_process_id,
-    int render_view_id,
+void AwGeolocationPermissionContext::RequestGeolocationPermission(
+    content::WebContents* web_contents,
     int bridge_id,
     const GURL& requesting_frame,
     bool user_gesture,
     base::Callback<void(bool)> callback) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

[benm (inactive), 2014/05/08 15:10:47]

can we keep this DCHECK?

[jam, 2014/05/08 15:54:32]

i had removed it for two reasons:
-i prefer embedders of content don't check that content is calling on the right
thread; these kind of checks should be done in content and not at every embedder
-the web_contents parameter now should make it obvious that this is called on
the UI thread

I think a better check would be in AwContents::FromID? I added one there
(hopefully it doesn't trigger!). I also removed the one in AwContents::FromID
since RenderViewHost::FromID checks that.

I defer to you since you're an owner there, if you're not convinced :)

-
-  AwContents* aw_contents =
-      AwContents::FromID(render_process_id, render_view_id);
+  AwContents* aw_contents = AwContents::FromWebContents(web_contents);
   if (!aw_contents) {
     callback.Run(false);
     return;
@@ -35,65 +30,20 @@
   aw_contents->ShowGeolocationPrompt(requesting_frame, callback);
 }
 
-void
-AwGeolocationPermissionContext::RequestGeolocationPermission(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id,
-    const GURL& requesting_frame,
-    bool user_gesture,
-    base::Callback<void(bool)> callback) {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &AwGeolocationPermissionContext::
-              RequestGeolocationPermissionOnUIThread,
-          this,
-          render_process_id,
-          render_view_id,
-          bridge_id,
-          requesting_frame,
-          user_gesture,
-          callback));
-}
-
 // static
 content::GeolocationPermissionContext*
 AwGeolocationPermissionContext::Create(AwBrowserContext* browser_context) {
   return new AwGeolocationPermissionContext();
 }
 
-void
-AwGeolocationPermissionContext::CancelGeolocationPermissionRequestOnUIThread(
-    int render_process_id,
-    int render_view_id,
+void AwGeolocationPermissionContext::CancelGeolocationPermissionRequest(
+    content::WebContents* web_contents,
     int bridge_id,
     const GURL& requesting_frame) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));

[benm (inactive), 2014/05/08 15:10:47]

ditto, can we keep this?

-
-  AwContents* aw_contents =
-      AwContents::FromID(render_process_id, render_view_id);
+  AwContents* aw_contents = AwContents::FromWebContents(web_contents);
   if (aw_contents) {
     aw_contents->HideGeolocationPrompt(requesting_frame);
   }
 }
 
-void
-AwGeolocationPermissionContext::CancelGeolocationPermissionRequest(
-    int render_process_id,
-    int render_view_id,
-    int bridge_id,
-    const GURL& requesting_frame) {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &AwGeolocationPermissionContext::
-              CancelGeolocationPermissionRequestOnUIThread,
-          this,
-          render_process_id,
-          render_view_id,
-          bridge_id,
-          requesting_frame));
-}
-
 }  // namespace android_webview