<webview>: Move Geolocation permission to chrome layer.

chrome/browser/guestview/webview/webview_guest.cc

Index: chrome/browser/guestview/webview/webview_guest.cc
diff --git a/chrome/browser/guestview/webview/webview_guest.cc b/chrome/browser/guestview/webview/webview_guest.cc
index a57a445e5dc99288c4e68244b08519da1159852d..1274ff928c9cee038dea22ec521430b2a6e8b7e6 100644
--- a/chrome/browser/guestview/webview/webview_guest.cc
+++ b/chrome/browser/guestview/webview/webview_guest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/guestview/webview/webview_guest.h"
 
+#include "base/message_loop/message_loop.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api.h"
@@ -18,6 +19,7 @@
 #include "chrome/browser/guestview/webview/webview_permission_types.h"
 #include "chrome/common/chrome_version_info.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/geolocation_permission_context.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
@@ -75,8 +77,6 @@ static std::string PermissionTypeToString(BrowserPluginPermissionType type) {
   switch (type) {
     case BROWSER_PLUGIN_PERMISSION_TYPE_DOWNLOAD:
       return webview::kPermissionTypeDownload;
-    case BROWSER_PLUGIN_PERMISSION_TYPE_GEOLOCATION:
-      return webview::kPermissionTypeGeolocation;
     case BROWSER_PLUGIN_PERMISSION_TYPE_MEDIA:
       return webview::kPermissionTypeMedia;
     case BROWSER_PLUGIN_PERMISSION_TYPE_NEW_WINDOW:
@@ -91,6 +91,8 @@ static std::string PermissionTypeToString(BrowserPluginPermissionType type) {
     default: {
       WebViewPermissionType webview = static_cast<WebViewPermissionType>(type);
       switch (webview) {
+        case WEB_VIEW_PERMISSION_TYPE_GEOLOCATION:
+          return webview::kPermissionTypeGeolocation;
         case WEB_VIEW_PERMISSION_TYPE_LOAD_PLUGIN:
           return webview::kPermissionTypeLoadPlugin;
       }
@@ -193,10 +195,6 @@ void WebViewGuest::RecordUserInitiatedUMA(const PermissionResponseInfo& info,
         content::RecordAction(
             UserMetricsAction("BrowserPlugin.PermissionAllow.Download"));
         break;
-      case BROWSER_PLUGIN_PERMISSION_TYPE_GEOLOCATION:
-        content::RecordAction(
-            UserMetricsAction("BrowserPlugin.PermissionAllow.Geolocation"));
-        break;
       case BROWSER_PLUGIN_PERMISSION_TYPE_MEDIA:
         content::RecordAction(
             UserMetricsAction("BrowserPlugin.PermissionAllow.Media"));
@@ -219,6 +217,10 @@ void WebViewGuest::RecordUserInitiatedUMA(const PermissionResponseInfo& info,
         WebViewPermissionType webview_permission_type =
             static_cast<WebViewPermissionType>(info.permission_type);
         switch (webview_permission_type) {
+          case WEB_VIEW_PERMISSION_TYPE_GEOLOCATION:
+            content::RecordAction(
+                UserMetricsAction("WebView.PermissionAllow.Geolocation"));
+            break;
           case WEB_VIEW_PERMISSION_TYPE_LOAD_PLUGIN:
             content::RecordAction(
                 UserMetricsAction("WebView.Guest.PermissionAllow.PluginLoad"));
@@ -234,10 +236,6 @@ void WebViewGuest::RecordUserInitiatedUMA(const PermissionResponseInfo& info,
         content::RecordAction(
             UserMetricsAction("BrowserPlugin.PermissionDeny.Download"));
         break;
-      case BROWSER_PLUGIN_PERMISSION_TYPE_GEOLOCATION:
-        content::RecordAction(
-            UserMetricsAction("BrowserPlugin.PermissionDeny.Geolocation"));
-        break;
       case BROWSER_PLUGIN_PERMISSION_TYPE_MEDIA:
         content::RecordAction(
             UserMetricsAction("BrowserPlugin.PermissionDeny.Media"));
@@ -260,6 +258,10 @@ void WebViewGuest::RecordUserInitiatedUMA(const PermissionResponseInfo& info,
         WebViewPermissionType webview_permission_type =
             static_cast<WebViewPermissionType>(info.permission_type);
         switch (webview_permission_type) {
+          case WEB_VIEW_PERMISSION_TYPE_GEOLOCATION:
+            content::RecordAction(
+                UserMetricsAction("WebView.PermissionDeny.Geolocation"));
+            break;
           case WEB_VIEW_PERMISSION_TYPE_LOAD_PLUGIN:
             content::RecordAction(
                 UserMetricsAction("WebView.Guest.PermissionDeny.PluginLoad"));
@@ -437,7 +439,7 @@ void WebViewGuest::RendererUnresponsive() {
   DispatchEvent(new GuestView::Event(webview::kEventUnresponsive, args.Pass()));
 }
 
-bool WebViewGuest::RequestPermission(
+int WebViewGuest::RequestPermission(
     BrowserPluginPermissionType permission_type,
     const base::DictionaryValue& request_info,
     const PermissionResponseCallback& callback,
@@ -445,8 +447,17 @@ bool WebViewGuest::RequestPermission(
   // If there are too many pending permission requests then reject this request.
   if (pending_permission_requests_.size() >=
       webview::kMaxOutstandingPermissionRequests) {
-    callback.Run(false, std::string());
-    return true;
+    // Let the stack unwind before we deny the permission request so that
+    // objects held by the permission request are not destroyed immediately
+    // after creation. This is to allow those same objects to be accessed again
+    // in the same scope without fear of use after freeing.
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&PermissionResponseCallback::Run,
+                  base::Owned(new PermissionResponseCallback(callback)),
+                  allowed_by_default,
+                  std::string()));
+    return webview::kInvalidPermissionRequestID;
   }
 
   int request_id = next_permission_request_id_++;
@@ -473,7 +484,7 @@ bool WebViewGuest::RequestPermission(
       break;
     }
   }
-  return true;
+  return request_id;
 }
 
 void WebViewGuest::Observe(int type,
@@ -544,6 +555,73 @@ void WebViewGuest::Reload() {
   guest_web_contents()->GetController().Reload(false);
 }
 
+

[lazyboy, 2014/04/11 22:00:53]

nit: remove empty line.

+void WebViewGuest::RequestGeolocationPermission(
+    int bridge_id,
+    const GURL& requesting_frame,
+    bool user_gesture,
+    const base::Callback<void(bool)>& callback) {
+  base::DictionaryValue request_info;
+  request_info.Set(guestview::kUrl,
+                   base::Value::CreateStringValue(requesting_frame.spec()));
+  request_info.Set(guestview::kUserGesture,
+                   base::Value::CreateBooleanValue(user_gesture));
+  int request_id = RequestPermission(
+      static_cast<BrowserPluginPermissionType>(
+          WEB_VIEW_PERMISSION_TYPE_GEOLOCATION),
+      request_info,
+      base::Bind(&WebViewGuest::OnWebViewGeolocationPermissionResponse,
+                 base::Unretained(this),

[lazyboy, 2014/04/11 21:47:21]

Same question like the other CL, what if WebViewGuest goes away before the bound
method gets called.

[Fady Samuel, 2014/04/11 21:57:16]

WebViewGuest::SetPermission calls the bound method so this is not possible.

[lazyboy, 2014/04/11 22:00:53]

Cool, can we add a quick comment here then.

[Fady Samuel, 2014/04/14 17:30:02]

Done.

+                 bridge_id,
+                 user_gesture,
+                 callback),
+      false /* allowed_by_default */);
+  bridge_id_to_request_id_map_[bridge_id] = request_id;
+}
+
+void WebViewGuest::OnWebViewGeolocationPermissionResponse(
+    int bridge_id,
+    bool user_gesture,
+    const base::Callback<void(bool)>& callback,
+    bool allow,
+    const std::string& user_input) {
+  // The <webview> embedder has allowed the permission. We now need to make sure
+  // that the embedder has geolocation permission.
+  RemoveBridgeID(bridge_id);
+
+  if (!allow || !attached()) {
+    callback.Run(false);
+    return;
+  }
+
+  content::GeolocationPermissionContext* geolocation_context =
+      browser_context()->GetGeolocationPermissionContext();
+
+  DCHECK(geolocation_context);
+  geolocation_context->RequestGeolocationPermission(
+      embedder_web_contents()->GetRenderProcessHost()->GetID(),
+      embedder_web_contents()->GetRoutingID(),
+      // The geolocation permission request here is not initiated
+      // through WebGeolocationPermissionRequest. We are only interested
+      // in the fact whether the embedder/app has geolocation
+      // permission. Therefore we use an invalid |bridge_id|.
+      -1 /* bridge_id */,
+      embedder_web_contents()->GetLastCommittedURL(),
+      user_gesture,
+      callback);
+}
+
+void WebViewGuest::CancelGeolocationPermissionRequest(int bridge_id) {
+  int request_id = RemoveBridgeID(bridge_id);
+  RequestMap::iterator request_itr =
+      pending_permission_requests_.find(request_id);
+
+  if (request_itr == pending_permission_requests_.end())
+    return;
+
+  pending_permission_requests_.erase(request_itr);
+}
+
 WebViewGuest::SetPermissionResult WebViewGuest::SetPermission(
     int request_id,
     PermissionResponseAction action,
@@ -818,6 +896,18 @@ void WebViewGuest::InjectChromeVoxIfNeeded(
 #endif
 }
 
+int WebViewGuest::RemoveBridgeID(int bridge_id) {
+  std::map<int, int>::iterator bridge_itr =
+      bridge_id_to_request_id_map_.find(bridge_id);
+  if (bridge_itr == bridge_id_to_request_id_map_.end())
+    return webview::kInvalidPermissionRequestID;
+
+  int request_id = bridge_itr->second;
+  bridge_id_to_request_id_map_.erase(bridge_itr);
+  return request_id;
+}
+
+

[lazyboy, 2014/04/11 21:47:21]

remove empty line

[Fady Samuel, 2014/04/11 21:57:16]

Done.

 WebViewGuest::PermissionResponseInfo::PermissionResponseInfo()
     : permission_type(BROWSER_PLUGIN_PERMISSION_TYPE_UNKNOWN),
       allowed_by_default(false) {