[PwdCheckAndroid] Add favicons to credential rows

This CL adds favicons to each credential row or a fallback if no icon is
available for a site.

Screenshots in the bug.

Bug: 1106724, 1092444
Change-Id: I246f128fc76838142f599b75666912a1187bf9bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362895Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803069}

chrome/browser/password_check/android/internal/BUILD.gn

@@ -63,6 +63,7 @@ android_library("internal_java") {
     "//chrome/browser/password_manager/android:java",
     "//chrome/browser/profiles/android:java",
     "//chrome/browser/settings:java",
+    "//chrome/browser/ui/android/favicon:java",
     "//chrome/browser/ui/android/strings:ui_strings_grd",
     "//components/browser_ui/widget/android:java",
     "//components/embedder_support/android:util_java",
@@ -91,6 +92,7 @@ android_library("internal_java") {
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewDialogFragment.java",
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewHolder.java",
     "java/src/org/chromium/chrome/browser/password_check/helper/PasswordCheckChangePasswordHelper.java",
+    "java/src/org/chromium/chrome/browser/password_check/helper/PasswordCheckIconHelper.java",
     "java/src/org/chromium/chrome/browser/password_check/helper/PasswordCheckReauthenticationHelper.java",
   ]
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]

chrome/browser/password_check/android/internal/java/res/layout/password_check_compromised_credential_item.xml

@@ -3,13 +3,25 @@
      Use of this source code is governed by a BSD-style license that can be
      found in the LICENSE file. -->
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
     android:fillViewport="true"
     android:gravity="center"
     android:layout_height="wrap_content"
     android:layout_width="match_parent"
     android:orientation="horizontal">
 
-    <!-- TODO(crbug.com/1106277): Remove the paddingStart if favicons fill that space. -->
+    <ImageView
+        android:id="@+id/credential_favicon"
+        android:background="@drawable/list_item_icon_modern_bg"
+        android:layout_gravity="top"
+        android:layout_marginEnd="@dimen/compromised_credential_row_icon_margin_start_end"
+        android:layout_marginStart="@dimen/compromised_credential_row_icon_margin_start_end"
+        android:layout_marginTop="@dimen/compromised_credential_row_icon_margin_top"
+        android:layout_height="@dimen/compromised_credential_row_icon_size"
+        android:layout_width="@dimen/compromised_credential_row_icon_size"
+        android:scaleType="center"
+        tools:ignore="ContentDescription" />
+
     <LinearLayout
         android:gravity="start"
         android:layout_gravity="top"
@@ -17,7 +29,6 @@
         android:layout_width="0dp"
         android:layout_weight="1"
         android:orientation="vertical"
-        android:paddingStart="@dimen/compromised_credential_row_padding_start"
         android:paddingBottom="@dimen/compromised_credential_row_padding_bottom"
         android:paddingTop="@dimen/compromised_credential_row_padding_top">
 

chrome/browser/password_check/android/internal/java/res/layout/password_check_compromised_credential_with_script_item.xml

@@ -3,13 +3,25 @@
      Use of this source code is governed by a BSD-style license that can be
      found in the LICENSE file. -->
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
     android:fillViewport="true"
     android:gravity="center"
     android:layout_height="wrap_content"
     android:layout_width="match_parent"
     android:orientation="horizontal">
 
-    <!-- TODO(crbug.com/1106277): Remove the paddingStart if favicons fill that space. -->
+    <ImageView
+        android:id="@+id/credential_favicon"
+        android:background="@drawable/list_item_icon_modern_bg"
+        android:layout_gravity="top"
+        android:layout_marginEnd="@dimen/compromised_credential_row_icon_margin_start_end"
+        android:layout_marginStart="@dimen/compromised_credential_row_icon_margin_start_end"
+        android:layout_marginTop="@dimen/compromised_credential_row_icon_margin_top"
+        android:layout_height="@dimen/compromised_credential_row_icon_size"
+        android:layout_width="@dimen/compromised_credential_row_icon_size"
+        android:scaleType="center"
+        tools:ignore="ContentDescription" />
+
     <LinearLayout
         android:gravity="start"
         android:layout_gravity="top"
