Make geolocation API and X-Geo header access consistent

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
index 9b010af10d5ff0733e59f93437a56f199462bf88..abae4243568d0eda4780007fa58e03a044c149a2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
@@ -22,6 +22,7 @@ import android.widget.ListView;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ContentSettingsType;
 import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
@@ -465,7 +466,12 @@ public class SingleWebsitePreferences extends PreferenceFragment
         ContentSetting permission = mSite.getGeolocationPermission();
         Context context = preference.getContext();
         Object locationAllowed = getArguments().getSerializable(EXTRA_LOCATION);
-        if (permission == null && hasXGeoLocationPermission(context)) {
+        if (shouldUseDSEGeolocationSetting()) {

[raymes, 2017/01/09 05:47:26]

I haven't reviewed the code here closely since I'm not too familiar with it. 

[benwells, 2017/01/09 21:02:21]

Acknowledged.

+            String origin = mSite.getAddress().getOrigin();
+            mSite.setGeolocationInfo(new GeolocationInfo(origin, origin, false));
+            setUpListPreference(preference, ContentSetting.ALLOW);
+            updateLocationPreferenceForDSESetting(preference);
+        } else if (permission == null && hasXGeoLocationPermission(context)) {
             String origin = mSite.getAddress().getOrigin();
             mSite.setGeolocationInfo(new GeolocationInfo(origin, origin, false));
             setUpListPreference(preference, ContentSetting.ALLOW);
@@ -486,12 +492,25 @@ public class SingleWebsitePreferences extends PreferenceFragment
      * @param context The current context.
      */
     private boolean hasXGeoLocationPermission(Context context) {
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.CONSISTENT_OMNIBOX_GEOLOCATION)) {
+            return false;
+        }
+
         String searchUrl = TemplateUrlService.getInstance().getUrlForSearchQuery("foo");
         return mSite.getAddress().matches(searchUrl)
                 && GeolocationHeader.isGeoHeaderEnabledForUrl(context, searchUrl, false);
     }
 
     /**
+     * Returns true if the DSE geolocation setting should be used for the current host. This will be
+     * the case when the host is the CCTLD of the DSE, and the DSE supports the X-Geo header.
+     */
+    private boolean shouldUseDSEGeolocationSetting() {
+        return WebsitePreferenceBridge.nativeShouldUseDSEGeolocationSetting(
+                mSite.getAddress().getOrigin(), false);

[raymes, 2017/01/09 05:47:26]

It looks like you're always passing false here for incognito? Is that intended?

[benwells, 2017/01/09 21:02:21]

Yes, the settings are always shown for the non-incognito profile. 'false' for is
incognito is also passed into the isGeoHeaderEnabledForUrl call on line 501
above.

[raymes, 2017/01/10 03:19:32]

I see so this function is only called for non-incognito then?

+    }
+
+    /**
      * Updates the location preference to indicate that the site has access to location (via X-Geo)
      * for searches that happen from the omnibox.
      * @param preference The Location preference to modify.
@@ -500,7 +519,7 @@ public class SingleWebsitePreferences extends PreferenceFragment
         ListPreference listPreference = (ListPreference) preference;
         Resources res = getResources();
         listPreference.setEntries(new String[] {
-                res.getString(R.string.website_settings_permissions_allow_dse),
+                res.getString(R.string.website_settings_permissions_allow_dse_address_bar),
                 res.getString(ContentSettingsResources.getSiteSummary(ContentSetting.BLOCK)),
         });
         listPreference.setEntryValues(new String[] {
@@ -510,6 +529,22 @@ public class SingleWebsitePreferences extends PreferenceFragment
         listPreference.setValueIndex(0);
     }
 
+    /**
+     * Updates the location preference to indicate that the site has access to location (via X-Geo)
+     * for searches that happen from the omnibox.
+     * @param preference The Location preference to modify.
+     */
+    private void updateLocationPreferenceForDSESetting(Preference preference) {
+        ListPreference listPreference = (ListPreference) preference;
+        Resources res = getResources();
+        listPreference.setEntries(new String[] {
+                res.getString(R.string.website_settings_permissions_allow_dse),
+                res.getString(R.string.website_settings_permissions_block_dse),
+        });
+        listPreference.setValueIndex(
+                WebsitePreferenceBridge.nativeGetDSEGeolocationSetting() ? 0 : 1);
+    }
+
     private int getContentSettingsTypeFromPreferenceKey(String preferenceKey) {
         switch (preferenceKey) {
             case PREF_AUTOPLAY_PERMISSION: