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..211f5ba2ef36819e1b6c20d34e5f3953185b6f32 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 |
@@ -25,7 +25,9 @@ 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; |
@@ -38,6 +40,7 @@ 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; |
/** |
@@ -53,7 +56,7 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
* one. |
* @param name Provides the name of it (with a simplified URL in parenthesis). |
Ian Wen
2016/10/19 00:03:31
You should update the javadoc for this function.
ltian
2016/10/19 18:56:31
Interface will be removed and this class will dire
|
*/ |
- void currentSearchEngineDetermined(int selectedIndex); |
+ void currentSearchEngineDetermined(String keyword); |
} |
// The current context. |
@@ -65,15 +68,21 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
// The callback to use for notifying caller of progress. |
private SelectSearchEngineCallback mCallback; |
- // The list of available search engines. |
- private List<TemplateUrl> mSearchEngines; |
- // The position (index into mSearchEngines) of the currently selected search engine. Can be -1 |
+ // The list of prepopluated and default search engines. |
+ private List<TemplateUrl> mPrepopulatedSearchEngines = new ArrayList<TemplateUrl>(); |
+ // The list of recently visited search engines. |
+ private List<TemplateUrl> mRecentVisitedSearchEngines = new ArrayList<TemplateUrl>(); |
Ian Wen
2016/10/19 00:03:30
How about renaming it to mRecentSearchEngines?
ltian
2016/10/19 18:56:31
Done.
|
+ // 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. |
Ian Wen
2016/10/19 00:03:31
Please reformat the comments.
ltian
2016/10/19 18:56:31
Done.
|
private int mSelectedSearchEnginePosition = -1; |
// The position of the default search engine before user's action. |
private int mInitialEnginePosition = -1; |
+ private static final int VIEW_TYPE_ITEM = 0; |
+ private static final int VIEW_TYPE_DIVIDER = 1; |
Ian Wen
2016/10/19 00:03:31
Like what I said in last patchset, public members
ltian
2016/10/19 18:56:31
Done.
|
+ |
/** |
* Construct a SearchEngineAdapter. |
* @param context The current context. |
@@ -88,21 +97,18 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
initEntries(); |
} |
- /** |
- * @return The index of the selected engine before user's action. |
- */ |
- public int getInitialSearchEnginePosition() { |
- return mInitialEnginePosition; |
- } |
- |
// Used for testing. |
String getValueForTesting() { |
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); |
} |
/** |
@@ -116,37 +122,65 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
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(); |
+ // Fetch all the search engine info and the currently active one, based on their types, |
+ // add into different lists. |
Ian Wen
2016/10/19 00:03:31
I would simply remove the comments between 126 and
ltian
2016/10/19 18:56:31
Done.
|
+ for (TemplateUrl templateUrl : templateUrlService.getLocalizedSearchEngines()) { |
Ian Wen
2016/10/19 00:03:31
I would rename getLocalizedSearchEngines() to getS
ltian
2016/10/19 18:56:31
Done.
|
+ if (templateUrl.getType() == TemplateUrlService.TYPE_PREPOPULATED |
+ || templateUrl.getType() == TemplateUrlService.TYPE_DEFAULT) { |
+ mPrepopulatedSearchEngines.add(templateUrl); |
+ } else { |
+ mRecentVisitedSearchEngines.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 < mRecentVisitedSearchEngines.size(); ++i) { |
+ if (mRecentVisitedSearchEngines.get(i).getIndex() == defaultSearchEngineIndex) { |
+ mSelectedSearchEnginePosition = i + mPrepopulatedSearchEngines.size() + 1; |
Ian Wen
2016/10/19 00:03:31
Please explain why you need to add one here.
"//
ltian
2016/10/19 18:56:31
Done.
|
+ } |
+ } |
+ |
mInitialEnginePosition = mSelectedSearchEnginePosition; |
// Report back what is selected. |
- mCallback.currentSearchEngineDetermined(toIndex(mSelectedSearchEnginePosition)); |
+ mCallback.currentSearchEngineDetermined(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; |
+ return mRecentVisitedSearchEngines.get(position).getKeyword(); |
+ } |
} |
// BaseAdapter: |
@Override |
public int getCount() { |
- return mSearchEngines == null ? 0 : mSearchEngines.size(); |
+ return mPrepopulatedSearchEngines == null |
+ ? 0 |
+ : mPrepopulatedSearchEngines.size() + mRecentVisitedSearchEngines.size() + 1; |
} |
@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 mRecentVisitedSearchEngines.get(pos); |
+ } |
+ return null; |
} |
@Override |
@@ -155,11 +189,29 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
} |
@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; |
- if (convertView == null) { |
+ TemplateUrl templateUrl = (TemplateUrl) getItem(position); |
+ int itemViewType = getItemViewType(position); |
+ if (itemViewType == VIEW_TYPE_DIVIDER) { |
Ian Wen
2016/10/19 00:03:31
Oops this is a mistake.
The reason why you need t
ltian
2016/10/19 18:56:31
Done.
|
+ view = mLayoutInflater.inflate(R.layout.search_engine_recent_title, null); |
+ } else { |
view = mLayoutInflater.inflate(R.layout.search_engine, null); |
} |
+ if (itemViewType == VIEW_TYPE_DIVIDER) { |
+ view.setOnClickListener(null); |
+ view.setEnabled(false); |
+ return view; |
+ } |
view.setOnClickListener(this); |
view.setTag(position); |
@@ -179,10 +231,17 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
radioButton.setChecked(selected); |
TextView description = (TextView) view.findViewById(R.id.description); |
- TemplateUrl templateUrl = mSearchEngines.get(position); |
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". |
@@ -247,7 +306,7 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
} |
} |
- 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 = |
@@ -255,7 +314,7 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
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); |
} |
@@ -265,10 +324,18 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
// Record the change in search engine. |
mSelectedSearchEnginePosition = position; |
+ String keyword = toKeyword(mSelectedSearchEnginePosition); |
// Report the change back. |
- mCallback.currentSearchEngineDetermined(toIndex(mSelectedSearchEnginePosition)); |
+ mCallback.currentSearchEngineDetermined(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() { |
@@ -278,7 +345,7 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
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)); |
@@ -291,7 +358,7 @@ public class SearchEngineAdapter extends BaseAdapter implements LoadListener, On |
if (position == -1) return false; |
String url = TemplateUrlService.getInstance().getSearchEngineUrlFromTemplateUrl( |
- toIndex(position)); |
+ toKeyword(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. |