Implementing geolocation for the Android Webview

android_webview/native/aw_contents.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "android_webview/native/aw_contents.h"
 
 #include <android/bitmap.h>
 #include <sys/system_properties.h>
 
 #include "android_webview/browser/aw_browser_context.h"
@@ skipping 709 matching lines @@
                  jobject obj,
                  jobject web_contents_delegate) {
   AwContents* tab = new AwContents(env, obj, web_contents_delegate);
   return reinterpret_cast<jint>(tab);
 }
 
 bool RegisterAwContents(JNIEnv* env) {
   return RegisterNativesImpl(env) >= 0;
 }
 
-void AwContents::OnGeolocationShowPrompt(int render_process_id,
-                                       int render_view_id,
-                                       int bridge_id,
-                                       const GURL& requesting_frame) {
+namespace {
+
+void GeolocationShowPromptTask(JavaObjectWeakGlobalRef java_ref,
+                               const GURL& origin) {
+  LOG(INFO) << "Running task geo";

[benm (inactive), 2013/02/12 10:46:30]

nit: remove logging?

[Kristian Monsen, 2013/02/12 18:53:03]

Done.

   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jstring> j_requesting_frame(
-      ConvertUTF8ToJavaString(env, requesting_frame.spec()));
+  ScopedJavaLocalRef<jstring> j_origin(
+      ConvertUTF8ToJavaString(env, origin.spec()));
   Java_AwContents_onGeolocationPermissionsShowPrompt(env,
-      java_ref_.get(env).obj(), render_process_id, render_view_id, bridge_id,
-      j_requesting_frame.obj());
+                                                     java_ref.get(env).obj(),

[benm (inactive), 2013/02/12 10:46:30]

This will NPE if java_ref has been collected.

[Kristian Monsen, 2013/02/12 18:53:03]

Done.

+                                                     j_origin.obj());
 }
 
-void AwContents::OnGeolocationHidePrompt() {
-  // TODO(kristianm): Implement this
+void GeolocationShowPrompt(JavaObjectWeakGlobalRef java_ref,
+                           const GURL& origin) {
+    content::BrowserThread::PostTask(content::BrowserThread::UI,
+                                     FROM_HERE,
+                                     base::Bind(&GeolocationShowPromptTask,
+                                                java_ref,
+                                                origin));
+}
+
+} // anonymous namespace
+
+void AwContents::OnGeolocationShowPrompt(const GURL& requesting_frame,
+                                         base::Callback<void(bool)> callback) {
+  GURL origin = requesting_frame.GetOrigin();
+  bool show_prompt = pending_geolocation_prompts_.empty();
+  pending_geolocation_prompts_.push_back(OriginCallback(origin, callback));
+  if (show_prompt) {
+    GeolocationShowPrompt(java_ref_, origin);

[benm (inactive), 2013/02/12 10:46:30]

I think we should take a strong ref to the java_ref_ or make
GeolocationShowPromptTask robust to receiving a null weak ref.

I'd be inclined to take a Global ref here, early out if it's null to avoid
posting unnecessary tasks, and then clear up the exta ref when we return from
java with the answer to the prompt. You can then use const JavaRef<jobject>& as
your parameter type for the helpers.

Same in Invoke and Hide below.

[Kristian Monsen, 2013/02/12 18:53:03]

I have checked if the weak pointer is not null before using. I don't think it
makes sense to take the global ref as it doesn't make sense to call the prompt
if the ref is going away.

[benm (inactive), 2013/02/12 19:03:21]

I don't think checking for null is enough, it may get collected after you've
checked for null. The only way to ensure the GC doesn't collect is if there is a
strong ref to the object.

[Kristian Monsen, 2013/02/12 19:17:20]

Right, but this is happening when you call get() (I think?), and it creates a
local ref. Updated to keep the ref inside the method.

[boliu, 2013/02/12 19:18:47]

Sorry, I got confused this morning, I confused WeakPtr with WeakRef. Do what Ben
says.

[Ben Murdoch, 2013/02/12 19:21:05]

Yes, get() will return a ScopedJavaLocalRef. I just looked at the patch again
and see that you are taking one of those (only spotted the null check last
time), so I think it should be fine now.

[Kristian Monsen, 2013/02/12 19:23:38]

I updated after your comment as I realized it could go bad between the if and
the use :-) Sorry if that was not clear.

+  }
+}
+
+void AwContents::InvokeGeolocationCallback(JNIEnv* env,
+                                           jobject obj,
+                                           jboolean value,
+                                           jstring origin) {
+  GURL callback_origin(base::android::ConvertJavaStringToUTF16(env, origin));
+  if (callback_origin.GetOrigin() ==
+      pending_geolocation_prompts_.front().first) {
+    pending_geolocation_prompts_.front().second.Run(value);
+    pending_geolocation_prompts_.pop_front();
+    if (!pending_geolocation_prompts_.empty()) {
+      GeolocationShowPrompt(java_ref_,
+                            pending_geolocation_prompts_.front().first);
+    }
+  }
+}
+
+void AwContents::OnGeolocationHidePrompt(const GURL& origin) {
+  bool removed_current_outstanding_callback = false;
+  std::list<OriginCallback>::iterator it = pending_geolocation_prompts_.begin();
+  while (it != pending_geolocation_prompts_.end()) {
+    if ((*it).first == origin.GetOrigin()) {
+      if (it == pending_geolocation_prompts_.begin()) {
+        removed_current_outstanding_callback = true;
+      }
+      it = pending_geolocation_prompts_.erase(it);
+    } else {
+      it++;

[benm (inactive), 2013/02/12 10:46:30]

nit: style guide says to pre-increment.

[Kristian Monsen, 2013/02/12 18:53:03]

Done.

+    }
+  }
+
+  if (removed_current_outstanding_callback) {
+    JNIEnv* env = AttachCurrentThread();
+    Java_AwContents_onGeolocationPermissionsHidePrompt(
+        env,
+        java_ref_.get(env).obj());
+    if (!pending_geolocation_prompts_.empty()) {
+      GeolocationShowPrompt(java_ref_,
+                            pending_geolocation_prompts_.front().first);
+    }
+  }
 }
 
 jint AwContents::FindAllSync(JNIEnv* env, jobject obj, jstring search_string) {
   return GetFindHelper()->FindAllSync(
       ConvertJavaStringToUTF16(env, search_string));
 }
 
 void AwContents::FindAllAsync(JNIEnv* env, jobject obj, jstring search_string) {
   GetFindHelper()->FindAllAsync(ConvertJavaStringToUTF16(env, search_string));
 }
@@ skipping 341 matching lines @@
   if (!picture) {
     render_view_host_ext_->CapturePictureSync();
     picture = RendererPictureMap::GetInstance()->GetRendererPicture(
         web_contents_->GetRoutingID());
   }
 
   return picture;
 }
 
 }  // namespace android_webview