Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(312)

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

Issue 2612993002: Make geolocation API and X-Geo header access consistent (Closed)
Patch Set: Remove stuff which shouldn't be there Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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()) {
+ 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
gone 2017/01/11 18:39:35 Is DSE a commonly understood acronym? It's used t
benwells 2017/01/11 22:29:26 Fair point. I've added comments here and in the We
+ * 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);
+ }
+
+ /**
* 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:

Powered by Google App Engine
This is Rietveld 408576698