Show the Android Location Settings Dialog when sites want permission.

chrome/browser/geolocation/geolocation_permission_context_android.cc

 // Copyright 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 "chrome/browser/geolocation/geolocation_permission_context_android.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
+#include "base/feature_list.h"
 #include "chrome/browser/android/location_settings.h"
 #include "chrome/browser/android/location_settings_impl.h"
 #include "chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h"
 #include "chrome/browser/android/search_geolocation/search_geolocation_service.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/permissions/permission_update_infobar_delegate_android.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/common/chrome_features.h"
 #include "components/infobars/core/infobar.h"
+#include "components/search_engines/template_url.h"
+#include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
 GeolocationPermissionContextAndroid::
     GeolocationPermissionContextAndroid(Profile* profile)
     : GeolocationPermissionContext(profile),
       location_settings_(new LocationSettingsImpl()),
       permission_update_infobar_(nullptr),
@@ skipping 41 matching lines @@
 
   return value;
 }
 
 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)) {

[raymes, 2017/03/02 03:04:04]

I think we will need to change CanSitesRequestLocationPermission? Right now it
will return false if the system permission isn't granted. But we will want it to
return true in that case, as long as the system permission can be /requested/.
We will also need to incorporate the backoff into that check. 

If we do that, then we won't need to check those values later on, we will just
show the prompt if it's needed.

Also, for GetPermissionStatus() we will need to split up those checks to give
the right result. Something like:
if (has system permission AND has chrome permission AND has content setting)
  return allow;

if (can request system permission AND can request chrome permission AND content
setting is ask) 
  return prompt;

return block;

Note that the definition of "can request system permission" is different to what
is currently implemented in CanPromptToEnableSystemLocationSetting.
CanPromptToEnableSystemLocationSetting will currently return false if location
is already enabled, whereas in the above code it would need to return true to
work properly.

[benwells, 2017/03/02 04:19:28]

Yep ... you're right - this won't work.

I think it's confusing the the check for whether location is on embedded in the
CanPromptToEnable... call. Also this CanSitesRequestLocationPermission is
confusing too, I think it is confusing with 'Request' in the name - what if it
doesnt' need to request?

Instead we could have the following functions on LocationSettings:
- IsLocationAccessPossible
  - would be true if location is enabled on the device (at device or chrome
permission level), or it isn't and the prompts required to enabled it can be
shown. Would take into account backoff, so would have SEARCH or DEFAULT as a
parameter
  - we'd call that in RequestPermission and block early if it is false
- ShouldShowLocationSettingsDialog
  - returns true if system permission is off and the prompt can be shown (incl.
backoff)
  - would be called in NotifyPermissionSet if the content setting is ALLOW
- ShowLocationSettingsDialog
  - resets backoff and shows the dialog, called in NotifyPermissionSet

I think all of this can happen in the C++ with the Java side (i.e. qfiard's
patch that this depends on) staying the same.

WDYT?

qfiard - WDYT as well?

[raymes, 2017/03/02 04:40:18]

Sounds good to me. I think we would still need to split this bit up to make it
possible to distinguish between having granted each of the permissions and
whether a prompt will be shown in order to implement GetPermissionStatus. We can
leave that until a subsequent patch though, but just making a note in case it
would make it easier to implement things like that initially.

[benwells, 2017/03/02 04:48:02]

In GetPermissoinStatus, if the content setting is ALLOW, we would:
if IsLocationAccessPossible is false, return BLOCK
if ShouldShowLocationSettingsDialog is true, return PROMPT
if PermissionUpdateInfoBarDelegate::ShouldShowPermissionInfobar is true, return
PROMPT.

Would that work?

     PermissionDecided(id, requesting_frame_origin,
                       web_contents->GetLastCommittedURL().GetOrigin(),
                       user_gesture, callback, false /* persist */,
                       CONTENT_SETTING_BLOCK);
     return;
   }
 
   GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
   ContentSetting content_setting =
       GeolocationPermissionContext::GetPermissionStatus(requesting_frame_origin,
@@ skipping 30 matching lines @@
   GeolocationPermissionContext::CancelPermissionRequest(web_contents, id);
 }
 
 void GeolocationPermissionContextAndroid::NotifyPermissionSet(
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     const BrowserPermissionCallback& callback,
     bool persist,
     ContentSetting content_setting) {
-  GeolocationPermissionContext::NotifyPermissionSet(id, requesting_origin,
-                                                    embedding_origin, callback,
-                                                    persist, content_setting);
+  if (content_setting == CONTENT_SETTING_ALLOW &&
+      WillShowLocationSettingsDialog(requesting_origin, embedding_origin)) {
+    // Work out if this is a default search engine origin.
+    bool is_dse_origin = false;
+    TemplateURLService* template_url_service =
+        TemplateURLServiceFactory::GetForProfile(profile());
+    if (template_url_service) {
+      const TemplateURL* template_url =
+          template_url_service->GetDefaultSearchProvider();
+      if (template_url) {
+        GURL search_url = template_url->GenerateSearchURL(
+            template_url_service->search_terms_data());
+        is_dse_origin = url::IsSameOriginWith(requesting_origin, search_url);
+      }
+    }

[raymes, 2017/03/02 03:04:04]

Just to clarify, this is intentionally different from the logic we use in
SearchGeolocationService? It looks like this will get the origin of any default
search engine, not just Google.

[benwells, 2017/03/02 04:19:28]

Yep, that's correct.

 
-  // If this is the default search origin, and the DSE Geolocation setting is
-  // being used, potentially show the disclosure.
-  if (requesting_origin == embedding_origin) {
     content::WebContents* web_contents =
         content::WebContents::FromRenderFrameHost(
             content::RenderFrameHost::FromID(id.render_process_id(),
                                              id.render_frame_id()));
-    SearchGeolocationDisclosureTabHelper* disclosure_helper =
-        SearchGeolocationDisclosureTabHelper::FromWebContents(web_contents);
+    location_settings_->PromptToEnableSystemLocationSetting(
+        is_dse_origin ? LocationSettings::SEARCH : LocationSettings::DEFAULT,
+        web_contents,
+        base::Bind(

[qfiard, 2017/03/01 20:38:49]

You will need to switch to base::BindOnce since I switched to use OnceCallbacks.

[benwells, 2017/03/03 01:43:09]

Done.

+            &GeolocationPermissionContextAndroid::OnLocationSettingsDialogShown,
+            weak_factory_.GetWeakPtr(), id, requesting_origin, embedding_origin,
+            callback, persist, content_setting));
+    return;
+  }
 
-    // The tab helper can be null in tests.
-    if (disclosure_helper)
-      disclosure_helper->MaybeShowDisclosure(requesting_origin);
-  }
+  FinishNotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                            persist, content_setting);
 }
 
 void GeolocationPermissionContextAndroid::HandleUpdateAndroidPermissions(
     const PermissionRequestID& id,
     const GURL& requesting_frame_origin,
     const GURL& embedding_origin,
     const BrowserPermissionCallback& callback,
     bool permissions_updated) {
   permission_update_infobar_ = nullptr;
 
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   ContentSetting new_setting = permissions_updated
       ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
 
   NotifyPermissionSet(id, requesting_frame_origin, embedding_origin, callback,
                       false /* persist */, new_setting);
 }
 
