Integrate android to FaviconRequestHandler

Bug: 955475
Change-Id: I8a393c33dabe8a974acee4cf27acdc7f8a08c4d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1634695Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#665846}

chrome/android/java/src/org/chromium/chrome/browser/favicon/FaviconHelper.java

@@ -170,16 +170,19 @@ public class FaviconHelper {
     }
 
     /**
-     * Get 16x16 Favicon bitmap for the requested arguments. Only retrives favicons in synced
-     * session storage. (e.g. favicons synced from other devices).
-     * TODO(apiccion): provide a way to obtain higher resolution favicons.
+     * Get foreign Favicon bitmap for the requested arguments.
      * @param profile Profile used for the FaviconService construction.
      * @param pageUrl The target Page URL to get the favicon.
-     * @return 16x16 favicon Bitmap corresponding to the pageUrl.
+     * @param desiredSizeInPixel The size of the favicon in pixel we want to get.
+     * @param faviconImageCallback A method to be called back when the result is available. Note
+     *         that this callback is not called if this method returns false.
+     * @return favicon Bitmap corresponding to the pageUrl.
      */
-    public Bitmap getSyncedFaviconImageForURL(Profile profile, String pageUrl) {
+    public boolean getForeignFaviconImageForURL(Profile profile, String pageUrl,
+            int desiredSizeInPixel, FaviconImageCallback faviconImageCallback) {
         assert mNativeFaviconHelper != 0;
-        return nativeGetSyncedFaviconImageForURL(mNativeFaviconHelper, profile, pageUrl);
+        return nativeGetForeignFaviconImageForURL(
+                mNativeFaviconHelper, profile, pageUrl, desiredSizeInPixel, faviconImageCallback);
     }
 
     // TODO(jkrcal): Remove these two methods when FaviconHelper is not used any more by
@@ -215,8 +218,9 @@ public class FaviconHelper {
     private static native boolean nativeGetLocalFaviconImageForURL(long nativeFaviconHelper,
             Profile profile, String pageUrl, int desiredSizeInDip,
             FaviconImageCallback faviconImageCallback);
-    private static native Bitmap nativeGetSyncedFaviconImageForURL(long nativeFaviconHelper,
-            Profile profile, String pageUrl);
+    private static native boolean nativeGetForeignFaviconImageForURL(long nativeFaviconHelper,
+            Profile profile, String pageUrl, int desiredSizeInDip,
+            FaviconImageCallback faviconImageCallback);
     private static native void nativeEnsureIconIsAvailable(long nativeFaviconHelper,
             Profile profile, WebContents webContents, String pageUrl, String iconUrl,
             boolean isLargeIcon, IconAvailabilityCallback callback);

chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsManager.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.ntp;
 
 import android.content.Context;
-import android.graphics.Bitmap;
 import android.support.annotation.IntDef;
 
 import org.chromium.base.VisibleForTesting;
@@ -248,13 +247,16 @@ public class RecentTabsManager implements AndroidSyncSettingsObserver, SignInSta
     }
 
     /**
-     * Returns a 16x16 favicon for a given synced url.
+     * Returns a favicon for a given foreign url.
      *
      * @param url The url to fetch the favicon for.
-     * @return 16x16 favicon or null if favicon unavailable.
+     * @param size the desired favicon size.
+     * @param faviconCallback the callback to be invoked when the favicon is available.
+     * @return favicon or null if favicon unavailable.
      */
-    public Bitmap getSyncedFaviconImageForURL(String url) {
-        return mFaviconHelper.getSyncedFaviconImageForURL(mProfile, url);
+    public boolean getForeignFaviconForUrl(
+            String url, int size, FaviconImageCallback faviconCallback) {
+        return mFaviconHelper.getForeignFaviconImageForURL(mProfile, url, size, faviconCallback);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsRowAdapter.java

@@ -297,7 +297,7 @@ public class RecentTabsRowAdapter extends BaseExpandableListAdapter {
                 viewHolder.domainView.setText("");
                 viewHolder.domainView.setVisibility(View.GONE);
             }
-            loadSyncedFavicon(viewHolder, sessionTab.url);
+            loadForeignFavicon(viewHolder, sessionTab.url);
         }
 
         @Override
@@ -638,7 +638,7 @@ public class RecentTabsRowAdapter extends BaseExpandableListAdapter {
     }
 
     private static class FaviconCache {
-        private static final String SYNCED_FAVICON_PREFIX = "Synced";
+        private static final String FOREIGN_FAVICON_PREFIX = "Foreign";
         private static final String LOCAL_FAVICON_PREFIX = "Local";
 
         private final LruCache<String, Drawable> mMemoryCache;
@@ -647,12 +647,12 @@ public class RecentTabsRowAdapter extends BaseExpandableListAdapter {
             mMemoryCache = new LruCache<String, Drawable>(size);
         }
 
-        public Drawable getSyncedFaviconImage(String url) {
-            return mMemoryCache.get(SYNCED_FAVICON_PREFIX + url);
+        public Drawable getForeignFaviconImage(String url) {
+            return mMemoryCache.get(FOREIGN_FAVICON_PREFIX + url);
         }
 
-        public void putSyncedFaviconImage(String url, Drawable image) {
-            mMemoryCache.put(SYNCED_FAVICON_PREFIX + url, image);
+        public void putForeignFaviconImage(String url, Drawable image) {
+            mMemoryCache.put(FOREIGN_FAVICON_PREFIX + url, image);
         }
 
         public Drawable getLocalFaviconImage(String url) {
@@ -718,14 +718,30 @@ public class RecentTabsRowAdapter extends BaseExpandableListAdapter {
                 ViewUtils.DEFAULT_FAVICON_CORNER_RADIUS);
     }
 
-    private void loadSyncedFavicon(final ViewHolder viewHolder, final String url) {
-        Drawable image = mFaviconCache.getSyncedFaviconImage(url);
-        if (image == null) {
-            image = faviconDrawable(mRecentTabsManager.getSyncedFaviconImageForURL(url), url);
-            image = (image == null)
-                    ? mDefaultFaviconHelper.getDefaultFaviconDrawable(mActivity, url, true)
-                    : image;
-            mFaviconCache.putSyncedFaviconImage(url, image);
+    private void loadForeignFavicon(final ViewHolder viewHolder, final String url) {
+        Drawable image;
+        if (url == null) {
+            // URL is null for print jobs, for example.
+            image = mDefaultFaviconHelper.getDefaultFaviconDrawable(mActivity, url, true);
+        } else {
+            image = mFaviconCache.getLocalFaviconImage(url);
+            if (image == null) {
+                FaviconImageCallback imageCallback = new FaviconImageCallback() {
+                    @Override
+                    public void onFaviconAvailable(Bitmap bitmap, String iconUrl) {
+                        if (this != viewHolder.imageCallback) return;
+                        Drawable image = faviconDrawable(bitmap, url);
+                        image = image == null ? mDefaultFaviconHelper.getDefaultFaviconDrawable(
+                                        mActivity, url, true)
+                                              : image;
+                        mFaviconCache.putLocalFaviconImage(url, image);
+                        viewHolder.imageView.setImageDrawable(image);
+                    }
+                };
+                viewHolder.imageCallback = imageCallback;
+                mRecentTabsManager.getForeignFaviconForUrl(url, mFaviconSize, imageCallback);
+                image = mDefaultFaviconHelper.getDefaultFaviconDrawable(mActivity, url, true);
+            }
         }
         viewHolder.imageView.setImageDrawable(image);
     }

chrome/browser/android/favicon_helper.cc

@@ -17,12 +17,16 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
+#include "chrome/browser/favicon/large_icon_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/session_sync_service_factory.h"
+#include "components/favicon/core/favicon_request_handler.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon/core/favicon_util.h"
 #include "components/favicon_base/favicon_util.h"
+#include "components/sync/driver/sync_service_utils.h"
 #include "components/sync_sessions/open_tabs_ui_delegate.h"
 #include "components/sync_sessions/session_sync_service.h"
 #include "content/public/browser/web_contents.h"
@@ -45,7 +49,7 @@ using base::android::ConvertUTF8ToJavaString;
 
 namespace {
 
-void OnLocalFaviconAvailable(
+void OnFaviconBitmapResultAvailable(
     const JavaRef<jobject>& j_favicon_image_callback,
     const favicon_base::FaviconRawBitmapResult& result) {
   JNIEnv* env = AttachCurrentThread();
@@ -63,7 +67,7 @@ void OnLocalFaviconAvailable(
       j_favicon_bitmap = gfx::ConvertToJavaBitmap(&favicon_bitmap);
   }
 
-  // Call java side OnLocalFaviconAvailable method.
+  // Call java side OnFaviconBitmapResultAvailable method.
   Java_FaviconImageCallback_onFaviconAvailable(env, j_favicon_image_callback,
                                                j_favicon_bitmap, j_icon_url);
 }
@@ -76,6 +80,30 @@ void OnEnsureIconIsAvailableFinished(
       env, j_availability_callback, newly_available);
 }
 
+sync_sessions::OpenTabsUIDelegate* GetOpenTabsUIDelegate(Profile* profile) {
+  sync_sessions::SessionSyncService* session_sync_service =
+      SessionSyncServiceFactory::GetInstance()->GetForProfile(profile);
+  DCHECK(session_sync_service);
+  return session_sync_service->GetOpenTabsUIDelegate();
+}
+
+scoped_refptr<base::RefCountedMemory> GetSyncedFaviconForPageURL(
+    Profile* profile,
+    const GURL& page_url) {
+  sync_sessions::OpenTabsUIDelegate* open_tabs = GetOpenTabsUIDelegate(profile);
+  return open_tabs ? open_tabs->GetSyncedFaviconForPageURL(page_url.spec())
+                   : nullptr;
+}
+
+// Check if user settings allow querying a Google server using history
+// information.
+bool CanSendHistoryDataToServer(Profile* profile) {
+  return syncer::GetUploadToGoogleState(
+             ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile),
+             syncer::ModelType::HISTORY_DELETE_DIRECTIVES) ==
+         syncer::UploadState::ACTIVE;
+}
+
 }  // namespace
 
 static jlong JNI_FaviconHelper_Init(JNIEnv* env) {
@@ -110,7 +138,7 @@ jboolean FaviconHelper::GetLocalFaviconImageForURL(
     return false;
 
   favicon_base::FaviconRawBitmapCallback callback_runner =
-      base::BindOnce(&OnLocalFaviconAvailable,
+      base::BindOnce(&OnFaviconBitmapResultAvailable,
                      ScopedJavaGlobalRef<jobject>(j_favicon_image_callback));
 
   // |j_page_url| is an origin, and it may not have had a favicon associated
@@ -132,38 +160,35 @@ jboolean FaviconHelper::GetLocalFaviconImageForURL(
   return true;
 }
 
-ScopedJavaLocalRef<jobject> FaviconHelper::GetSyncedFaviconImageForURL(
+jboolean FaviconHelper::GetForeignFaviconImageForURL(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jobject>& jprofile,
-    const JavaParamRef<jstring>& j_page_url) {
+    const JavaParamRef<jstring>& j_page_url,
+    jint j_desired_size_in_pixel,
+    const base::android::JavaParamRef<jobject>& j_favicon_image_callback) {
   Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile);
-  DCHECK(profile);
-
-  std::string page_url = ConvertJavaStringToUTF8(env, j_page_url);
-
-  sync_sessions::SessionSyncService* session_sync_service =
-      SessionSyncServiceFactory::GetInstance()->GetForProfile(profile);
-  DCHECK(session_sync_service);
-
-  sync_sessions::OpenTabsUIDelegate* open_tabs =
-      session_sync_service->GetOpenTabsUIDelegate();
-  DCHECK(open_tabs);
-
-  scoped_refptr<base::RefCountedMemory> favicon_png =
-      open_tabs->GetSyncedFaviconForPageURL(page_url);
-  if (!favicon_png)
-    return ScopedJavaLocalRef<jobject>();
-
-  // Convert favicon_image_result to java objects.
-  gfx::Image favicon_image = gfx::Image::CreateFrom1xPNGBytes(favicon_png);
-  SkBitmap favicon_bitmap = favicon_image.AsBitmap();
+  if (!profile)
+    return false;
 
-  ScopedJavaLocalRef<jobject> j_favicon_bitmap;
-  if (favicon_bitmap.isNull())
-    return ScopedJavaLocalRef<jobject>();
+  GURL page_url(ConvertJavaStringToUTF8(env, j_page_url));
 
-  return gfx::ConvertToJavaBitmap(&favicon_bitmap);
+  sync_sessions::OpenTabsUIDelegate* open_tabs = GetOpenTabsUIDelegate(profile);
+  // TODO(victorvianna): Consider passing icon types to the API.
+  favicon_request_handler_.GetRawFaviconForPageURL(
+      page_url, static_cast<int>(j_desired_size_in_pixel),
+      base::BindOnce(&OnFaviconBitmapResultAvailable,
+                     ScopedJavaGlobalRef<jobject>(j_favicon_image_callback)),
+      favicon::FaviconRequestOrigin::RECENTLY_CLOSED_TABS,
+      favicon::FaviconRequestPlatform::kMobile,
+      FaviconServiceFactory::GetForProfile(profile,
+                                           ServiceAccessType::EXPLICIT_ACCESS),
+      LargeIconServiceFactory::GetForBrowserContext(profile),
+      /*icon_url_for_uma=*/
+      open_tabs ? open_tabs->GetIconUrlForPageUrl(page_url) : GURL(),
+      base::BindOnce(&GetSyncedFaviconForPageURL, profile),
+      CanSendHistoryDataToServer(profile), cancelable_task_tracker_.get());
+  return true;
 }
 
 void FaviconHelper::EnsureIconIsAvailable(

chrome/browser/android/favicon_helper.h

@@ -13,6 +13,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "components/favicon/core/favicon_request_handler.h"
 #include "components/favicon_base/favicon_types.h"
 #include "url/gurl.h"
 
@@ -33,11 +34,13 @@ class FaviconHelper {
       const base::android::JavaParamRef<jstring>& j_page_url,
       jint j_desired_size_in_pixel,
       const base::android::JavaParamRef<jobject>& j_favicon_image_callback);
-  base::android::ScopedJavaLocalRef<jobject> GetSyncedFaviconImageForURL(
+  jboolean GetForeignFaviconImageForURL(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,
       const base::android::JavaParamRef<jobject>& jprofile,
-      const base::android::JavaParamRef<jstring>& j_page_url);
+      const base::android::JavaParamRef<jstring>& j_page_url,
+      jint j_desired_size_in_pixel,
+      const base::android::JavaParamRef<jobject>& j_favicon_image_callback);
 
   void EnsureIconIsAvailable(
       JNIEnv* env,
@@ -81,6 +84,8 @@ class FaviconHelper {
 
   static size_t GetLargestSizeIndex(const std::vector<gfx::Size>& sizes);
 
+  favicon::FaviconRequestHandler favicon_request_handler_;
+
   std::unique_ptr<base::CancelableTaskTracker> cancelable_task_tracker_;
 
   virtual ~FaviconHelper();