@@ -17,7 +29,6 @@
         android:layout_width="0dp"
         android:layout_weight="1"
         android:orientation="vertical"
-        android:paddingStart="@dimen/compromised_credential_row_padding_start"
         android:paddingBottom="@dimen/compromised_credential_row_padding_bottom"
         android:paddingTop="@dimen/compromised_credential_row_padding_top">
 

chrome/browser/password_check/android/internal/java/res/values/dimens.xml

@@ -22,11 +22,13 @@
   <dimen name="compromised_credential_row_button_icon_start_padding">12dp</dimen>
   <dimen name="compromised_credential_row_button_icon_end_padding">16dp</dimen>
   <dimen name="compromised_credential_row_button_margin_top">16dp</dimen>
+  <dimen name="compromised_credential_row_icon_margin_start_end">16dp</dimen>
+  <dimen name="compromised_credential_row_icon_margin_top">20dp</dimen>
+  <dimen name="compromised_credential_row_icon_size">36dp</dimen>
   <dimen name="compromised_credential_row_more_padding_end">8dp</dimen>
   <dimen name="compromised_credential_row_more_padding_start">16dp</dimen>
   <dimen name="compromised_credential_row_more_size">48dp</dimen>
   <dimen name="compromised_credential_row_padding_bottom">16dp</dimen>
-  <dimen name="compromised_credential_row_padding_start">16dp</dimen>
   <dimen name="compromised_credential_row_padding_top">12dp</dimen>
   <dimen name="compromised_credential_row_reason_margin_top">2dp</dimen>
 

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckCoordinator.java

@@ -11,8 +11,10 @@ import androidx.lifecycle.LifecycleObserver;
 
 import org.chromium.chrome.browser.help.HelpAndFeedback;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckChangePasswordHelper;
