[NTP] Use server thumbnails from SuggestionsService on Android NTP.

chrome/browser/android/most_visited_sites.cc

Index: chrome/browser/android/most_visited_sites.cc
diff --git a/chrome/browser/android/most_visited_sites.cc b/chrome/browser/android/most_visited_sites.cc
index 6fc326f7d9426cbad6c43d979ed6810d674a4b71..c9c3ce364927c19dd78161a3029b8df097601d95 100644
--- a/chrome/browser/android/most_visited_sites.cc
+++ b/chrome/browser/android/most_visited_sites.cc
@@ -8,6 +8,7 @@
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/callback.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -98,10 +99,31 @@ void AddForcedURLOnUIThread(scoped_refptr<history::TopSites> top_sites,
   top_sites->AddForcedURL(url, base::Time::Now());
 }
 
+void OnSuggestionsThumbnailAvailable(
+    ScopedJavaGlobalRef<jobject>* j_callback,
+    const GURL& url,
+    const SkBitmap* bitmap) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  JNIEnv* env = AttachCurrentThread();
+
+  ScopedJavaGlobalRef<jobject>* j_bitmap_ref =
+      new ScopedJavaGlobalRef<jobject>();
+  if (bitmap) {
+    j_bitmap_ref->Reset(
+        env,
+        gfx::ConvertToJavaBitmap(bitmap).obj());
+  }
+
+  Java_ThumbnailCallback_onMostVisitedURLsThumbnailAvailable(
+      env, j_callback->obj(), j_bitmap_ref->obj());
+}
+
+// Runs on the DB thread.
 void GetUrlThumbnailTask(
     std::string url_string,
     scoped_refptr<TopSites> top_sites,
-    ScopedJavaGlobalRef<jobject>* j_callback) {
+    ScopedJavaGlobalRef<jobject>* j_callback,
+    base::Closure lookup_failed_ui_callback) {
   JNIEnv* env = AttachCurrentThread();
 
   ScopedJavaGlobalRef<jobject>* j_bitmap_ref =
@@ -123,6 +145,13 @@ void GetUrlThumbnailTask(
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(AddForcedURLOnUIThread, top_sites, gurl));
+
+    // If appropriate, return on the UI thread to execute the proper callback.
+    if (!lookup_failed_ui_callback.is_null()) {
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE, lookup_failed_ui_callback);
+      return;
+    }
   }
 
   // Since j_callback is owned by this callback, when the callback falls out of
@@ -136,6 +165,16 @@ void GetUrlThumbnailTask(
           base::Owned(j_bitmap_ref), base::Owned(j_callback_pass)));
 }
 
+void GetSuggestionsThumbnailOnUIThread(
+    SuggestionsService* suggestions_service,
+    const std::string& url_string,
+    ScopedJavaGlobalRef<jobject>* j_callback) {
+  suggestions_service->GetPageThumbnail(
+      GURL(url_string),
+      base::Bind(&OnSuggestionsThumbnailAvailable,
+                 base::Owned(new ScopedJavaGlobalRef<jobject>(*j_callback))));
+}
+
 }  // namespace
 
 MostVisitedSites::MostVisitedSites(Profile* profile)
@@ -176,7 +215,7 @@ void MostVisitedSites::SetMostVisitedURLsObserver(JNIEnv* env,
   }
 }
 
-// May be called from any thread
+// Called from the UI Thread.

[Ted C, 2014/06/02 22:36:26]

Should we DCHECK we are on the UI thread here now?

[Mathieu, 2014/06/03 15:01:55]

Done.

 void MostVisitedSites::GetURLThumbnail(JNIEnv* env,
                                        jobject obj,
                                        jstring url,
@@ -187,11 +226,21 @@ void MostVisitedSites::GetURLThumbnail(JNIEnv* env,
 
   std::string url_string = ConvertJavaStringToUTF8(env, url);
   scoped_refptr<TopSites> top_sites(profile_->GetTopSites());
+
+  // If the Suggestions service is enabled, create a callback to fetch a
+  // server thumbnail from it, in case the local thumbnail is not found.
+  SuggestionsService* suggestions_service =
+      SuggestionsServiceFactory::GetForProfile(profile_);
+  base::Closure lookup_failed_callback = suggestions_service ?
+      base::Bind(&GetSuggestionsThumbnailOnUIThread,

[Ted C, 2014/06/02 22:36:26]

Out of curiosity, would it be better to batch all failed thumbnails into a
single request?  Just wondering if we should try to avoid 6 nearly concurrent
requests to the same server.

[Mathieu, 2014/06/03 15:01:55]

My next CL is going to be implementing local caching of server thumbnails
(hopefully transparent to this file). We will be able to choose whether to make
GetPageThumbnail only return a thumbnail if it's cached locally.

As well we can implement batching, but that will be in SuggestionsService. Lots
of improvements to be made for sure. 

+                 suggestions_service, url_string,
+                 base::Owned(new ScopedJavaGlobalRef<jobject>(*j_callback))) :
+      base::Closure();
   BrowserThread::PostTask(
-      BrowserThread::DB, FROM_HERE, base::Bind(
-          &GetUrlThumbnailTask,
-          url_string,
-          top_sites, base::Owned(j_callback)));
+      BrowserThread::DB, FROM_HERE,
+          base::Bind(
+              &GetUrlThumbnailTask, url_string, top_sites,
+              base::Owned(j_callback), lookup_failed_callback));
 }
 
 void MostVisitedSites::BlacklistUrl(JNIEnv* env,