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

chrome/browser/search_engines/template_url_service_android.cc

 // Copyright (c) 2013 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.
 
 #include "chrome/browser/search_engines/template_url_service_android.h"
 
 #include <stddef.h>
 
 #include "base/android/jni_string.h"
 #include "base/bind.h"
@@ skipping 24 matching lines @@
 TemplateUrlServiceAndroid::TemplateUrlServiceAndroid(JNIEnv* env,
                                                      jobject obj)
     : weak_java_obj_(env, obj),
       template_url_service_(
           TemplateURLServiceFactory::GetForProfile(GetOriginalProfile())) {
   template_url_subscription_ =
       template_url_service_->RegisterOnLoadedCallback(
           base::Bind(&TemplateUrlServiceAndroid::OnTemplateURLServiceLoaded,
                      base::Unretained(this)));
   template_url_service_->AddObserver(this);
+  if (template_url_service_->loaded() && template_urls_.size() == 0) {

[Peter Kasting, 2016/11/29 03:32:18]

Nit: Prefer .empty() to .size() == 0.  {} not necessary

[ltian, 2016/11/30 06:36:17]

Done.

+    LoadTemplateURLs();
+  }
 }
 
 TemplateUrlServiceAndroid::~TemplateUrlServiceAndroid() {
   template_url_service_->RemoveObserver(this);
 }
 
 void TemplateUrlServiceAndroid::Load(JNIEnv* env,
                                      const JavaParamRef<jobject>& obj) {
   template_url_service_->Load();
 }
 
 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_->GetTemplateURLForKeyword(keyword);
   template_url_service_->SetUserSelectedDefaultSearchProvider(template_url);
 }
 
