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

chrome/browser/search_engines/template_url_service_android.cc

Index: chrome/browser/search_engines/template_url_service_android.cc
diff --git a/chrome/browser/search_engines/template_url_service_android.cc b/chrome/browser/search_engines/template_url_service_android.cc
index 6563f7e0d9fe6723a26a201d775e14c8373f0bb8..6db57b2a52d8f7d7d794cb2d8152680e8ea92435 100644
--- a/chrome/browser/search_engines/template_url_service_android.cc
+++ b/chrome/browser/search_engines/template_url_service_android.cc
@@ -57,29 +57,46 @@ void TemplateUrlServiceAndroid::Load(JNIEnv* env,
 void TemplateUrlServiceAndroid::SetUserSelectedDefaultSearchProvider(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    jint selected_index) {
-  std::vector<TemplateURL*> template_urls =
-      template_url_service_->GetTemplateURLs();
-  size_t selected_index_size_t = static_cast<size_t>(selected_index);
-  DCHECK_LT(selected_index_size_t, template_urls.size()) <<
-      "Wrong index for search engine";
-
-  TemplateURL* template_url = template_urls[selected_index_size_t];
+    const JavaParamRef<jstring>& jkeyword) {
+  base::string16 keyword(
+      base::android::ConvertJavaStringToUTF16(env, jkeyword));
+  TemplateURL* template_url =
+      template_url_service_->FindNonExtensionTemplateURLForKeyword(keyword);
   template_url_service_->SetUserSelectedDefaultSearchProvider(template_url);
 }
 
 jint TemplateUrlServiceAndroid::GetDefaultSearchProvider(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
+  size_t index = -1;
+  template_url_list.clear();
   std::vector<TemplateURL*> template_urls =
       template_url_service_->GetTemplateURLs();
   TemplateURL* default_search_provider =
       template_url_service_->GetDefaultSearchProvider();
+  std::vector<TemplateURL*> recent_visit_list;
   for (size_t i = 0; i < template_urls.size(); ++i) {
-    if (default_search_provider == template_urls[i])
-      return i;
+    if (IsPrepopulatedTemplate(template_urls[i]) ||
+        template_urls[i]->created_by_policy()) {
+      template_url_list.push_back(template_urls[i]);
+      if (default_search_provider == template_urls[i])
+        index = template_url_list.size() - 1;

[Ian Wen, 2016/10/17 23:32:21]

Why is index always size() - 1? Shouldn't it be 1?

Also you should create a helper static method that returns whether a template
url is in the default list or not, so that you don't need to check
isPrepopulated and created_by_policy all the time.

[ltian, 2016/10/18 01:01:55]

Our plan is if the default search engine is a recently visited engine, we
manually append it to the bottom of top list, otherwise keep the order as it is
provided. So it is not always 1.

+    } else if (default_search_provider == template_urls[i]) {
+      continue;
+    } else {
+      recent_visit_list.push_back(template_urls[i]);
+    }
+  }
+  // If default search engine is recently visited one, force it

[Ian Wen, 2016/10/17 23:32:21]

Nit: force it *to* append

[ltian, 2016/10/18 01:01:55]

Done.

+  // append at the bottom of default list.
+  if (!IsPrepopulatedTemplate(default_search_provider) &&
+      !default_search_provider->created_by_policy()) {
+    template_url_list.push_back(default_search_provider);
+    index = template_url_list.size() - 1;
   }
-  return -1;
+  for (size_t i = 0; i < recent_visit_list.size(); ++i)
+    template_url_list.push_back(recent_visit_list[i]);
+  return index;
 }
 
 jboolean TemplateUrlServiceAndroid::IsLoaded(JNIEnv* env,
@@ -90,7 +107,7 @@ jboolean TemplateUrlServiceAndroid::IsLoaded(JNIEnv* env,
 jint TemplateUrlServiceAndroid::GetTemplateUrlCount(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
-  return template_url_service_->GetTemplateURLs().size();
+  return template_url_list.size();
 }
 
 jboolean TemplateUrlServiceAndroid::IsSearchProviderManaged(
@@ -124,12 +141,13 @@ base::android::ScopedJavaLocalRef<jobject>
 TemplateUrlServiceAndroid::GetTemplateUrlAt(JNIEnv* env,
                                             const JavaParamRef<jobject>& obj,
                                             jint index) {
-  TemplateURL* template_url = template_url_service_->GetTemplateURLs()[index];
+  TemplateURL* template_url = template_url_list[index];

[Ian Wen, 2016/10/17 23:32:21]

Since this class does not own the template_urls that it stores, you should add
null check here and return null if a template url is somehow deleted in the
service.

[ltian, 2016/10/18 01:01:55]

Done.

   return Java_TemplateUrl_create(
       env, index,
       base::android::ConvertUTF16ToJavaString(env, template_url->short_name()),
-      IsPrepopulatedTemplate(template_url) ||
-          template_url->created_by_policy());
+      base::android::ConvertUTF8ToJavaString(env, template_url->url()),
+      IsPrepopulatedTemplate(template_url) || template_url->created_by_policy(),
+      base::android::ConvertUTF16ToJavaString(env, template_url->keyword()));
 }
 
 bool TemplateUrlServiceAndroid::IsPrepopulatedTemplate(TemplateURL* url) {
@@ -260,8 +278,11 @@ base::android::ScopedJavaLocalRef<jstring>
 TemplateUrlServiceAndroid::GetSearchEngineUrlFromTemplateUrl(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    jint index) {
-  TemplateURL* template_url = template_url_service_->GetTemplateURLs()[index];
+    const JavaParamRef<jstring>& jkeyword) {
+  base::string16 keyword(
+      base::android::ConvertJavaStringToUTF16(env, jkeyword));
+  TemplateURL* template_url =
+      template_url_service_->FindNonExtensionTemplateURLForKeyword(keyword);
   std::string url(template_url->url_ref().ReplaceSearchTerms(
       TemplateURLRef::SearchTermsArgs(base::ASCIIToUTF16("query")),
       template_url_service_->search_terms_data()));