[Android] Allow setting recently visited search engines as default search engine

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.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.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.locale.LocaleManager;
 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.util.ArrayList;
 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 VIEW_TYPE_ITEM = 0;
+    private static final int VIEW_TYPE_DIVIDER = 1;
+

[gone, 2016/11/28 21:37:02]

Remove newline above

[ltian, 2016/11/29 02:44:02]

Done.

+    private static final int VIEW_TYPE_COUNT = 2;
 
     // The current context.

[gone, 2016/11/28 21:37:02]

Switch these to /** javadoc style */ comments so they pop up in IDEs when you
hover over them.

[ltian, 2016/11/29 02:44:02]

Done.

     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 prepopluated and default search engines.
+    private List<TemplateUrl> mPrepopulatedSearchEngines = new ArrayList<TemplateUrl>();

[gone, 2016/11/28 21:37:02]

= new ArrayList<>();

[ltian, 2016/11/29 02:44:02]

Done.

 
-    // The list of available search engines.
-    private List<TemplateUrl> mSearchEngines;
-    // The position (index into mSearchEngines) of the currently selected search engine. Can be -1
-    // if current search engine is managed and set to something other than the pre-populated values.
+    // The list of recently visited search engines.
+    private List<TemplateUrl> mRecentSearchEngines = new ArrayList<TemplateUrl>();
+
+    // The position (index into mPrepopulatedSearchEngines) of the currently selected search engine.
+    // Can be -1 if current search engine is managed and set to something other than the
+    // pre-populated values.
     private int mSelectedSearchEnginePosition = -1;
 
     // The position of the default search engine before user's action.
     private int mInitialEnginePosition = -1;
 
     /**
      * Construct a SearchEngineAdapter.
      * @param context The current context.
-     * @param callback The callback to use to communicate back.
      */