+import org.chromium.chrome.browser.password_check.helper.PasswordCheckIconHelper;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckReauthenticationHelper;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.ui.favicon.LargeIconBridge;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 
@@ -76,7 +78,12 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
 
         mMediator = new PasswordCheckMediator(
                 new PasswordCheckChangePasswordHelper(mFragmentView.getActivity()),
-                mReauthenticationHelper);
+                mReauthenticationHelper,
+                new PasswordCheckIconHelper(
+                        new LargeIconBridge(Profile.getLastUsedRegularProfile()),
+                        mFragmentView.getResources().getDimensionPixelSize(
+                                org.chromium.chrome.browser.ui.favicon.R.dimen
+                                        .default_favicon_size)));
     }
 
     private void launchCheckupInAccount() {

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckMediator.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.password_check;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.FAVICON_OR_FALLBACK;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
@@ -26,6 +27,7 @@ import android.util.Pair;
 import androidx.appcompat.app.AlertDialog;
 
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckChangePasswordHelper;
+import org.chromium.chrome.browser.password_check.helper.PasswordCheckIconHelper;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckReauthenticationHelper;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckReauthenticationHelper.ReauthReason;
 import org.chromium.ui.modelutil.ListModel;
@@ -49,11 +51,14 @@ class PasswordCheckMediator
     private PasswordCheckComponentUi.Delegate mDelegate;
     private Runnable mLaunchCheckupInAccount;
     private HashSet<CompromisedCredential> mPreCheckSet;
+    private final PasswordCheckIconHelper mIconHelper;
 
     PasswordCheckMediator(PasswordCheckChangePasswordHelper changePasswordDelegate,
-            PasswordCheckReauthenticationHelper reauthenticationHelper) {
+            PasswordCheckReauthenticationHelper reauthenticationHelper,
+            PasswordCheckIconHelper passwordCheckIconHelper) {
         mChangePasswordDelegate = changePasswordDelegate;
         mReauthenticationHelper = reauthenticationHelper;
+        mIconHelper = passwordCheckIconHelper;
     }
 
     void initialize(PropertyModel model, PasswordCheckComponentUi.Delegate delegate,
@@ -257,16 +262,21 @@ class PasswordCheckMediator
     }
 
     private ListItem createEntryForCredential(CompromisedCredential credential) {
-        return new ListItem(credential.hasScript()
-                        ? PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT
-                        : PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
+        PropertyModel credentialModel =
                 new PropertyModel
                         .Builder(PasswordCheckProperties.CompromisedCredentialProperties.ALL_KEYS)
                         .with(COMPROMISED_CREDENTIAL, credential)
                         .with(HAS_MANUAL_CHANGE_BUTTON,
                                 mChangePasswordDelegate.canManuallyChangeCredential(credential))
                         .with(CREDENTIAL_HANDLER, this)
-                        .build());
+                        .build();
+        mIconHelper.getLargeIcon(credential, (faviconOrFallback) -> {
+            credentialModel.set(FAVICON_OR_FALLBACK, faviconOrFallback);
+        });
+        return new ListItem(credential.hasScript()
+                        ? PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL_WITH_SCRIPT
+                        : PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
+                credentialModel);
     }
 
     private void sortCredentials(List<CompromisedCredential> credentials) {

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckProperties.java

@@ -8,6 +8,7 @@ import android.util.Pair;
 
 import androidx.annotation.IntDef;
 
+import org.chromium.chrome.browser.password_check.helper.PasswordCheckIconHelper.FaviconOrFallback;
 import org.chromium.ui.modelutil.ListModel;
 import org.chromium.ui.modelutil.MVCListAdapter;
 import org.chromium.ui.modelutil.PropertyKey;
@@ -52,9 +53,12 @@ class PasswordCheckProperties {
                 new PropertyModel.ReadableObjectPropertyKey<>("credential_handler");
         static final PropertyModel.ReadableBooleanPropertyKey HAS_MANUAL_CHANGE_BUTTON =
                 new PropertyModel.ReadableBooleanPropertyKey("has_change_button");
+        static final PropertyModel
+                .WritableObjectPropertyKey<FaviconOrFallback> FAVICON_OR_FALLBACK =
+                new PropertyModel.WritableObjectPropertyKey<>("favicon");
 
-        static final PropertyKey[] ALL_KEYS = {
-                COMPROMISED_CREDENTIAL, CREDENTIAL_HANDLER, HAS_MANUAL_CHANGE_BUTTON};
+        static final PropertyKey[] ALL_KEYS = {COMPROMISED_CREDENTIAL, CREDENTIAL_HANDLER,
+                HAS_MANUAL_CHANGE_BUTTON, FAVICON_OR_FALLBACK};
 
         private CompromisedCredentialProperties() {}
     }

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewBinder.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.password_check;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.FAVICON_OR_FALLBACK;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.HAS_MANUAL_CHANGE_BUTTON;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
@@ -39,7 +40,9 @@ import androidx.appcompat.content.res.AppCompatResources;
 import androidx.core.graphics.drawable.DrawableCompat;
 
 import org.chromium.chrome.browser.password_check.PasswordCheckProperties.ItemType;
+import org.chromium.chrome.browser.password_check.helper.PasswordCheckIconHelper;
 import org.chromium.chrome.browser.password_check.internal.R;
+import org.chromium.chrome.browser.ui.favicon.FaviconUtils;
 import org.chromium.components.browser_ui.widget.listmenu.BasicListMenu;
 import org.chromium.components.browser_ui.widget.listmenu.ListMenu;
 import org.chromium.components.browser_ui.widget.listmenu.ListMenuButton;
@@ -178,6 +181,15 @@ class PasswordCheckViewBinder {
             changeHint.setVisibility(model.get(HAS_MANUAL_CHANGE_BUTTON) || credential.hasScript()
                             ? View.GONE
                             : View.VISIBLE);
+        } else if (propertyKey == FAVICON_OR_FALLBACK) {
+            ImageView imageView = view.findViewById(R.id.credential_favicon);
+            PasswordCheckIconHelper.FaviconOrFallback data = model.get(FAVICON_OR_FALLBACK);
+            imageView.setImageDrawable(FaviconUtils.getIconDrawableWithoutFilter(data.mIcon,
+                    data.mUrl, PasswordCheckIconHelper.getIconColor(data, view.getResources()),
+                    FaviconUtils.createCircularIconGenerator(view.getResources()),
+                    view.getResources(),
+                    view.getResources().getDimensionPixelSize(
+                            org.chromium.chrome.browser.ui.favicon.R.dimen.default_favicon_size)));
         } else {
             assert false : "Unhandled update to property:" + propertyKey;
         }

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/helper/PasswordCheckIconHelper.java

+// Copyright 2020 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.
+
+package org.chromium.chrome.browser.password_check.helper;
+
+import android.content.res.Resources;
+import android.graphics.Bitmap;
+
+import androidx.annotation.Nullable;
+import androidx.core.util.Pair;
+
+import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.Callback;
+import org.chromium.chrome.browser.password_check.CompromisedCredential;
+import org.chromium.chrome.browser.ui.favicon.IconType;
+import org.chromium.chrome.browser.ui.favicon.LargeIconBridge;
+import org.chromium.chrome.browser.ui.favicon.R;
+import org.chromium.url.GURL;
+
+/**
+ * Helper used to fetch or create an appropriate icon for a compromised credential.
+ */
+public class PasswordCheckIconHelper {
+    /**
+     * Data object containing all data required to set an icon or construct a fallback.
+     */
+    public static class FaviconOrFallback {
+        public final String mUrl;
+        public final @Nullable Bitmap mIcon;
+        public final int mFallbackColor;
+        public final boolean mIsFallbackColorDefault;
+        public final int mIconType;
+        public final int mIconSize;
+
+        FaviconOrFallback(String originUrl, @Nullable Bitmap icon, int fallbackColor,
+                boolean isFallbackColorDefault, int iconType, int iconSize) {
+            mUrl = originUrl;
+            mIcon = icon;
+            mFallbackColor = fallbackColor;
+            mIsFallbackColorDefault = isFallbackColorDefault;
+            mIconType = iconType;
+            mIconSize = iconSize;
+        }
+    }
+
+    private final LargeIconBridge mLargeIconBridge;
+    private final int mDesiredIconSize;
+
+    public PasswordCheckIconHelper(LargeIconBridge largeIconBridge, int desiredIconSize) {
+        mLargeIconBridge = largeIconBridge;
+        mDesiredIconSize = desiredIconSize;
+    }
+
+    public void getLargeIcon(
+            CompromisedCredential credential, Callback<FaviconOrFallback> iconCallback) {
+        final Pair<GURL, String> originAndFallback = getIconOriginAndFallback(credential);
+        if (!originAndFallback.first.isValid()) {
+            iconCallback.onResult(new FaviconOrFallback(
+                    originAndFallback.second, null, 0, true, IconType.INVALID, mDesiredIconSize));
+            return; // Abort because an invalid icon URLs will crash Chrome!
+        }
+        mLargeIconBridge.getLargeIconForUrl(originAndFallback.first, mDesiredIconSize,
+                (icon, fallbackColor, hasDefaultColor, type) -> {
+                    iconCallback.onResult(new FaviconOrFallback(originAndFallback.second, icon,
+                            fallbackColor, hasDefaultColor, type, mDesiredIconSize));
+                });
+    }
+
+    /**
+     * @param faviconOrFallbackData A {@link FaviconOrFallback} containing color or fallback hints.
+     * @param resources A {@link Resources} object used to load default colors if necessary.
+     * @return A color (non-resource value) to use for monogram icons.
+     */
+    public static int getIconColor(
+            PasswordCheckIconHelper.FaviconOrFallback faviconOrFallbackData, Resources resources) {
+        return faviconOrFallbackData.mIsFallbackColorDefault ? ApiCompatibilityUtils.getColor(
+                       resources, R.color.default_favicon_background_color)
+                                                             : faviconOrFallbackData.mFallbackColor;
+    }
+
+    /**
+     * Determines which origin to use for retrieving a favicon.
+     * @param credential A {@link CompromisedCredential}.
+     * @return A pair with (potentially invalid) icon origin and a fallback URL for monograms.
+     */
+    private static Pair<GURL, String> getIconOriginAndFallback(CompromisedCredential credential) {
+        // Ideally, the sign-on realm is valid and has an adjacent, valid icon.
+        GURL iconOrigin = new GURL(credential.getSignonRealm());
+        String fallbackUrl = credential.getSignonRealm();
+        if (!iconOrigin.isValid()) {
+            // If the sign-on realm isn't valid, try the change URL instead.
+            iconOrigin = new GURL(credential.getPasswordChangeUrl());
+            fallbackUrl = credential.getPasswordChangeUrl();
+        }
+        if (!iconOrigin.isValid()) {
+            // If neither URL is valid, try the display origin as a last, very weak hope.
+            iconOrigin = credential.getOrigin();
+            fallbackUrl = credential.getDisplayOrigin();
+        }
+        return new Pair<>(iconOrigin, fallbackUrl);
+    }
+}
\ No newline at end of file

chrome/browser/password_check/android/javatests/src/org/chromium/chrome/browser/password_check/PasswordCheckViewTest.java

@@ -175,6 +175,10 @@ public class PasswordCheckViewTest {
         assertThat(getCredentialMoreButtonAt(1).getVisibility(), is(View.VISIBLE));
         assertThat(getCredentialMoreButtonAt(1).getContentDescription(),
                 is(getString(org.chromium.chrome.R.string.more)));
+
+        // Has a favicon.
+        assertNotNull(getCredentialFaviconAt(1));
+        assertThat(getCredentialFaviconAt(1).getVisibility(), is(View.VISIBLE));
     }
 
     @Test
@@ -809,6 +813,10 @@ public class PasswordCheckViewTest {
                 R.id.credential_menu_button);
     }
 
+    private ImageView getCredentialFaviconAt(int index) {
+        return getPasswordCheckViewList().getChildAt(index).findViewById(R.id.credential_favicon);
+    }
+
     private String getString(@IdRes int stringResource) {
         return mTestRule.getActivity().getString(stringResource);
     }

chrome/browser/password_check/android/junit/src/org/chromium/chrome/browser/password_check/PasswordCheckControllerTest.java

@@ -56,6 +56,7 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.password_check.PasswordCheckProperties.ItemType;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckChangePasswordHelper;
+import org.chromium.chrome.browser.password_check.helper.PasswordCheckIconHelper;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckReauthenticationHelper;
 import org.chromium.chrome.browser.password_check.helper.PasswordCheckReauthenticationHelper.ReauthReason;
 import org.chromium.chrome.test.util.browser.Features;
@@ -92,6 +93,8 @@ public class PasswordCheckControllerTest {
     private PasswordCheck mPasswordCheck;
     @Mock
     private PasswordCheckReauthenticationHelper mReauthenticationHelper;
+    @Mock
+    private PasswordCheckIconHelper mIconHelper;
 
     // DO NOT INITIALIZE HERE! The objects would be shared here which leaks state between tests.
     private PasswordCheckMediator mMediator;
@@ -101,7 +104,8 @@ public class PasswordCheckControllerTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
         mModel = PasswordCheckProperties.createDefaultModel();
-        mMediator = new PasswordCheckMediator(mChangePasswordDelegate, mReauthenticationHelper);
+        mMediator = new PasswordCheckMediator(
+                mChangePasswordDelegate, mReauthenticationHelper, mIconHelper);
         PasswordCheckFactory.setPasswordCheckForTesting(mPasswordCheck);
         mMediator.initialize(mModel, mDelegate, PasswordCheckReferrer.PASSWORD_SETTINGS, () -> {});
     }
@@ -208,6 +212,7 @@ public class PasswordCheckControllerTest {
         assertThat(mModel.get(ITEMS).get(1).model.get(COMPROMISED_CREDENTIAL), equalTo(ANA));
         assertThat(mModel.get(ITEMS).get(1).model.get(CREDENTIAL_HANDLER), is(mMediator));
         assertThat(mModel.get(ITEMS).get(1).model.get(HAS_MANUAL_CHANGE_BUTTON), is(true));
+        verify(mIconHelper).getLargeIcon(eq(ANA), any(Callback.class));
     }
 
     @Test