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

 // Copyright 2015 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.
 
 package org.chromium.chrome.browser.preferences;
 
 import android.content.Context;
 import android.content.Intent;
 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;
 import android.view.View;
 import android.view.View.AccessibilityDelegate;
 import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.view.accessibility.AccessibilityEvent;
 import android.view.accessibility.AccessibilityNodeInfo;
 import android.widget.BaseAdapter;
 import android.widget.RadioButton;
 import android.widget.TextView;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
 import org.chromium.chrome.browser.preferences.website.ContentSetting;
 import org.chromium.chrome.browser.preferences.website.GeolocationInfo;
 import org.chromium.chrome.browser.preferences.website.SingleWebsitePreferences;
 import org.chromium.chrome.browser.preferences.website.WebsitePreferenceBridge;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService.LoadListener;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrl;
 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;
 
 /**
 * A custom adapter for listing search engines.
 */
 public class SearchEngineAdapter extends BaseAdapter implements LoadListener, OnClickListener {
     /**
      * A callback for reporting progress to the owner.
      */
     public interface SelectSearchEngineCallback {
         /**
          * Called when the search engine data has loaded and we've determined the currently active
          * one.
          * @param name Provides the name of it (with a simplified URL in parenthesis).
          */
         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;
 
     // The layout inflater to use for the custom views.
     private LayoutInflater mLayoutInflater;
 
     // The callback to use for notifying caller of progress.
     private SelectSearchEngineCallback mCallback;
 
     // The list of available search engines.
@@ skipping 129 matching lines @@
             public void onInitializeAccessibilityNodeInfo(View host, AccessibilityNodeInfo info) {
                 super.onInitializeAccessibilityNodeInfo(host, info);
                 info.setCheckable(true);
                 info.setChecked(selected);
             }
         });
 
         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;
     }
 
     // TemplateUrlService.LoadListener
 
     @Override
     public void onTemplateUrlServiceLoaded() {
         TemplateUrlService.getInstance().unregisterLoadListener(this);
@@ skipping 45 matching lines @@
             String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
                     toIndex(mSelectedSearchEnginePosition));
             Bundle fragmentArgs = SingleWebsitePreferences.createFragmentArgsForSite(url);
             fragmentArgs.putBoolean(SingleWebsitePreferences.EXTRA_LOCATION,
                     locationEnabled(mSelectedSearchEnginePosition, true));
             settingsIntent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, fragmentArgs);
             mContext.startActivity(settingsIntent);
         }
     }
 
-    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.

     }
 }