-    public SearchEngineAdapter(Context context, SelectSearchEngineCallback callback) {
+    public SearchEngineAdapter(Context context) {
         mContext = context;
         mLayoutInflater = (LayoutInflater) mContext.getSystemService(
                 Context.LAYOUT_INFLATER_SERVICE);
-        mCallback = callback;
 
         initEntries();
     }
 
-    /**
-     * @return The index of the selected engine before user's action.
-     */
-    public int getInitialSearchEnginePosition() {
-        return mInitialEnginePosition;
-    }
-
     // Used for testing.
 
     String getValueForTesting() {

[gone, 2016/11/28 21:37:02]

Adding @VisibleForTesting or naming the function SomethingForTesting() usually
implies that without the comment.

[ltian, 2016/11/29 02:44:02]

Done.

         return Integer.toString(mSelectedSearchEnginePosition);
     }
 
-    void setValueForTesting(String value) {
-        searchEngineSelected(Integer.parseInt(value));
+    String setValueForTesting(String value) {
+        return searchEngineSelected(Integer.parseInt(value));
+    }
+
+    String getKeywordForTesting(int index) {
+        return toKeyword(index);
     }
 
     /**
      * Initialize the search engine list.
      */
     private void initEntries() {
         TemplateUrlService templateUrlService = TemplateUrlService.getInstance();
         if (!templateUrlService.isLoaded()) {
             templateUrlService.registerLoadListener(this);
             templateUrlService.load();
             return;  // Flow continues in onTemplateUrlServiceLoaded below.
         }
 
-        // Fetch all the search engine info and the currently active one.
-        mSearchEngines = templateUrlService.getLocalizedSearchEngines();
-        int searchEngineIndex = templateUrlService.getDefaultSearchEngineIndex();
-        // Convert the TemplateUrl index into an index into mSearchEngines.
+        int defaultSearchEngineIndex = templateUrlService.getDefaultSearchEngineIndex();
+        for (TemplateUrl templateUrl : templateUrlService.getSearchEngines()) {
+            if (templateUrl.getType() == TemplateUrlService.TYPE_PREPOPULATED
+                    || templateUrl.getType() == TemplateUrlService.TYPE_DEFAULT) {
+                mPrepopulatedSearchEngines.add(templateUrl);
+            } else {
+                mRecentSearchEngines.add(templateUrl);
+            }
+        }
+
+        // Convert the TemplateUrl index into an index of mSearchEngines.
         mSelectedSearchEnginePosition = -1;
-        for (int i = 0; i < mSearchEngines.size(); ++i) {
-            if (mSearchEngines.get(i).getIndex() == searchEngineIndex) {
+        for (int i = 0; i < mPrepopulatedSearchEngines.size(); ++i) {
+            if (mPrepopulatedSearchEngines.get(i).getIndex() == defaultSearchEngineIndex) {
                 mSelectedSearchEnginePosition = i;
             }
         }
+
+        for (int i = 0; i < mRecentSearchEngines.size(); ++i) {
+            if (mRecentSearchEngines.get(i).getIndex() == defaultSearchEngineIndex) {
+                // Add one to offset the title for recent search engine list

[gone, 2016/11/28 21:37:02]

1) Add period to end of the comment
2) the title for the recent search engine list

[ltian, 2016/11/29 02:44:02]

Done.

+                mSelectedSearchEnginePosition = i + mPrepopulatedSearchEngines.size() + 1;
+            }
+        }
+
         mInitialEnginePosition = mSelectedSearchEnginePosition;
 
-        // Report back what is selected.
-        mCallback.currentSearchEngineDetermined(toIndex(mSelectedSearchEnginePosition));
+        TemplateUrlService.getInstance().setSearchEngine(toKeyword(mSelectedSearchEnginePosition));
     }
 
-    private int toIndex(int position) {
-        return mSearchEngines.get(position).getIndex();
+    private String toKeyword(int position) {
+        if (position < mPrepopulatedSearchEngines.size()) {
+            return mPrepopulatedSearchEngines.get(position).getKeyword();
+        } else {
+            position -= mPrepopulatedSearchEngines.size() + 1;

[gone, 2016/11/28 21:37:02]

You use "mPrepopulatedSearchEngines.size() + 1" in a lot of places; you should
pull it out so that it's easy to fix if the list changes again, and so you can
explain why you need to +1 it in exactly one place.  I can't think of anything
better than "computeStartIndexForRecentSearchEngines" unfortunately :/

[ltian, 2016/11/29 02:44:02]

Done.

+            return mRecentSearchEngines.get(position).getKeyword();
+        }
     }
 
     // BaseAdapter:
 
     @Override
     public int getCount() {
-        return mSearchEngines == null ? 0 : mSearchEngines.size();
+        return mPrepopulatedSearchEngines == null
+                ? 0
+                : mPrepopulatedSearchEngines.size() + mRecentSearchEngines.size() + 1;
+    }
+
+    @Override
+    public int getViewTypeCount() {
+        return VIEW_TYPE_COUNT;
     }
 
     @Override
     public Object getItem(int pos) {
-        TemplateUrl templateUrl = mSearchEngines.get(pos);
-        return templateUrl.getShortName();
+        if (pos < mPrepopulatedSearchEngines.size()) {
+            return mPrepopulatedSearchEngines.get(pos);
+        } else if (pos > mPrepopulatedSearchEngines.size()) {
+            pos -= mPrepopulatedSearchEngines.size() + 1;
+            return mRecentSearchEngines.get(pos);
+        }
+        return null;
     }
 
     @Override
     public long getItemId(int position) {
         return position;
     }
 
     @Override
+    public int getItemViewType(int position) {
+        if (position == mPrepopulatedSearchEngines.size()) {
+            return VIEW_TYPE_DIVIDER;
+        } else {
+            return VIEW_TYPE_ITEM;
+        }
+    }
+
+    @Override
     public View getView(int position, View convertView, ViewGroup parent) {
         View view = convertView;
+        TemplateUrl templateUrl = (TemplateUrl) getItem(position);
+        int itemViewType = getItemViewType(position);
         if (convertView == null) {
-            view = mLayoutInflater.inflate(R.layout.search_engine, null);
+            if (itemViewType == VIEW_TYPE_DIVIDER) {
+                view = mLayoutInflater.inflate(R.layout.search_engine_recent_title, null);
+            } else {
+                view = mLayoutInflater.inflate(R.layout.search_engine, null);
+            }

[gone, 2016/11/28 21:37:02]

view = mLayoutInflater.inflate(itemViewType == VIEW_TYPE_DIVIDER ?
R.layout.search_engine_recent_title : R.layout.search_engine, null);

[ltian, 2016/11/29 02:44:02]

Done.

+        }
+        if (itemViewType == VIEW_TYPE_DIVIDER) {
+            return view;
         }
 
         view.setOnClickListener(this);
         view.setTag(position);
 
         // TODO(finnur): There's a tinting bug in the AppCompat lib (see http://crbug.com/474695),
         // which causes the first radiobox to always appear selected, even if it is not. It is being
         // addressed, but in the meantime we should use the native RadioButton instead.
         RadioButton radioButton = (RadioButton) view.findViewById(R.id.radiobutton);
         // On Lollipop this removes the redundant animation ring on selection but on older versions
         // it would cause the radio button to disappear.
         // TODO(finnur): Remove the encompassing if statement once we go back to using the AppCompat
         // control.
         final boolean selected = position == mSelectedSearchEnginePosition;
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
             radioButton.setBackgroundResource(0);
         }
         radioButton.setChecked(selected);
 
-        TextView description = (TextView) view.findViewById(R.id.description);
-        TemplateUrl templateUrl = mSearchEngines.get(position);
+        TextView description = (TextView) view.findViewById(R.id.name);
         Resources resources = mContext.getResources();
         description.setText(templateUrl.getShortName());
 
+        TextView url = (TextView) view.findViewById(R.id.url);
+        url.setText(templateUrl.getUrl());
+        if (templateUrl.getType() == TemplateUrlService.TYPE_PREPOPULATED
+                || templateUrl.getType() == TemplateUrlService.TYPE_DEFAULT
+                || templateUrl.getUrl().length() == 0) {
+            url.setVisibility(View.GONE);
+        }
+
         // To improve the explore-by-touch experience, the radio button is hidden from accessibility
         // and instead, "checked" or "not checked" is read along with the search engine's name, e.g.
         // "google.com checked" or "google.com not checked".
         radioButton.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_NO);
         description.setAccessibilityDelegate(new AccessibilityDelegate() {
             @Override
             public void onInitializeAccessibilityEvent(View host, AccessibilityEvent event) {
                 super.onInitializeAccessibilityEvent(host, event);
                 event.setChecked(selected);
             }
 
             @Override
             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);
+        TextView link = (TextView) view.findViewById(R.id.location_permission);
         link.setVisibility(selected ? View.VISIBLE : View.GONE);
         if (selected) {
             if (getLocationPermissionType(position, true) == ContentSetting.ASK) {
                 link.setVisibility(View.GONE);
             } else {
                 ForegroundColorSpan linkSpan = new ForegroundColorSpan(
                         ApiCompatibilityUtils.getColor(resources, R.color.pref_accent_color));
                 if (LocationUtils.getInstance().isSystemLocationSettingEnabled()) {
                     String message = mContext.getString(
                             locationEnabled(position, true)
@@ skipping 28 matching lines @@
 
     @Override
     public void onClick(View view) {
         if (view.getTag() == null) {
             onLocationLinkClicked();
         } else {
             searchEngineSelected((int) view.getTag());
         }
     }
 
-    private void searchEngineSelected(int position) {
+    private String searchEngineSelected(int position) {
         // First clean up any automatically added permissions (if any) for the previously selected
         // search engine.
         SharedPreferences sharedPreferences =
                 ContextUtils.getAppSharedPreferences();
         if (sharedPreferences.getBoolean(PrefServiceBridge.LOCATION_AUTO_ALLOWED, false)) {
             if (locationEnabled(mSelectedSearchEnginePosition, false)) {
                 String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
-                        toIndex(mSelectedSearchEnginePosition));
+                        toKeyword(mSelectedSearchEnginePosition));
                 WebsitePreferenceBridge.nativeSetGeolocationSettingForOrigin(
                         url, url, ContentSetting.DEFAULT.toInt(), false);
             }
             sharedPreferences.edit().remove(PrefServiceBridge.LOCATION_AUTO_ALLOWED).apply();
         }
 
         // Record the change in search engine.
         mSelectedSearchEnginePosition = position;
 
-        // Report the change back.
-        mCallback.currentSearchEngineDetermined(toIndex(mSelectedSearchEnginePosition));
+        String keyword = toKeyword(mSelectedSearchEnginePosition);
+        TemplateUrlService.getInstance().setSearchEngine(keyword);
 
+        // If the user has manually set the default search engine, disable auto switching.
+        boolean manualSwitch = mSelectedSearchEnginePosition != mInitialEnginePosition;
+        if (manualSwitch) {
+            RecordUserAction.record("SearchEngine_ManualChange");
+            LocaleManager.getInstance().setSearchEngineAutoSwitch(false);
+        }
         notifyDataSetChanged();
+        return keyword;
     }
 
     private void onLocationLinkClicked() {
         if (!LocationUtils.getInstance().isSystemLocationSettingEnabled()) {
             mContext.startActivity(LocationUtils.getInstance().getSystemLocationSettingsIntent());
         } else {
             Intent settingsIntent = PreferencesLauncher.createIntentForSettingsPage(
                     mContext, SingleWebsitePreferences.class.getName());
             String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
-                    toIndex(mSelectedSearchEnginePosition));
+                    toKeyword(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 ContentSetting getLocationPermissionType(int position, boolean checkGeoHeader) {
         if (position == -1) {
             return ContentSetting.BLOCK;
         }
 
         String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl(
-                toIndex(position));
+                toKeyword(position));
         GeolocationInfo locationSettings = new GeolocationInfo(url, null, false);
         ContentSetting locationPermission = locationSettings.getContentSetting();
         if (locationPermission == ContentSetting.ASK) {
             // Handle the case where the geoHeader being sent when no permission has been specified.
             if (checkGeoHeader && GeolocationHeader.isGeoHeaderEnabledForUrl(
                     mContext, url, false)) {
                 locationPermission = ContentSetting.ALLOW;
             }
         }
         return locationPermission;
     }
 
     private boolean locationEnabled(int position, boolean checkGeoHeader) {
         return getLocationPermissionType(position, checkGeoHeader) == ContentSetting.ALLOW;
     }
 
 }