+bool GeolocationPermissionContextAndroid::WillShowLocationSettingsDialog(
+    const GURL& requesting_origin,
+    const GURL& embedding_origin) {
+  if (!base::FeatureList::IsEnabled(features::kLsdPermissionPrompt))
+    return false;
+
+  // TODO(benwells): Also check backoff.
+  return location_settings_->CanPromptToEnableSystemLocationSetting();
+}
+
+void GeolocationPermissionContextAndroid::OnLocationSettingsDialogShown(
+    const PermissionRequestID& id,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    const BrowserPermissionCallback& callback,
+    bool persist,
+    ContentSetting content_setting,
+    LocationSettings::LocationSettingsDialogPromptOutcome prompt_outcome) {
+  if (prompt_outcome != LocationSettings::GRANTED) {
+    content_setting = CONTENT_SETTING_BLOCK;
+    persist = false;
+  }
+
+  FinishNotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                            persist, content_setting);
+}
+
+void GeolocationPermissionContextAndroid::FinishNotifyPermissionSet(
+    const PermissionRequestID& id,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    const BrowserPermissionCallback& callback,
+    bool persist,
+    ContentSetting content_setting) {
+  GeolocationPermissionContext::NotifyPermissionSet(id, requesting_origin,
+                                                    embedding_origin, callback,
+                                                    persist, content_setting);
+
+  // If this is the default search origin, and the DSE Geolocation setting is
+  // being used, potentially show the disclosure.
+  if (requesting_origin == embedding_origin) {
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderFrameHost(
+            content::RenderFrameHost::FromID(id.render_process_id(),
+                                             id.render_frame_id()));
+    SearchGeolocationDisclosureTabHelper* disclosure_helper =
+        SearchGeolocationDisclosureTabHelper::FromWebContents(web_contents);
+
+    // The tab helper can be null in tests.
+    if (disclosure_helper)
+      disclosure_helper->MaybeShowDisclosure(requesting_origin);
+  }
+}
+
 void GeolocationPermissionContextAndroid::SetLocationSettingsForTesting(
     std::unique_ptr<LocationSettings> settings) {
   location_settings_ = std::move(settings);
 }