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

chrome/browser/android/most_visited_sites.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/most_visited_sites.h"
 
 #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"
@@ skipping 80 matching lines @@
   Java_ThumbnailCallback_onMostVisitedURLsThumbnailAvailable(
       env, j_callback->obj(), bitmap->obj());
 }
 
 void AddForcedURLOnUIThread(scoped_refptr<history::TopSites> top_sites,
                             const GURL& url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   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());
+}
+
+void GetSuggestionsThumbnailOnUIThread(
+  const GURL& gurl,

[Ted C, 2014/05/28 00:21:59]

+2 for the param lines

[Mathieu, 2014/05/28 02:48:17]

Done.

+  SuggestionsService* suggestions_service,
+  ScopedJavaGlobalRef<jobject>* j_callback) {
+  suggestions_service->GetPageThumbnail(gurl,
+      base::Bind(&OnSuggestionsThumbnailAvailable,
+                 base::Owned(new ScopedJavaGlobalRef<jobject>(*j_callback))));
+}
+
 void GetUrlThumbnailTask(
     std::string url_string,
     scoped_refptr<TopSites> top_sites,
+    Profile* profile,
     ScopedJavaGlobalRef<jobject>* j_callback) {
   JNIEnv* env = AttachCurrentThread();
 
   ScopedJavaGlobalRef<jobject>* j_bitmap_ref =
       new ScopedJavaGlobalRef<jobject>();
 
   GURL gurl(url_string);
 
   scoped_refptr<base::RefCountedMemory> data;
   if (top_sites->GetPageThumbnail(gurl, false, &data)) {
     SkBitmap thumbnail_bitmap = ExtractThumbnail(*data.get());
     if (!thumbnail_bitmap.empty()) {
       j_bitmap_ref->Reset(
           env,
           gfx::ConvertToJavaBitmap(&thumbnail_bitmap).obj());
     }
   } else {
     // A thumbnail is not locally available for |gurl|. Make sure it is put in
     // the list to be fetched at the next visit to this site.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(AddForcedURLOnUIThread, top_sites, gurl));
+
+    SuggestionsService* service =
+        SuggestionsServiceFactory::GetForProfile(profile);

[Ted C, 2014/05/28 00:21:59]

BrowserContextKeyedBaseFactory is NonThreadSafe, so I don't think you can do
this here.

Maybe this is always being called on the UI thread, so this doesn't matter, but
then we are doing a bunch of UI thread work below unnecessarily and we should
assert we are on the UI thread to prevent this breaking in the future.

[Mathieu, 2014/05/28 02:48:17]

Ah, thanks. Passing the profile pointer instead should be good then? Have a
look.

[Ted C, 2014/05/28 17:45:38]

I don't think profile is thread safe either.  And GetForProfile shouldn't be
called on anything put the UI thread either (ironically it like
SuggestionsServiceFactory might have overriden just enough methods that the base
ThreadChecker calls are all being skipped an no DCHECKs are being hit).

top_sites is passed a ref pointer and is thread safe, but I don't think you can
make the same assumptions with profile.

I think you need to get back to the MostVisitedSites object on the UI thread to
do what you want instead of passing these values.

+    if (service) {
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(GetSuggestionsThumbnailOnUIThread, gurl, service,
+                     base::Owned(
+                         new ScopedJavaGlobalRef<jobject>(*j_callback))));
+      return;
+    }
   }
 
   // Since j_callback is owned by this callback, when the callback falls out of
   // scope it will be deleted. We need to pass ownership to the next callback.
   ScopedJavaGlobalRef<jobject>* j_callback_pass =
       new ScopedJavaGlobalRef<jobject>(*j_callback);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(
           &OnObtainedThumbnail,
@@ skipping 48 matching lines @@
   ScopedJavaGlobalRef<jobject>* j_callback =
       new ScopedJavaGlobalRef<jobject>();
   j_callback->Reset(env, j_callback_obj);
 
   std::string url_string = ConvertJavaStringToUTF8(env, url);
   scoped_refptr<TopSites> top_sites(profile_->GetTopSites());
   BrowserThread::PostTask(
       BrowserThread::DB, FROM_HERE, base::Bind(
           &GetUrlThumbnailTask,
           url_string,
-          top_sites, base::Owned(j_callback)));
+          top_sites, profile_, base::Owned(j_callback)));
 }
 
 void MostVisitedSites::BlacklistUrl(JNIEnv* env,
                                     jobject obj,
                                     jstring j_url) {
   TopSites* top_sites = profile_->GetTopSites();
   if (!top_sites)
     return;
 
   std::string url_string = ConvertJavaStringToUTF8(env, j_url);
@@ skipping 68 matching lines @@
       j_observer->obj(),
       ToJavaArrayOfStrings(env, titles).obj(),
       ToJavaArrayOfStrings(env, urls).obj());
 }
 
 static jlong Init(JNIEnv* env, jobject obj, jobject jprofile) {
   MostVisitedSites* most_visited_sites =
       new MostVisitedSites(ProfileAndroid::FromProfileAndroid(jprofile));
   return reinterpret_cast<intptr_t>(most_visited_sites);
 }