-jint TemplateUrlServiceAndroid::GetDefaultSearchProvider(
+jint TemplateUrlServiceAndroid::GetDefaultSearchProviderIndex(
     JNIEnv* env,
-    const JavaParamRef<jobject>& obj) {
-  std::vector<TemplateURL*> template_urls =
-      template_url_service_->GetTemplateURLs();
+    const base::android::JavaParamRef<jobject>& obj) const {
   TemplateURL* default_search_provider =
-      template_url_service_->GetDefaultSearchProvider();
-  for (size_t i = 0; i < template_urls.size(); ++i) {
-    if (default_search_provider == template_urls[i])
-      return i;
-  }
-  return -1;
+        template_url_service_->GetDefaultSearchProvider();
+  auto it = std::find(template_urls_.begin(), template_urls_.end(),
+                      default_search_provider);
+  size_t default_search_provider_index_ = (it == template_urls_.end())
+                                         ? -1
+                                         : (it - template_urls_.begin());

[Peter Kasting, 2016/11/29 03:32:18]

Nit: Indenting looks a bit odd.  Is this what git cl format did?

[ltian, 2016/11/30 06:36:17]

Sorry the Eclipse setting and git cl format do in different ways. Change it to
use git cl format.

+  return default_search_provider_index_;
 }
 
-jboolean TemplateUrlServiceAndroid::IsLoaded(JNIEnv* env,
-                                             const JavaParamRef<jobject>& obj) {
+jboolean TemplateUrlServiceAndroid::IsLoaded(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj) const {
   return template_url_service_->loaded();
 }
 
 jint TemplateUrlServiceAndroid::GetTemplateUrlCount(
     JNIEnv* env,
-    const JavaParamRef<jobject>& obj) {
-  return template_url_service_->GetTemplateURLs().size();
+    const JavaParamRef<jobject>& obj) const {
+  return template_urls_.size();
 }
 
 jboolean TemplateUrlServiceAndroid::IsSearchProviderManaged(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
   return template_url_service_->is_default_search_managed();
 }
 
 jboolean TemplateUrlServiceAndroid::IsSearchByImageAvailable(
     JNIEnv* env,
@@ skipping 12 matching lines @@
   TemplateURL* default_search_provider =
       template_url_service_->GetDefaultSearchProvider();
   return default_search_provider &&
       default_search_provider->url_ref().HasGoogleBaseURLs(
           template_url_service_->search_terms_data());
 }
 
 base::android::ScopedJavaLocalRef<jobject>
 TemplateUrlServiceAndroid::GetTemplateUrlAt(JNIEnv* env,
                                             const JavaParamRef<jobject>& obj,
-                                            jint index) {
-  TemplateURL* template_url = template_url_service_->GetTemplateURLs()[index];
+                                            jint index) const {
+  TemplateURL* template_url = template_urls_[index];
   return Java_TemplateUrl_create(
       env, index,
       base::android::ConvertUTF16ToJavaString(env, template_url->short_name()),
-      IsPrepopulatedTemplate(template_url) ||
-          template_url->created_by_policy());
-}
-
-bool TemplateUrlServiceAndroid::IsPrepopulatedTemplate(TemplateURL* url) {
-  return url->prepopulate_id() > 0;
+      base::android::ConvertUTF8ToJavaString(env, template_url->url()),
+      template_url_service_->ShowInPrepopulatedList(template_url),
+      base::android::ConvertUTF16ToJavaString(env, template_url->keyword()));
 }
 
 void TemplateUrlServiceAndroid::OnTemplateURLServiceLoaded() {
   template_url_subscription_.reset();
   JNIEnv* env = base::android::AttachCurrentThread();
   if (weak_java_obj_.get(env).is_null())
     return;
+  LoadTemplateURLs();
 
   Java_TemplateUrlService_templateUrlServiceLoaded(env,
                                                    weak_java_obj_.get(env));
 }
 
+void TemplateUrlServiceAndroid::LoadTemplateURLs() {
+  template_urls_.clear();

[Peter Kasting, 2016/11/29 03:32:18]

This call is unnecessary, since you reset template_urls_ just below.

[ltian, 2016/11/30 06:36:17]

Done.

+
+  template_urls_ = template_url_service_->GetTemplateURLs();
+  TemplateURL* default_search_provider =
+      template_url_service_->GetDefaultSearchProvider();
+
+  auto typeCompare = [&] (const TemplateURL* lhs, const TemplateURL* rhs)

[Peter Kasting, 2016/11/29 03:32:18]

Nit: Use normal variable capitalization scheme.

I don't really know what "type compare" as a name means, since you're not
comparing the types of anything, in the C++ sense of the word type.  I might use
"comp", which matches the name used in the standard for this argument to sort().

[ltian, 2016/11/30 06:36:17]

Done.

+      -> bool {

[Peter Kasting, 2016/11/29 03:32:18]

Nit: Is the return type necessary for compilation?  If not, I'd probably omit it
for brevity, as I'm not sure it adds much.

[ltian, 2016/11/30 06:36:17]

Done.

+    if (template_url_service_->ShowInPrepopulatedList(lhs)) {
+      return !template_url_service_->ShowInPrepopulatedList(rhs) ||

[Peter Kasting, 2016/11/29 03:32:18]

Nit: This call is verbose and you make it regardless of the value of the
conditional; maybe store in a temp like |rhs_prepopulated|?

[ltian, 2016/11/30 06:36:17]

Done.

+          (lhs->prepopulate_id() < rhs->prepopulate_id());
+    }
+    return (lhs == default_search_provider &&
+        !template_url_service_->ShowInPrepopulatedList(rhs));
+  };
+

[Peter Kasting, 2016/11/29 03:32:18]

Nit: I'd avoid this blank line, since the lambda just above it is declared
solely for the call below.

[ltian, 2016/11/30 06:36:17]

Done.

+  std::sort(template_urls_.begin(), template_urls_.end(), typeCompare);
+}
+
 void TemplateUrlServiceAndroid::OnTemplateURLServiceChanged() {
   JNIEnv* env = base::android::AttachCurrentThread();
   if (weak_java_obj_.get(env).is_null())
     return;
+  LoadTemplateURLs();
 
   Java_TemplateUrlService_onTemplateURLServiceChanged(env,
                                                       weak_java_obj_.get(env));
 }
 
 base::android::ScopedJavaLocalRef<jstring>
 TemplateUrlServiceAndroid::GetUrlForSearchQuery(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jstring>& jquery) {
@@ skipping 90 matching lines @@
     url = gurl.spec();
   }
 
   return base::android::ConvertUTF8ToJavaString(env, url);
 }
 
 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_->GetTemplateURLForKeyword(keyword);
   std::string url(template_url->url_ref().ReplaceSearchTerms(
       TemplateURLRef::SearchTermsArgs(base::ASCIIToUTF16("query")),
       template_url_service_->search_terms_data()));
   return base::android::ConvertUTF8ToJavaString(env, url);
 }
 
 static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   TemplateUrlServiceAndroid* template_url_service_android =
       new TemplateUrlServiceAndroid(env, obj);
   return reinterpret_cast<intptr_t>(template_url_service_android);
 }
 
 // static
 bool TemplateUrlServiceAndroid::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }