Modify Physical Web auto-optin behavior on Chrome for iOS

ios/chrome/browser/physical_web/start_physical_web_discovery.mm

Index: ios/chrome/browser/physical_web/start_physical_web_discovery.mm
diff --git a/ios/chrome/browser/physical_web/start_physical_web_discovery.mm b/ios/chrome/browser/physical_web/start_physical_web_discovery.mm
index e1b9502f4d8cc56e01ed7dbdf06d8108e2fde45f..91bab32f6b18edd273a8ff6bdbee52d64a690cf3 100644
--- a/ios/chrome/browser/physical_web/start_physical_web_discovery.mm
+++ b/ios/chrome/browser/physical_web/start_physical_web_discovery.mm
@@ -6,16 +6,20 @@
 
 #import <CoreLocation/CoreLocation.h>
 
+#include "base/strings/utf_string_conversions.h"
 #include "components/physical_web/data_source/physical_web_data_source.h"
+#include "components/search_engines/template_url_service.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #import "ios/chrome/browser/experimental_flags.h"
 #import "ios/chrome/browser/geolocation/omnibox_geolocation_config.h"
 #include "ios/chrome/browser/physical_web/physical_web_constants.h"
 #include "ios/chrome/browser/pref_names.h"
+#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
 #include "url/gurl.h"
 
-void StartPhysicalWebDiscovery(PrefService* pref_service, bool is_incognito) {
+void StartPhysicalWebDiscovery(PrefService* pref_service,
+                               ios::ChromeBrowserState* browser_state) {
   // Do not scan if the Physical Web feature is disabled by a command line flag
   // or Chrome Variations experiment.
   if (!experimental_flags::IsPhysicalWebEnabled()) {
@@ -28,6 +32,10 @@ void StartPhysicalWebDiscovery(PrefService* pref_service, bool is_incognito) {
   int preference_state =
       pref_service->GetInteger(prefs::kIosPhysicalWebEnabled);
   if (preference_state == physical_web::kPhysicalWebOnboarding) {
+    // Check whether the user is in Incognito mode. Physical Web will only be
+    // auto-enabled if the user is not in Incognito.
+    bool is_incognito = browser_state->IsOffTheRecord();
+
     // Check that Location Services is enabled.
     bool location_services_enabled =
         [CLLocationManager locationServicesEnabled];
@@ -43,13 +51,22 @@ void StartPhysicalWebDiscovery(PrefService* pref_service, bool is_incognito) {
     bool geolocation_eligible = [[OmniboxGeolocationConfig sharedInstance]
         URLHasEligibleDomain:GURL("https://www.google.com")];
 
-    if (!is_incognito && location_services_enabled && location_authorized &&
-        geolocation_eligible) {
-      pref_service->SetInteger(prefs::kIosPhysicalWebEnabled,
-                               physical_web::kPhysicalWebOn);
-      preference_state =
-          pref_service->GetInteger(prefs::kIosPhysicalWebEnabled);
-    }
+    // Check that Google Search is configured as the default search engine.
+    TemplateURLService* template_url_service =
+        ios::TemplateURLServiceFactory::GetForBrowserState(browser_state);
+    const base::string16& default_search_name =

[sdefresne, 2016/12/14 10:15:56]

components/google/core/browser/google_util.h has a method
google_util::IsGoogleSearchUrl(const GURL&). Can this be used here instead of
comparing strings?

[mattreynolds, 2016/12/14 20:35:02]

I couldn't get IsGoogleSearchUrl to work since the url stored by TemplateURL
uses a keyword for the domain. But, TemplateURL has
IsGoogleSearchURLWithReplaceableKeyword which does what we want.

+        template_url_service->GetDefaultSearchProvider()->short_name();
+    bool google_search_enabled =
+        (default_search_name == base::UTF8ToUTF16("Google"));
+
+    bool auto_enable = !is_incognito && location_services_enabled &&
+                       location_authorized && geolocation_eligible &&
+                       google_search_enabled;
+
+    pref_service->SetInteger(prefs::kIosPhysicalWebEnabled,

[sdefresne, 2016/12/14 10:15:56]

If I understand correctly, since all new install use Google as the default
search engine, and have no incognito tab opens, this will be enabled by default
for all new installs? Correct?

[mattreynolds, 2016/12/14 20:35:02]

We have five requirements for auto-enable:

* Location Services is on
* Chrome is authorized to use your location
* Google.com is eligible for geolocation (AFAICT this is a no-op on iOS since
there's no Site Settings)
* Google Search is the default search provider
* User is not currently in incognito mode

On top of that, since we'll be launching under a Finch flag, it will take at
least one restart before the auto-enable logic will execute.

So, on first launch, I expect:

* Location Services: could be on or off
* Location app permission: off for first launch, but NTP will prompt for it
* Google.com geolocation: defaults to eligible, no way to disable it
* Default search provider: Google Search
* Incognito: no
* Finch: disabled

Supposing Location Services is on and the user grants the Location permission
when prompted, then on the second launch the Finch flag will be enabled and
we'll auto-enable. If the user doesn't grant Location or changes any of the
other settings, we'll auto-disable.

Eventually we'll remove Finch or have it default to enabled. In that case, I'd
expect the new install to never auto-enable since the location permission won't
be granted in time.

+                             auto_enable ? physical_web::kPhysicalWebOn
+                                         : physical_web::kPhysicalWebOff);
+    preference_state = pref_service->GetInteger(prefs::kIosPhysicalWebEnabled);
   }
 
   // Scan only if the feature is enabled.
@@ -60,7 +77,3 @@ void StartPhysicalWebDiscovery(PrefService* pref_service, bool is_incognito) {
   }
 }
 
-void StartPhysicalWebDiscovery(PrefService* pref_service,
-                               ios::ChromeBrowserState* browser_state) {
-  StartPhysicalWebDiscovery(pref_service, browser_state->IsOffTheRecord());
-}