Reintroduce plumbing for user gesture into permission system.

This will allow metrics to be gathered about the acceptance / denial rates of permission prompts with gestures and without. This effectively reverts: 1. https://crrev.com/acea72be67d92e56665f5357f4441f65c8073c15 and 2. https://crrev.com/a8e32b6fc3a4696ba24c8d6f257ec3fe848cf940 Those patches could not be reverted individually due to refactorings in the permissions sytem. TBR=torne@chromium.org BUG=614599 Review-Url: https://codereview.chromium.org/2110343002 Cr-Commit-Position: refs/heads/master@{#403738}
Denger-Network · Jul 5, 2016 · fd2b155 · fd2b155
1 parent febbb09
commit fd2b155
Show file tree

Hide file tree

Showing 49 changed files with 157 additions and 60 deletions.
diff --git a/android_webview/browser/aw_permission_manager.cc b/android_webview/browser/aw_permission_manager.cc
@@ -169,6 +169,7 @@ int AwPermissionManager::RequestPermission(
     PermissionType permission,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
+    bool user_gesture,
     const base::Callback<void(PermissionStatus)>& callback) {
   int render_process_id = render_frame_host->GetProcess()->GetID();
   int render_frame_id = render_frame_host->GetRoutingID();
@@ -262,6 +263,7 @@ int AwPermissionManager::RequestPermissions(
     const std::vector<PermissionType>& permissions,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
+    bool user_gesture,
     const base::Callback<void(
         const std::vector<PermissionStatus>&)>& callback) {
   NOTIMPLEMENTED() << "RequestPermissions has not been implemented in WebView";

diff --git a/android_webview/browser/aw_permission_manager.h b/android_webview/browser/aw_permission_manager.h
@@ -27,12 +27,14 @@ class AwPermissionManager : public content::PermissionManager {
       content::PermissionType permission,
       content::RenderFrameHost* render_frame_host,
       const GURL& requesting_origin,
+      bool user_gesture,
       const base::Callback<void(blink::mojom::PermissionStatus)>& callback)
       override;
   int RequestPermissions(
       const std::vector<content::PermissionType>& permissions,
       content::RenderFrameHost* render_frame_host,
       const GURL& requesting_origin,
+      bool user_gesture,
       const base::Callback<
           void(const std::vector<blink::mojom::PermissionStatus>&)>& callback)
       override;

diff --git a/blimp/engine/app/blimp_permission_manager.cc b/blimp/engine/app/blimp_permission_manager.cc
@@ -21,6 +21,7 @@ int BlimpPermissionManager::RequestPermission(
     content::PermissionType permission,
     content::RenderFrameHost* render_frame_host,
     const GURL& origin,
+    bool user_gesture,
     const base::Callback<void(blink::mojom::PermissionStatus)>& callback) {
   callback.Run(blink::mojom::PermissionStatus::DENIED);
   return kNoPendingOperation;
@@ -30,6 +31,7 @@ int BlimpPermissionManager::RequestPermissions(
     const std::vector<content::PermissionType>& permission,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
+    bool user_gesture,
     const base::Callback<
         void(const std::vector<blink::mojom::PermissionStatus>&)>& callback) {
   callback.Run(std::vector<blink::mojom::PermissionStatus>(

diff --git a/blimp/engine/app/blimp_permission_manager.h b/blimp/engine/app/blimp_permission_manager.h
@@ -24,12 +24,14 @@ class BlimpPermissionManager : public content::PermissionManager {
       content::PermissionType permission,
       content::RenderFrameHost* render_frame_host,
       const GURL& requesting_origin,
+      bool user_gesture,
       const base::Callback<void(blink::mojom::PermissionStatus)>& callback)
       override;
   int RequestPermissions(
       const std::vector<content::PermissionType>& permission,
       content::RenderFrameHost* render_frame_host,
       const GURL& requesting_origin,
+      bool user_gesture,
       const base::Callback<
           void(const std::vector<blink::mojom::PermissionStatus>&)>& callback)
       override;

diff --git a/chrome/browser/background_sync/background_sync_permission_context.cc b/chrome/browser/background_sync/background_sync_permission_context.cc
@@ -27,6 +27,7 @@ void BackgroundSyncPermissionContext::DecidePermission(
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
+    bool user_gesture,
     const BrowserPermissionCallback& callback) {
   // The user should never be prompted to authorize background sync.
   NOTREACHED();

diff --git a/chrome/browser/background_sync/background_sync_permission_context.h b/chrome/browser/background_sync/background_sync_permission_context.h
@@ -32,6 +32,7 @@ class BackgroundSyncPermissionContext : public PermissionContextBase {
                         const PermissionRequestID& id,
                         const GURL& requesting_origin,
                         const GURL& embedding_origin,
+                        bool user_gesture,
                         const BrowserPermissionCallback& callback) override;
   bool IsRestrictedToSecureOrigins() const override;
 

diff --git a/chrome/browser/background_sync/background_sync_permission_context_unittest.cc b/chrome/browser/background_sync/background_sync_permission_context_unittest.cc
@@ -40,7 +40,7 @@ class BackgroundSyncPermissionContextTest
         web_contents()->GetRenderProcessHost()->GetID(),
         web_contents()->GetMainFrame()->GetRoutingID(), -1 /* request_id */);
     permission_context->RequestPermission(
-        web_contents(), id, url,
+        web_contents(), id, url, false /* user_gesture */,
         base::Bind(
             &BackgroundSyncPermissionContextTest::TrackPermissionDecision,
             base::Unretained(this), run_loop.QuitClosure()));

diff --git a/chrome/browser/geolocation/geolocation_permission_context.cc b/chrome/browser/geolocation/geolocation_permission_context.cc
@@ -28,14 +28,15 @@ void GeolocationPermissionContext::DecidePermission(
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
+    bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   bool permission_set;
   bool new_permission;
   if (extensions_context_.DecidePermission(
-      web_contents, id, id.request_id(), requesting_origin, callback,
-      &permission_set, &new_permission)) {
+      web_contents, id, id.request_id(), requesting_origin, user_gesture,
+      callback, &permission_set, &new_permission)) {
     if (permission_set) {
       ContentSetting content_setting =
           new_permission ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
@@ -53,6 +54,7 @@ void GeolocationPermissionContext::DecidePermission(
                                           id,
                                           requesting_origin,
                                           embedding_origin,
+                                          user_gesture,
                                           callback);
 }
 

diff --git a/chrome/browser/geolocation/geolocation_permission_context.h b/chrome/browser/geolocation/geolocation_permission_context.h
@@ -29,6 +29,7 @@ class GeolocationPermissionContext  : public PermissionContextBase {
                         const PermissionRequestID& id,
                         const GURL& requesting_origin,
                         const GURL& embedding_origin,
+                        bool user_gesture,
                         const BrowserPermissionCallback& callback) override;
 
   // Adds special logic when called through an extension.

diff --git a/chrome/browser/geolocation/geolocation_permission_context_android.cc b/chrome/browser/geolocation/geolocation_permission_context_android.cc
@@ -32,6 +32,7 @@ void GeolocationPermissionContextAndroid::RequestPermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
     const GURL& requesting_frame_origin,
+    bool user_gesture,
     const BrowserPermissionCallback& callback) {
   if (!location_settings_->CanSitesRequestLocationPermission(web_contents)) {
     PermissionDecided(id, requesting_frame_origin,
@@ -61,7 +62,7 @@ void GeolocationPermissionContextAndroid::RequestPermission(
   }
 
   GeolocationPermissionContext::RequestPermission(
-      web_contents, id, requesting_frame_origin, callback);
+      web_contents, id, requesting_frame_origin, user_gesture, callback);
 }
 
 void GeolocationPermissionContextAndroid::CancelPermissionRequest(

diff --git a/chrome/browser/geolocation/geolocation_permission_context_android.h b/chrome/browser/geolocation/geolocation_permission_context_android.h
@@ -52,6 +52,7 @@ class GeolocationPermissionContextAndroid
       content::WebContents* web_contents,
       const PermissionRequestID& id,
       const GURL& requesting_frame_origin,
+      bool user_gesture,
       const BrowserPermissionCallback& callback) override;
   void CancelPermissionRequest(content::WebContents* web_contents,
                                const PermissionRequestID& id) override;

diff --git a/chrome/browser/geolocation/geolocation_permission_context_extensions.cc b/chrome/browser/geolocation/geolocation_permission_context_extensions.cc
@@ -49,6 +49,7 @@ bool GeolocationPermissionContextExtensions::DecidePermission(
     const PermissionRequestID& request_id,
     int bridge_id,
     const GURL& requesting_frame,
+    bool user_gesture,
     const base::Callback<void(ContentSetting)>& callback,
     bool* permission_set,
     bool* new_permission) {
@@ -59,7 +60,7 @@ bool GeolocationPermissionContextExtensions::DecidePermission(
       extensions::WebViewPermissionHelper::FromWebContents(web_contents);
   if (web_view_permission_helper) {
     web_view_permission_helper->RequestGeolocationPermission(
-        bridge_id, requesting_frame,
+        bridge_id, requesting_frame, user_gesture,
         base::Bind(&CallbackContentSettingWrapper, callback));
     *permission_set = false;
     *new_permission = false;

diff --git a/chrome/browser/geolocation/geolocation_permission_context_extensions.h b/chrome/browser/geolocation/geolocation_permission_context_extensions.h
@@ -30,6 +30,7 @@ class GeolocationPermissionContextExtensions {
                         const PermissionRequestID& request_id,
                         int bridge_id,
                         const GURL& requesting_frame,
+                        bool user_gesture,
                         const base::Callback<void(ContentSetting)>& callback,
                         bool* permission_set,
                         bool* new_permission);