Update MostVisitedSites observer interface.

chrome/browser/android/ntp/most_visited_sites_bridge.cc

 // Copyright 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/android/ntp/most_visited_sites_bridge.h"
 
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "chrome/browser/android/ntp/popular_sites.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "jni/MostVisitedSites_jni.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "url/gurl.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ScopedJavaGlobalRef;
 using base::android::ScopedJavaLocalRef;
 using base::android::ToJavaArrayOfStrings;
 
 namespace {
 
 void CallJavaWithBitmap(
     std::unique_ptr<ScopedJavaGlobalRef<jobject>> j_callback,
     bool is_local_thumbnail,
     const SkBitmap* bitmap) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> j_bitmap;
   if (bitmap)
     j_bitmap = gfx::ConvertToJavaBitmap(bitmap);
   Java_ThumbnailCallback_onMostVisitedURLsThumbnailAvailable(
       env, j_callback->obj(), j_bitmap.obj(), is_local_thumbnail);
 }
 
 }  // namespace
 
 class MostVisitedSitesBridge::Observer
-    : public MostVisitedSitesObserver {
+    : public MostVisitedSites::Observer {
  public:
   Observer(JNIEnv* env, const JavaParamRef<jobject>& obj);
 
   void OnMostVisitedURLsAvailable(
-      const std::vector<base::string16>& titles,
-      const std::vector<std::string>& urls,
-      const std::vector<std::string>& whitelist_icon_paths) override;
+      const MostVisitedSites::SuggestionsVector& suggestions) override;
 
   void OnPopularURLsAvailable(
-      const std::vector<std::string>& urls,
-      const std::vector<std::string>& favicon_urls,
-      const std::vector<std::string>& large_icon_urls) override;
+      const MostVisitedSites::PopularSitesVector& sites) override;
 
  private:
   ScopedJavaGlobalRef<jobject> observer_;
 
   DISALLOW_COPY_AND_ASSIGN(Observer);
 };
 
 MostVisitedSitesBridge::Observer::Observer(
     JNIEnv* env, const JavaParamRef<jobject>& obj)
     : observer_(env, obj) {}
 
 void MostVisitedSitesBridge::Observer::OnMostVisitedURLsAvailable(
-    const std::vector<base::string16>& titles,
-    const std::vector<std::string>& urls,
-    const std::vector<std::string>& whitelist_icon_paths) {
+    const MostVisitedSites::SuggestionsVector& suggestions) {
   JNIEnv* env = AttachCurrentThread();
-  DCHECK_EQ(titles.size(), urls.size());
-  DCHECK_EQ(titles.size(), whitelist_icon_paths.size());
+  std::vector<base::string16> titles;
+  std::vector<std::string> urls;
+  std::vector<std::string> whitelist_icon_paths;
+  titles.reserve(suggestions.size());
+  urls.reserve(suggestions.size());

[Bernhard Bauer, 2016/04/26 14:29:58]

Why are we not doing that for |whitelist_icon_paths|? :)

[sfiera, 2016/04/26 14:42:26]

It looks like here I was just moving the old code from most_visited_sites.cc...

+  for (const auto& suggestion : suggestions) {
+    titles.push_back(suggestion.title);
+    urls.push_back(suggestion.url.spec());
+    whitelist_icon_paths.push_back(suggestion.whitelist_icon_path.value());
+  }
   Java_MostVisitedURLsObserver_onMostVisitedURLsAvailable(
       env, observer_.obj(), ToJavaArrayOfStrings(env, titles).obj(),
       ToJavaArrayOfStrings(env, urls).obj(),
       ToJavaArrayOfStrings(env, whitelist_icon_paths).obj());
 }
 
 void MostVisitedSitesBridge::Observer::OnPopularURLsAvailable(
-    const std::vector<std::string>& urls,
-    const std::vector<std::string>& favicon_urls,
-    const std::vector<std::string>& large_icon_urls) {
+    const MostVisitedSites::PopularSitesVector& sites) {
   JNIEnv* env = AttachCurrentThread();
+  std::vector<std::string> urls;
+  std::vector<std::string> favicon_urls;
+  std::vector<std::string> large_icon_urls;
+  for (const auto& site : sites) {
+    urls.emplace_back(site.url.spec());

[Bernhard Bauer, 2016/04/26 14:29:58]

Why emplace_back here, but push_back above?

[sfiera, 2016/04/26 14:42:26]

...but here I wrote it myself.

So, the inefficient, inconsistent code is the code that I didn't write :)

[Bernhard Bauer, 2016/04/26 14:50:49]

:-D

To be fair, the old code was written before emplace_back was allowed. Personally
I don't feel that strongly, but it would be nice to be consistent (and if we can
in the process improve the existing code, hey!)

+    favicon_urls.emplace_back(site.favicon_url.spec());
+    large_icon_urls.emplace_back(site.large_icon_url.spec());
+  }
   Java_MostVisitedURLsObserver_onPopularURLsAvailable(
       env, observer_.obj(), ToJavaArrayOfStrings(env, urls).obj(),
       ToJavaArrayOfStrings(env, favicon_urls).obj(),
       ToJavaArrayOfStrings(env, large_icon_urls).obj());
 }
 
 MostVisitedSitesBridge::MostVisitedSitesBridge(Profile* profile)
     : most_visited_(profile) {}
 
 MostVisitedSitesBridge::~MostVisitedSitesBridge() {}
@@ skipping 56 matching lines @@
 }
 
 static jlong Init(JNIEnv* env,
                   const JavaParamRef<jobject>& obj,
                   const JavaParamRef<jobject>& jprofile) {
   MostVisitedSitesBridge* most_visited_sites =
       new MostVisitedSitesBridge(
           ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }