Hide location permission if a search engine is non-Google or its location permission is unset.

chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
index 792ea9b31ae8182bf99982129943194274e2f44c..430f69af85ba92eecd4702f03f60353a891bde8b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
@@ -10,6 +10,7 @@ import android.content.SharedPreferences;
 import android.content.res.Resources;
 import android.os.Build;
 import android.os.Bundle;
+import android.support.annotation.IntDef;
 import android.text.SpannableString;
 import android.text.style.ForegroundColorSpan;
 import android.view.LayoutInflater;
@@ -38,6 +39,8 @@ import org.chromium.components.location.LocationUtils;
 import org.chromium.ui.text.SpanApplier;
 import org.chromium.ui.text.SpanApplier.SpanInfo;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.List;
 
 /**
@@ -56,6 +59,13 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On
         void currentSearchEngineDetermined(int selectedIndex);
     }
 
+    private static final int TYPE_LOCATION_ALLOW = 0;
+    private static final int TYPE_LOCATION_BLOCK = 1;
+    private static final int TYPE_LOCATION_ASK = 2;
+    @IntDef({TYPE_LOCATION_ALLOW, TYPE_LOCATION_BLOCK, TYPE_LOCATION_ASK})
+    @Retention(RetentionPolicy.SOURCE)
+    private @interface LocationType {}

[Peter Kasting, 2016/11/18 20:25:43]

Can you just return the ContentSetting directly?  That would simplify a lot of
code, and I'm not sure what this separate LocationType thing buys you.

[ltian, 2016/11/18 23:10:51]

Done.

+
     // The current context.
     private Context mContext;
 
@@ -205,23 +215,27 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On
         TextView link = (TextView) view.findViewById(R.id.link);
         link.setVisibility(selected ? View.VISIBLE : View.GONE);
         if (selected) {
-            ForegroundColorSpan linkSpan = new ForegroundColorSpan(
-                    ApiCompatibilityUtils.getColor(resources, R.color.pref_accent_color));
-            if (LocationUtils.getInstance().isSystemLocationSettingEnabled()) {
-                String message = mContext.getString(
-                        locationEnabled(position, true)
-                        ? R.string.search_engine_location_allowed
-                        : R.string.search_engine_location_blocked);
-                SpannableString messageWithLink = new SpannableString(message);
-                messageWithLink.setSpan(linkSpan, 0, messageWithLink.length(), 0);
-                link.setText(messageWithLink);
+            if (shouldShowLocationInfo(position)) {
+                ForegroundColorSpan linkSpan = new ForegroundColorSpan(
+                        ApiCompatibilityUtils.getColor(resources, R.color.pref_accent_color));
+                if (LocationUtils.getInstance().isSystemLocationSettingEnabled()) {
+                    String message = mContext.getString(
+                            locationEnabled(position, true)
+                            ? R.string.search_engine_location_allowed
+                            : R.string.search_engine_location_blocked);
+                    SpannableString messageWithLink = new SpannableString(message);
+                    messageWithLink.setSpan(linkSpan, 0, messageWithLink.length(), 0);
+                    link.setText(messageWithLink);
+                } else {
+                    link.setText(SpanApplier.applySpans(
+                            mContext.getString(R.string.android_location_off),
+                            new SpanInfo("<link>", "</link>", linkSpan)));
+                }
+
+                link.setOnClickListener(this);
             } else {
-                link.setText(SpanApplier.applySpans(
-                        mContext.getString(R.string.android_location_off),
-                        new SpanInfo("<link>", "</link>", linkSpan)));
+                link.setVisibility(View.GONE);
             }
-
-            link.setOnClickListener(this);
         }
 
         return view;
@@ -287,17 +301,37 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On
         }
     }
 
-    private boolean locationEnabled(int position, boolean checkGeoHeader) {
-        if (position == -1) return false;
+    @LocationType private int getLocationPermissionType(int position, boolean checkGeoHeader) {
+        if (position == -1) {
+            return TYPE_LOCATION_BLOCK;
+        }
 
         String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
                 toIndex(position));
         GeolocationInfo locationSettings = new GeolocationInfo(url, null, false);
         ContentSetting locationPermission = locationSettings.getContentSetting();
-        // Handle the case where the geoHeader being sent when no permission has been specified.
-        if (locationPermission == ContentSetting.ASK && checkGeoHeader) {
-            return GeolocationHeader.isGeoHeaderEnabledForUrl(mContext, url, false);
+        if (locationPermission == ContentSetting.ASK) {
+            // Handle the case where the geoHeader being sent when no permission has been specified.
+            if (checkGeoHeader && GeolocationHeader.isGeoHeaderEnabledForUrl(

[Peter Kasting, 2016/11/18 20:25:43]

Nit: Simpler:

  if (checkGeoHeader && ...)
    locationPermission = ContentSetting.ALLOW;

Then just return the mapped type directly.  (If you return a ContentSetting,
this is just a simple return statement.)

[ltian, 2016/11/18 23:10:51]

Done.

+                    mContext, url, false)) {
+                return TYPE_LOCATION_ALLOW;
+            } else {
+                return TYPE_LOCATION_ASK;
+            }
+        } else if (locationPermission == ContentSetting.ALLOW) {
+            return TYPE_LOCATION_ALLOW;
+        } else {
+            return TYPE_LOCATION_BLOCK;
         }
-        return locationPermission == ContentSetting.ALLOW;
+    }
+
+    private boolean locationEnabled(int position, boolean checkGeoHeader) {
+        return getLocationPermissionType(position, checkGeoHeader) == TYPE_LOCATION_ALLOW;
+    }
+
+    // For Google search engine, if location permission is granted for the whole app,
+    // even its own location permission is unset, location permission is still granted for it.

[Peter Kasting, 2016/11/18 20:25:43]

Nit: This comment no longer seems relevant.

[ltian, 2016/11/18 23:10:51]

Done.

+    private boolean shouldShowLocationInfo(int position) {
+        return getLocationPermissionType(position, true) != TYPE_LOCATION_ASK;

[Peter Kasting, 2016/11/18 20:25:43]

Nit: Could just inline this into the lone caller, which might be clearer.

[ltian, 2016/11/18 23:10:51]

Done.

     }
 }