Migrate LargeIconBridge to GURL

After some trial and error, I think the best strategy for migrating our
java code to GURL is to convert all of the leaf nodes (typically jni
boundaries) to GURL, providing a deprecated interface for existing
usage, then convert the rest.

Bug: 783819
Change-Id: I8c00386e70008a588e0d5e249e3c398112c94e99
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2044759Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743402}

chrome/android/features/keyboard_accessory/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/sheet_tabs/PasswordAccessorySheetViewBinder.java

@@ -228,7 +228,7 @@ class PasswordAccessorySheetViewBinder {
         void fetchFavicon(String origin, Callback<Drawable> setIconCallback) {
             final LargeIconBridge mIconBridge =
                     new LargeIconBridge(Profile.getLastUsedRegularProfile());
-            mIconBridge.getLargeIconForUrl(origin, mDesiredSize,
+            mIconBridge.getLargeIconForStringUrl(origin, mDesiredSize,
                     (icon, fallbackColor, isFallbackColorDefault, iconType) -> {
                         Drawable drawable = FaviconUtils.getIconDrawableWithoutFilter(icon, origin,
                                 fallbackColor, mIconGenerator, mResources, mDesiredSize);

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkItemRow.java

@@ -63,7 +63,7 @@ public class BookmarkItemRow extends BookmarkRow implements LargeIconCallback {
         mStartIconView.setImageDrawable(null);
         mTitleView.setText(item.getTitle());
         mDescriptionView.setText(item.getUrlForDisplay());
-        mDelegate.getLargeIconBridge().getLargeIconForUrl(mUrl, mMinIconSize, this);
+        mDelegate.getLargeIconBridge().getLargeIconForStringUrl(mUrl, mMinIconSize, this);
         return item;
     }
 

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManager.java

@@ -36,6 +36,7 @@ import org.chromium.components.browser_ui.widget.dragreorder.DragStateDelegate;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectableListLayout;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectableListToolbar.SearchDelegate;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectionDelegate;
+import org.chromium.url.GURL;
 
 import java.util.Stack;
 
@@ -282,7 +283,7 @@ public class BookmarkManager
 
     @Override
     public void onUpdateFavicon(String url) {
-        mLargeIconBridge.clearFavicon(url);
+        mLargeIconBridge.clearFavicon(new GURL(url));
         mFaviconsNeedRefresh = true;
     }
 

chrome/android/java/src/org/chromium/chrome/browser/bookmarkswidget/BookmarkWidgetService.java

@@ -254,7 +254,7 @@ public class BookmarkWidgetService extends RemoteViewsService {
                     taskFinished();
                 }
             };
-            mLargeIconBridge.getLargeIconForUrl(bookmark.url, mMinIconSizeDp, callback);
+            mLargeIconBridge.getLargeIconForStringUrl(bookmark.url, mMinIconSizeDp, callback);
         }
 
         @UiThread

chrome/android/java/src/org/chromium/chrome/browser/contextmenu/RevampedContextMenuHeaderMediator.java

@@ -49,7 +49,7 @@ class RevampedContextMenuHeaderMediator implements View.OnClickListener {
 
         if (!params.isImage() && !params.isVideo()) {
             LargeIconBridge iconBridge = new LargeIconBridge(Profile.getLastUsedProfile());
-            iconBridge.getLargeIconForUrl(mPlainUrl,
+            iconBridge.getLargeIconForStringUrl(mPlainUrl,
                     context.getResources().getDimensionPixelSize(R.dimen.default_favicon_min_size),
                     this::onFaviconAvailable);
         } else if (params.isVideo()) {

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

@@ -14,6 +14,7 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.util.ConversionUtils;
+import org.chromium.url.GURL;
 
 /**
  * A Java API for using the C++ LargeIconService.
@@ -24,7 +25,7 @@ public class LargeIconBridge {
     private static final int CACHE_ENTRY_MIN_SIZE_BYTES = ConversionUtils.BYTES_PER_KILOBYTE;
     private final Profile mProfile;
     private long mNativeLargeIconBridge;
-    private LruCache<String, CachedFavicon> mFaviconCache;
+    private LruCache<GURL, CachedFavicon> mFaviconCache;
 
     private static class CachedFavicon {
         public Bitmap icon;
@@ -70,8 +71,8 @@ public class LargeIconBridge {
 
     /**
      * Constructor that leaves the bridge independent from the native side.
-     * Note: {@link #getLargeIconForUrl(String, int, LargeIconCallback)} will crash with the default
-     * implementation, it should then be overridden.
+     * Note: {@link #getLargeIconForStringUrl(String, int, LargeIconCallback)} will crash with the
+     * default implementation, it should then be overridden.
      */
     @VisibleForTesting
     public LargeIconBridge() {
@@ -88,9 +89,9 @@ public class LargeIconBridge {
     public void createCache(int cacheSizeBytes) {
         assert cacheSizeBytes > 0;
 
-        mFaviconCache = new LruCache<String, CachedFavicon>(cacheSizeBytes) {
+        mFaviconCache = new LruCache<GURL, CachedFavicon>(cacheSizeBytes) {
             @Override
-            protected int sizeOf(String key, CachedFavicon favicon) {
+            protected int sizeOf(GURL key, CachedFavicon favicon) {
                 int iconBitmapSize = favicon.icon == null ? 0 : favicon.icon.getByteCount();
                 return Math.max(CACHE_ENTRY_MIN_SIZE_BYTES, iconBitmapSize);
             }
@@ -107,6 +108,17 @@ public class LargeIconBridge {
         }
     }
 
+    /**
+     * See {@link #getLargeIconForUrl(GURL, int, LargeIconCallback)}
+     *
+     * @deprecated please use getLargeIconForUrl instead.
+     */
+    @Deprecated
+    public boolean getLargeIconForStringUrl(
+            final String pageUrl, int desiredSizePx, final LargeIconCallback callback) {
+        return getLargeIconForUrl(new GURL(pageUrl), desiredSizePx, callback);
+    }
+
     /**
      * Given a URL, returns a large icon for that URL if one is available (e.g. a favicon or
      * touch icon). If none is available, a fallback color is returned, based on the dominant color
@@ -120,8 +132,8 @@ public class LargeIconBridge {
      *                 will not be called if this method returns false.
      * @return True if a callback should be expected.
      */
-    public boolean getLargeIconForUrl(final String pageUrl, int desiredSizePx,
-            final LargeIconCallback callback) {
+    public boolean getLargeIconForUrl(
+            final GURL pageUrl, int desiredSizePx, final LargeIconCallback callback) {
         assert mNativeLargeIconBridge != 0;
         assert callback != null;
 
@@ -155,7 +167,7 @@ public class LargeIconBridge {
     /**
      * Removes the favicon from the local cache for the given URL.
      */
-    public void clearFavicon(String url) {
+    public void clearFavicon(GURL url) {
         mFaviconCache.remove(url);
     }
 
@@ -163,7 +175,7 @@ public class LargeIconBridge {
     interface Natives {
         long init();
         void destroy(long nativeLargeIconBridge);
-        boolean getLargeIconForURL(long nativeLargeIconBridge, Profile profile, String pageUrl,
+        boolean getLargeIconForURL(long nativeLargeIconBridge, Profile profile, GURL pageUrl,
                 int desiredSizePx, LargeIconCallback callback);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/history/HistoryItemView.java

@@ -179,7 +179,7 @@ public class HistoryItemView extends SelectableItemView<HistoryItem> implements
     private void requestIcon() {
         if (mHistoryManager == null || mHistoryManager.getLargeIconBridge() == null) return;
 
-        mHistoryManager.getLargeIconBridge().getLargeIconForUrl(
+        mHistoryManager.getLargeIconBridge().getLargeIconForStringUrl(
                 getItem().getUrl(), mMinIconSize, this);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaSessionTabHelper.java

@@ -580,7 +580,7 @@ public class MediaSessionTabHelper implements MediaImageCallback {
             }
         };
 
-        return mLargeIconBridge.getLargeIconForUrl(pageUrl, size, callback);
+        return mLargeIconBridge.getLargeIconForStringUrl(pageUrl, size, callback);
     }
 
     private boolean isNotificationHiddingOrHidden() {

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/BasicSuggestionProcessor.java

@@ -217,7 +217,7 @@ public class BasicSuggestionProcessor extends BaseSuggestionViewProcessor {
         final LargeIconBridge iconBridge = mIconBridgeSupplier.get();
         if (iconBridge == null) return;
 
-        iconBridge.getLargeIconForUrl(url, mDesiredFaviconWidthPx,
+        iconBridge.getLargeIconForStringUrl(url, mDesiredFaviconWidthPx,
                 (Bitmap icon, int fallbackColor, boolean isFallbackColorDefault, int iconType) -> {
                     if (icon == null) return;
 

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/editurl/EditUrlSuggestionProcessor.java

@@ -189,7 +189,7 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
         final LargeIconBridge iconBridge = mIconBridgeSupplier.get();
         if (iconBridge == null) return;
 
-        iconBridge.getLargeIconForUrl(url, mDesiredFaviconWidthPx,
+        iconBridge.getLargeIconForStringUrl(url, mDesiredFaviconWidthPx,
                 (Bitmap icon, int fallbackColor, boolean isFallbackColorDefault,
                         int iconType) -> model.set(EditUrlSuggestionProperties.SITE_FAVICON, icon));
     }

chrome/android/java/src/org/chromium/chrome/browser/settings/privacy/ConfirmImportantSitesDialogFragment.java

@@ -122,7 +122,7 @@ public class ConfirmImportantSitesDialogFragment extends DialogFragment {
                     viewHolder.imageView.setImageDrawable(image);
                 }
             };
-            mLargeIconBridge.getLargeIconForUrl(url, mFaviconSize, viewHolder.imageCallback);
+            mLargeIconBridge.getLargeIconForStringUrl(url, mFaviconSize, viewHolder.imageCallback);
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/suggestions/ImageFetcher.java

@@ -99,7 +99,7 @@ public class ImageFetcher {
             String url, int size, LargeIconBridge.LargeIconCallback callback) {
         assert !mIsDestroyed;
 
-        getLargeIconBridge().getLargeIconForUrl(url, size, callback);
+        getLargeIconBridge().getLargeIconForStringUrl(url, size, callback);
     }
 
     public void onDestroy() {

chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageLoadTest.java

@@ -93,7 +93,7 @@ public class NewTabPageLoadTest {
 
     private static class AsyncMockLargeIconBridge extends LargeIconBridge {
         @Override
-        public boolean getLargeIconForUrl(String pageUrl, int desiredSizePx,
+        public boolean getLargeIconForStringUrl(String pageUrl, int desiredSizePx,
                 final LargeIconBridge.LargeIconCallback callback) {
             new Handler().postDelayed(new Runnable() {
                 @Override

chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NtpUiCaptureTestData.java

@@ -192,7 +192,7 @@ public class NtpUiCaptureTestData {
         }
         return new LargeIconBridge() {
             @Override
-            public boolean getLargeIconForUrl(
+            public boolean getLargeIconForStringUrl(
                     String url, int desiredSizePx, LargeIconCallback callback) {
                 PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> {
                     int fallbackColor =

chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationFaviconTest.java

@@ -57,7 +57,7 @@ public class MediaNotificationFaviconTest extends MediaNotificationManagerTestBa
         }
 
         @Override
-        public boolean getLargeIconForUrl(
+        public boolean getLargeIconForStringUrl(
                 final String pageUrl, int desiredSizePx, final LargeIconCallback callback) {
             mGetIconCalledAtLeastOnce = true;
             mCallback = callback;

chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationTestTabHolder.java

@@ -49,7 +49,7 @@ public class MediaNotificationTestTabHolder {
     // Mock LargeIconBridge that always returns false.
     private class TestLargeIconBridge extends LargeIconBridge {
         @Override
-        public boolean getLargeIconForUrl(
+        public boolean getLargeIconForStringUrl(
                 final String pageUrl, int desiredSizePx, final LargeIconCallback callback) {
             return false;
         }

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/suggestions/basic/BasicSuggestionProcessorTest.java

@@ -299,7 +299,7 @@ public class BasicSuggestionProcessorTest {
         SuggestionDrawableState icon1 = mModel.get(BaseSuggestionViewProperties.ICON);
         Assert.assertNotNull(icon1);
 
-        verify(mIconBridge).getLargeIconForUrl(eq(url), anyInt(), callback.capture());
+        verify(mIconBridge).getLargeIconForStringUrl(eq(url), anyInt(), callback.capture());
         callback.getValue().onLargeIconAvailable(mFakeBitmap, 0, false, 0);
         SuggestionDrawableState icon2 = mModel.get(BaseSuggestionViewProperties.ICON);
         Assert.assertNotNull(icon2);
@@ -318,7 +318,7 @@ public class BasicSuggestionProcessorTest {
         SuggestionDrawableState icon1 = mModel.get(BaseSuggestionViewProperties.ICON);
         Assert.assertNotNull(icon1);
 
-        verify(mIconBridge).getLargeIconForUrl(eq(url), anyInt(), callback.capture());
+        verify(mIconBridge).getLargeIconForStringUrl(eq(url), anyInt(), callback.capture());
         callback.getValue().onLargeIconAvailable(null, 0, false, 0);
         SuggestionDrawableState icon2 = mModel.get(BaseSuggestionViewProperties.ICON);
         Assert.assertNotNull(icon2);

chrome/android/junit/src/org/chromium/chrome/browser/suggestions/SuggestionsImageFetcherTest.java

@@ -122,7 +122,7 @@ public class SuggestionsImageFetcherTest {
         imageFetcher.makeLargeIconRequest(URL_STRING, IMAGE_SIZE_PX, mock(LargeIconCallback.class));
 
         verify(mLargeIconBridge)
-                .getLargeIconForUrl(
+                .getLargeIconForStringUrl(
                         eq(URL_STRING), eq(IMAGE_SIZE_PX), any(LargeIconCallback.class));
     }
 }

chrome/browser/android/large_icon_bridge.cc

@@ -7,10 +7,8 @@
 #include <jni.h>
 
 #include "base/android/jni_android.h"
-#include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
-#include "base/strings/utf_string_conversions.h"
 #include "chrome/android/chrome_jni_headers/LargeIconBridge_jni.h"
 #include "chrome/browser/favicon/large_icon_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -21,9 +19,10 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "url/android/gurl_android.h"
+#include "url/gurl.h"
 
 using base::android::AttachCurrentThread;
-using base::android::ConvertJavaStringToUTF16;
 using base::android::JavaParamRef;
 using base::android::JavaRef;
 using base::android::ScopedJavaGlobalRef;
@@ -72,7 +71,7 @@ void LargeIconBridge::Destroy(JNIEnv* env) {
 jboolean LargeIconBridge::GetLargeIconForURL(
     JNIEnv* env,
     const JavaParamRef<jobject>& j_profile,
-    const JavaParamRef<jstring>& j_page_url,
+    const JavaParamRef<jobject>& j_page_url,
     jint min_source_size_px,
     const JavaParamRef<jobject>& j_callback) {
   Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
@@ -87,10 +86,12 @@ jboolean LargeIconBridge::GetLargeIconForURL(
   favicon_base::LargeIconCallback callback_runner = base::BindOnce(
       &OnLargeIconAvailable, ScopedJavaGlobalRef<jobject>(env, j_callback));
 
+  std::unique_ptr<GURL> url = url::GURLAndroid::ToNativeGURL(env, j_page_url);
+
   // Use desired_size = 0 for getting the icon from the cache (so that
   // the icon is not poorly rescaled by LargeIconService).
   large_icon_service->GetLargeIconRawBitmapOrFallbackStyleForPageUrl(
-      GURL(ConvertJavaStringToUTF16(env, j_page_url)), min_source_size_px,
+      *url, min_source_size_px,
       /*desired_size_in_pixel=*/0, std::move(callback_runner),
       &cancelable_task_tracker_);
 

chrome/browser/android/large_icon_bridge.h

@@ -22,7 +22,7 @@ class LargeIconBridge {
   jboolean GetLargeIconForURL(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& j_profile,
-      const base::android::JavaParamRef<jstring>& j_page_url,
+      const base::android::JavaParamRef<jobject>& j_page_url,
       jint min_source_size_px,
       const base::android::JavaParamRef<jobject>& j_callback);
 

chrome/browser/touch_to_fill/android/internal/java/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillMediator.java

@@ -110,12 +110,12 @@ class TouchToFillMediator {
         };
         final LargeIconCallback setIconOrRetry = (icon, fallbackColor, hasDefaultColor, type) -> {
             if (icon == null && iconOrigin.equals(credential.getOriginUrl())) {
-                mLargeIconBridge.getLargeIconForUrl(url, mDesiredIconSize, setIcon);
+                mLargeIconBridge.getLargeIconForStringUrl(url, mDesiredIconSize, setIcon);
                 return; // Unlikely but retry for exact path if there is no icon for the origin.
             }
             setIcon.onLargeIconAvailable(icon, fallbackColor, hasDefaultColor, type);
         };
-        mLargeIconBridge.getLargeIconForUrl(iconOrigin, mDesiredIconSize, setIconOrRetry);
+        mLargeIconBridge.getLargeIconForStringUrl(iconOrigin, mDesiredIconSize, setIconOrRetry);
     }
 
     private String getIconOrigin(String credentialOrigin, String siteUrl) {

chrome/browser/touch_to_fill/android/junit/src/org/chromium/chrome/browser/touch_to_fill/TouchToFillControllerTest.java

@@ -163,11 +163,11 @@ public class TouchToFillControllerTest {
         assertThat(itemList.get(3).model.get(FAVICON_OR_FALLBACK), is(nullValue()));
 
         verify(mMockIconBridge)
-                .getLargeIconForUrl(eq(CARL.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
+                .getLargeIconForStringUrl(eq(CARL.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
         verify(mMockIconBridge)
-                .getLargeIconForUrl(eq(ANA.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
+                .getLargeIconForStringUrl(eq(ANA.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
         verify(mMockIconBridge)
-                .getLargeIconForUrl(eq(BOB.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
+                .getLargeIconForStringUrl(eq(BOB.getOriginUrl()), eq(DESIRED_FAVICON_SIZE), any());
     }
 
     @Test
@@ -181,7 +181,7 @@ public class TouchToFillControllerTest {
 
         // ANA and CARL both have TEST_URL as their origin URL
         verify(mMockIconBridge)
-                .getLargeIconForUrl(
+                .getLargeIconForStringUrl(
                         eq(TEST_URL), eq(DESIRED_FAVICON_SIZE), mCallbackArgumentCaptor.capture());
         LargeIconBridge.LargeIconCallback callback = mCallbackArgumentCaptor.getValue();
         Bitmap bitmap = Bitmap.createBitmap(