[Android] Display favicons in password accessory sheet

This CL adds an image view to all usernames in the passwords accessory
sheet which separates username/password groups visually.
It also indicates the origin of the credentials.

The icons are loaded by origin on the native side. If icons are
requested multiple times, they will be grouped by origin and result
in only one request to the FaviconService.
Already requested icons are cached - when one of these is requested,
the callback will return synchronously instead of async.

Bug: 853748
Change-Id: Ia97c5c62710daf709d7ee19d732d2e226ac7634f
Reviewed-on: https://chromium-review.googlesource.com/1138319Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577876}

chrome/android/java/res/layout/password_accessory_sheet_suggestion.xml

@@ -5,11 +5,11 @@
 
 <TextView
     xmlns:android="http://schemas.android.com/apk/res/android"
-    android:paddingTop="8dp"
-    android:paddingBottom="8dp"
-    android:paddingStart="52dp"
-    android:paddingEnd="16dp"
+    android:id="@+id/suggestion_text"
+    android:layout_marginStart="@dimen/keyboard_accessory_suggestion_margin"
+    android:paddingEnd="@dimen/keyboard_accessory_suggestion_margin"
+    android:gravity="center_vertical|start"
     android:fillViewport="true"
-    android:layout_height="48dp"
+    android:layout_height="@dimen/keyboard_accessory_suggestion_height"
     android:textAppearance="@style/BlackTitle1"
     android:layout_width="match_parent"/>
\ No newline at end of file

chrome/android/java/res/values/dimens.xml

@@ -129,6 +129,9 @@
     <dimen name="keyboard_accessory_padding">6dp</dimen>
     <dimen name="keyboard_accessory_sheet_height">330dp</dimen>
     <dimen name="keyboard_accessory_text_size">14sp</dimen>
+    <dimen name="keyboard_accessory_suggestion_margin">16dp</dimen>
+    <dimen name="keyboard_accessory_suggestion_height">48dp</dimen>
+    <dimen name="keyboard_accessory_suggestion_icon_size">20dp</dimen>
 
     <!-- Password generation popup dimensions -->
     <dimen name="password_generation_divider_height">1dp</dimen>

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import android.graphics.Bitmap;
 import android.graphics.drawable.Drawable;
 import android.support.annotation.LayoutRes;
 import android.support.annotation.Nullable;
@@ -146,6 +147,19 @@ public class KeyboardAccessoryData {
         private final String mContentDescription;
         private final boolean mIsPassword;
         private final @Nullable Callback<Item> mItemSelectedCallback;
+        private final @Nullable FaviconProvider mFaviconProvider;
+
+        /**
+         * Items will call a class that implements this interface to request a favicon.
+         */
+        interface FaviconProvider {
+            /**
+             * Starts a request for a favicon. The callback can be called either asynchronously or
+             * synchronously (depending on whether the icon was cached).
+             * @param favicon The icon to be used for this Item. If null, use the default icon.
+             */
+            void fetchFavicon(Callback<Bitmap> favicon);
+        }
 
         /**
          * Creates a new Item of type {@link ItemType#LABEL}. It is not interactive.
@@ -153,8 +167,9 @@ public class KeyboardAccessoryData {
          * @param contentDescription The description of this item (i.e. used for accessibility).
          */
         public static Item createLabel(String caption, String contentDescription) {
-            return new Item(ItemType.LABEL, caption, contentDescription, false, null);
+            return new Item(ItemType.LABEL, caption, contentDescription, false, null, null);
         }
+
         /**
          * Creates a new Item of type {@link ItemType#SUGGESTION} if has a callback, otherwise, it
          * will be {@link ItemType#NON_INTERACTIVE_SUGGESTION}. It usually is part of a list of
@@ -165,20 +180,21 @@ public class KeyboardAccessoryData {
          * @param itemSelectedCallback A click on this item will invoke this callback. Optional.
          */
         public static Item createSuggestion(String caption, String contentDescription,
-                boolean isPassword, @Nullable Callback<Item> itemSelectedCallback) {
+                boolean isPassword, @Nullable Callback<Item> itemSelectedCallback,
+                @Nullable FaviconProvider faviconProvider) {
             if (itemSelectedCallback == null) {
                 return new Item(ItemType.NON_INTERACTIVE_SUGGESTION, caption, contentDescription,
-                        isPassword, null);
+                        isPassword, null, faviconProvider);
             }
             return new Item(ItemType.SUGGESTION, caption, contentDescription, isPassword,
-                    itemSelectedCallback);
+                    itemSelectedCallback, faviconProvider);
         }
 
         /**
          * Creates an Item of type {@link ItemType#DIVIDER}. Basically, it's a horizontal line.
          */
         public static Item createDivider() {
-            return new Item(ItemType.DIVIDER, null, null, false, null);
+            return new Item(ItemType.DIVIDER, null, null, false, null, null);
         }
 
         /**
@@ -190,7 +206,7 @@ public class KeyboardAccessoryData {
          */
         public static Item createOption(
                 String caption, String contentDescription, Callback<Item> callback) {
-            return new Item(ItemType.OPTION, caption, contentDescription, false, callback);
+            return new Item(ItemType.OPTION, caption, contentDescription, false, callback, null);
         }
 
         /**
@@ -200,14 +216,17 @@ public class KeyboardAccessoryData {
          * @param contentDescription The description of this item (i.e. used for accessibility).
          * @param isPassword If true, the displayed caption is transformed into stars.
          * @param itemSelectedCallback If the Item is interactive, a click on it will trigger this.
+         * @param faviconProvider
          */
         private Item(@ItemType int type, String caption, String contentDescription,
-                boolean isPassword, @Nullable Callback<Item> itemSelectedCallback) {
+                boolean isPassword, @Nullable Callback<Item> itemSelectedCallback,
+                @Nullable FaviconProvider faviconProvider) {
             mType = type;
             mCaption = caption;
             mContentDescription = contentDescription;
             mIsPassword = isPassword;
             mItemSelectedCallback = itemSelectedCallback;
+            mFaviconProvider = faviconProvider;
         }
 
         /**
@@ -249,6 +268,14 @@ public class KeyboardAccessoryData {
         public Callback<Item> getItemSelectedCallback() {
             return mItemSelectedCallback;
         }
+
+        public void fetchFavicon(Callback<Bitmap> faviconCallback) {
+            if (mFaviconProvider == null) {
+                faviconCallback.onResult(null); // Use default icon without provider.
+                return;
+            }
+            mFaviconProvider.fetchFavicon(faviconCallback);
+        }
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java

@@ -4,8 +4,10 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import android.graphics.Bitmap;
 import android.support.annotation.Nullable;
 
+import org.chromium.base.Callback;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
@@ -92,11 +94,11 @@ class PasswordAccessoryBridge {
                                         != 0 : "Controller was destroyed but the bridge wasn't!";
                                 nativeOnFillingTriggered(
                                         mNativeView, item.isPassword(), item.getCaption());
-                            });
+                            }, this::fetchFavicon);
                     continue;
                 case ItemType.NON_INTERACTIVE_SUGGESTION:
                     items[i] = Item.createSuggestion(
-                            text[i], description[i], isPassword[i] == 1, null);
+                            text[i], description[i], isPassword[i] == 1, null, this::fetchFavicon);
                     continue;
                 case ItemType.DIVIDER:
                     items[i] = Item.createDivider();
@@ -113,6 +115,13 @@ class PasswordAccessoryBridge {
         return items;
     }
 
+    public void fetchFavicon(Callback<Bitmap> faviconCallback) {
+        assert mNativeView != 0 : "Favicon was requested after the bridge was destroyed!";
+        nativeOnFaviconRequested(mNativeView, faviconCallback);
+    }
+
+    private native void nativeOnFaviconRequested(
+            long nativePasswordAccessoryViewAndroid, Callback<Bitmap> faviconCallback);
     private native void nativeOnFillingTriggered(
             long nativePasswordAccessoryViewAndroid, boolean isPassword, String textToFill);
     private native void nativeOnOptionSelected(

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetCoordinator.java

@@ -5,7 +5,9 @@
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
 import android.content.Context;
+import android.graphics.Bitmap;
 import android.graphics.drawable.Drawable;
+import android.support.annotation.Nullable;
 import android.support.v7.content.res.AppCompatResources;
 import android.support.v7.widget.RecyclerView;
 import android.view.View;
@@ -56,6 +58,11 @@ public class PasswordAccessorySheetCoordinator {
         }
     }
 
+    interface FaviconProvider {
+        @Nullable
+        Bitmap getFavicon();
+    }
+
     /**
      * Creates the passwords tab.
      * @param context The {@link Context} containing resources like icons and layouts for this tab.

chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java

@@ -4,6 +4,12 @@
 
 package org.chromium.chrome.browser.autofill.keyboard_accessory;
 
+import android.graphics.Bitmap;
+import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.Drawable;
+import android.support.annotation.Nullable;
+import android.support.v4.view.MarginLayoutParamsCompat;
+import android.support.v7.content.res.AppCompatResources;
 import android.support.v7.widget.LinearLayoutManager;
 import android.support.v7.widget.RecyclerView;
 import android.text.method.PasswordTransformationMethod;
@@ -12,6 +18,7 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.widget.TextView;
 
+import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.autofill.keyboard_accessory.KeyboardAccessoryData.Item;
 import org.chromium.chrome.browser.modelutil.RecyclerViewAdapter;
@@ -39,7 +46,7 @@ class PasswordAccessorySheetViewBinder {
                                             false));
                 case ItemType.SUGGESTION: // Intentional fallthrough.
                 case ItemType.NON_INTERACTIVE_SUGGESTION: {
-                    return new TextViewHolder(
+                    return new IconTextViewHolder(
                             LayoutInflater.from(parent.getContext())
                                     .inflate(R.layout.password_accessory_sheet_suggestion, parent,
                                             false));
@@ -79,21 +86,79 @@ class PasswordAccessorySheetViewBinder {
          * Returns the text view of this item if there is one.
          * @return Returns a {@link TextView}.
          */
-        private TextView getTextView() {
+        protected TextView getTextView() {
             return (TextView) itemView;
         }
 
         @Override
         protected void bind(Item item) {
             super.bind(item);
-            if (item.isPassword()) {
-                getTextView().setTransformationMethod(new PasswordTransformationMethod());
-            }
+            getTextView().setTransformationMethod(
+                    item.isPassword() ? new PasswordTransformationMethod() : null);
             getTextView().setText(item.getCaption());
             if (item.getItemSelectedCallback() != null) {
                 getTextView().setOnClickListener(
                         src -> item.getItemSelectedCallback().onResult(item));
+            } else {
+                getTextView().setOnClickListener(null);
+            }
+        }
+    }
+
+    /**
+     * Holds a TextView that represents a list entry.
+     */
+    static class IconTextViewHolder extends TextViewHolder {
+        private final TextView mSuggestionText;
+        private final int mMargin;
+        private final int mIconSize;
+
+        IconTextViewHolder(View itemView) {
+            super(itemView);
+            mSuggestionText = itemView.findViewById(R.id.suggestion_text);
+            mMargin = itemView.getContext().getResources().getDimensionPixelSize(
+                    R.dimen.keyboard_accessory_suggestion_margin);
+            mIconSize = itemView.getContext().getResources().getDimensionPixelSize(
+                    R.dimen.keyboard_accessory_suggestion_icon_size);
+        }
+
+        @Override
+        protected TextView getTextView() {
+            return mSuggestionText;
+        }
+
+        @Override
+        protected void bind(Item item) {
+            super.bind(item);
+            ViewGroup.MarginLayoutParams params =
+                    new ViewGroup.MarginLayoutParams(mSuggestionText.getLayoutParams());
+            MarginLayoutParamsCompat.setMarginEnd(params, mMargin);
+            if (!item.isPassword()) {
+                setIconForBitmap(null); // Set the default icon, then try to get a better one.
+                item.fetchFavicon(this::setIconForBitmap);
+                MarginLayoutParamsCompat.setMarginStart(params, mMargin);
+            } else {
+                ApiCompatibilityUtils.setCompoundDrawablesRelative(
+                        mSuggestionText, null, null, null, null);
+                MarginLayoutParamsCompat.setMarginStart(params, 2 * mMargin + mIconSize);
+            }
+            mSuggestionText.setLayoutParams(params);
+        }
+
+        private void setIconForBitmap(@Nullable Bitmap favicon) {
+            Drawable icon;
+            if (favicon == null) {
+                icon = AppCompatResources.getDrawable(
+                        itemView.getContext(), R.drawable.ic_globe_36dp);
+            } else {
+                icon = new BitmapDrawable(itemView.getContext().getResources(), favicon);
+            }
+            if (icon != null) { // AppCompatResources.getDrawable is @Nullable.
+                icon.setBounds(0, 0, mIconSize, mIconSize);
             }
+            mSuggestionText.setCompoundDrawablePadding(mMargin);
+            ApiCompatibilityUtils.setCompoundDrawablesRelative(
+                    mSuggestionText, icon, null, null, null);
         }
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java

@@ -222,10 +222,10 @@ public class PasswordAccessoryIntegrationTest {
     }
 
     private static Item createSuggestion(String caption, Callback<Item> callback) {
-        return Item.createSuggestion(caption, "Description_" + caption, false, callback);
+        return Item.createSuggestion(caption, "Description_" + caption, false, callback, null);
     }
 
     private static Item createPassword(String caption) {
-        return Item.createSuggestion(caption, "Description_" + caption, true, null);
+        return Item.createSuggestion(caption, "Description_" + caption, true, null, null);
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewTest.java

@@ -110,15 +110,13 @@ public class PasswordAccessorySheetViewTest {
         ThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> mModel.add(Item.createSuggestion(
-                                "Name Suggestion", null, false, item -> clicked.set(true))));
+                                "Name Suggestion", null, false, item -> clicked.set(true), null)));
 
         CriteriaHelper.pollUiThread(Criteria.equals(1, () -> mView.get().getChildCount()));
-        assertThat(mView.get().getChildAt(0), instanceOf(TextView.class));
 
-        TextView suggestion = (TextView) mView.get().getChildAt(0);
-        assertThat(suggestion.getText(), is("Name Suggestion"));
+        assertThat(getFirstSuggestion().getText(), is("Name Suggestion"));
 
-        ThreadUtils.runOnUiThreadBlocking(suggestion::performClick);
+        ThreadUtils.runOnUiThreadBlocking(getFirstSuggestion()::performClick);
         assertThat(clicked.get(), is(true));
     }
 
@@ -130,18 +128,21 @@ public class PasswordAccessorySheetViewTest {
 
         ThreadUtils.runOnUiThreadBlocking(
                 ()
-                        -> mModel.add(Item.createSuggestion(
-                                "Password Suggestion", null, true, item -> clicked.set(true))));
+                        -> mModel.add(Item.createSuggestion("Password Suggestion", null, true,
+                                item -> clicked.set(true), null)));
 
         CriteriaHelper.pollUiThread(Criteria.equals(1, () -> mView.get().getChildCount()));
-        assertThat(mView.get().getChildAt(0), instanceOf(TextView.class));
 
-        TextView suggestion = (TextView) mView.get().getChildAt(0);
-        assertThat(suggestion.getText(), is("Password Suggestion"));
-        assertThat(suggestion.getTransformationMethod(),
+        assertThat(getFirstSuggestion().getText(), is("Password Suggestion"));
+        assertThat(getFirstSuggestion().getTransformationMethod(),
                 instanceOf(PasswordTransformationMethod.class));
 
-        ThreadUtils.runOnUiThreadBlocking(suggestion::performClick);
+        ThreadUtils.runOnUiThreadBlocking(getFirstSuggestion()::performClick);
         assertThat(clicked.get(), is(true));
     }
+
+    private TextView getFirstSuggestion() {
+        assertThat(mView.get().getChildAt(0), instanceOf(TextView.class));
+        return (TextView) mView.get().getChildAt(0);
+    }
 }
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java

@@ -149,15 +149,15 @@ public class ManualFillingControllerTest {
         Tab firstTab = addTab(mediator, 1111, null);
         mController.registerPasswordProvider(firstTabProvider);
         firstTabProvider.notifyObservers(new Item[] {
-                Item.createSuggestion("FirstPassword", "FirstPassword", true, result -> {})});
+                Item.createSuggestion("FirstPassword", "FirstPassword", true, result -> {}, null)});
         assertThat(mediator.getPasswordAccessorySheet().getModelForTesting().get(0).getCaption(),
                 is("FirstPassword"));
 
         // Simulate creating a second tab:
         Tab secondTab = addTab(mediator, 2222, firstTab);
         mController.registerPasswordProvider(secondTabProvider);
-        secondTabProvider.notifyObservers(new Item[] {
-                Item.createSuggestion("SecondPassword", "SecondPassword", true, result -> {})});
+        secondTabProvider.notifyObservers(new Item[] {Item.createSuggestion(
+                "SecondPassword", "SecondPassword", true, result -> {}, null)});
         assertThat(mediator.getPasswordAccessorySheet().getModelForTesting().get(0).getCaption(),
                 is("SecondPassword"));
 

chrome/browser/android/password_manager/password_accessory_view_android.cc

@@ -9,6 +9,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/android/callback_android.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
@@ -16,6 +17,8 @@
 #include "jni/PasswordAccessoryBridge_jni.h"
 #include "ui/android/view_android.h"
 #include "ui/android/window_android.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "ui/gfx/image/image.h"
 
 PasswordAccessoryViewAndroid::PasswordAccessoryViewAndroid(
     PasswordAccessoryController* controller)
@@ -71,6 +74,16 @@ void PasswordAccessoryViewAndroid::OnAutomaticGenerationStatusChanged(
       env, java_object_, available /* available */);
 }
 
+void PasswordAccessoryViewAndroid::OnFaviconRequested(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj,
+    const base::android::JavaParamRef<jobject>& j_callback) {
+  controller_->GetFavicon(
+      base::BindOnce(&PasswordAccessoryViewAndroid::OnImageFetched,
+                     base::Unretained(this),  // Outlives or cancels request.
+                     base::android::ScopedJavaGlobalRef<jobject>(j_callback)));
+}
+
 void PasswordAccessoryViewAndroid::OnFillingTriggered(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
@@ -94,6 +107,16 @@ void PasswordAccessoryViewAndroid::OnGenerationRequested(
   controller_->OnGenerationRequested();
 }
 
+void PasswordAccessoryViewAndroid::OnImageFetched(
+    const base::android::ScopedJavaGlobalRef<jobject>& j_callback,
+    const gfx::Image& image) {
+  base::android::ScopedJavaLocalRef<jobject> j_bitmap;
+  if (!image.IsEmpty())
+    j_bitmap = gfx::ConvertToJavaBitmap(image.ToSkBitmap());
+
+  RunObjectCallbackAndroid(j_callback, j_bitmap);
+}
+
 // static
 std::unique_ptr<PasswordAccessoryViewInterface>
 PasswordAccessoryViewInterface::Create(

chrome/browser/android/password_manager/password_accessory_view_android.h

@@ -10,6 +10,10 @@
 #include "base/android/scoped_java_ref.h"
 #include "chrome/browser/password_manager/password_accessory_view_interface.h"
 
+namespace gfx {
+class Image;
+}
+
 class PasswordAccessoryController;
 
 // This Android-specific implementation of the |PasswordAccessoryViewInterface|
@@ -27,6 +31,10 @@ class PasswordAccessoryViewAndroid : public PasswordAccessoryViewInterface {
   void OnAutomaticGenerationStatusChanged(bool available) override;
 
   // Called from Java via JNI:
+  void OnFaviconRequested(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj,
+      const base::android::JavaParamRef<jobject>& j_callback);
   void OnFillingTriggered(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj,
@@ -40,6 +48,10 @@ class PasswordAccessoryViewAndroid : public PasswordAccessoryViewInterface {
                              const base::android::JavaParamRef<jobject>& obj);
 
  private:
+  void OnImageFetched(
+      const base::android::ScopedJavaGlobalRef<jobject>& j_callback,
+      const gfx::Image& image);
+
   // The controller provides data for this view and owns it.
   PasswordAccessoryController* controller_;
 

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -461,7 +461,7 @@ void ChromePasswordManagerClient::PasswordWasAutofilled(
       !base::FeatureList::IsEnabled(features::kExperimentalUi)) {
     return;  // No need to even create the bridge if it's not going to be used.
   }
-  // If an accessory exists already, |CreateForWebContents| is a NoOp.
+  // If an accessory exists already, |CreateForWebContents| is a NoOp
   PasswordAccessoryController::CreateForWebContents(web_contents());
   PasswordAccessoryController::FromWebContents(web_contents())
       ->SavePasswordsForOrigin(best_matches, url::Origin::Create(origin));
@@ -561,7 +561,12 @@ void ChromePasswordManagerClient::DidFinishNavigation(
   web_contents()->GetRenderViewHost()->GetWidget()->RemoveInputEventObserver(
       this);
   web_contents()->GetRenderViewHost()->GetWidget()->AddInputEventObserver(this);
-#endif
+#else   // defined(OS_ANDROID)
+  PasswordAccessoryController* accessory =
+      PasswordAccessoryController::FromWebContents(web_contents());
+  if (accessory)
+    accessory->DidNavigateMainFrame();
+#endif  // defined(OS_ANDROID)
 }
 
 #if !defined(OS_ANDROID)
@@ -1078,16 +1083,17 @@ void ChromePasswordManagerClient::FocusedInputChanged(bool is_fillable,
       !base::FeatureList::IsEnabled(features::kExperimentalUi)) {
     return;  // No need to even create the bridge if it's not going to be used.
   }
-  if (is_fillable)  // Refresh but don't create a new accessory in this case.
+  if (is_fillable) {  // If not fillable, update existing an accessory only.
     PasswordAccessoryController::CreateForWebContents(web_contents());
+  }
   PasswordAccessoryController* accessory =
       PasswordAccessoryController::FromWebContents(web_contents());
-  if (!accessory)
-    return;  // No accessory needs change here.
-  accessory->RefreshSuggestionsForField(
-      password_manager_driver_bindings_.GetCurrentTargetFrame()
-          ->GetLastCommittedOrigin(),
-      is_fillable, is_password_field);
+  if (accessory) {
+    accessory->RefreshSuggestionsForField(
+        password_manager_driver_bindings_.GetCurrentTargetFrame()
+            ->GetLastCommittedOrigin(),
+        is_fillable, is_password_field);
+  }
 #endif  // defined(OS_ANDROID)
 }
 

chrome/browser/password_manager/password_accessory_controller.cc

@@ -6,14 +6,18 @@
 
 #include <vector>
 
+#include "base/callback.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/android/preferences/preferences_launcher.h"
+#include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/password_manager/password_generation_dialog_view_interface.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/content/browser/content_autofill_driver_factory.h"
 #include "components/autofill/core/common/password_form.h"
+#include "components/favicon/core/favicon_service.h"
 #include "components/password_manager/content/browser/content_password_manager_driver.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
@@ -65,22 +69,35 @@ struct PasswordAccessoryController::SuggestionElementData {
   Item::Type username_type;
 };
 
+struct PasswordAccessoryController::FaviconRequestData {
+  // List of requests waiting for favicons to be available.
+  std::vector<base::OnceCallback<void(const gfx::Image&)>> pending_requests;
+
+  // Cached image for this origin. |IsEmpty()| unless a favicon was found.
+  gfx::Image cached_icon;
+};
+
 PasswordAccessoryController::PasswordAccessoryController(
     content::WebContents* web_contents)
     : web_contents_(web_contents),
       view_(PasswordAccessoryViewInterface::Create(this)),
       create_dialog_factory_(
           base::BindRepeating(&PasswordGenerationDialogViewInterface::Create)),
+      favicon_service_(FaviconServiceFactory::GetForProfile(
+          Profile::FromBrowserContext(web_contents->GetBrowserContext()),
+          ServiceAccessType::EXPLICIT_ACCESS)),
       weak_factory_(this) {}
 
 // Additional creation functions in unit tests only:
 PasswordAccessoryController::PasswordAccessoryController(
     content::WebContents* web_contents,
     std::unique_ptr<PasswordAccessoryViewInterface> view,
-    CreateDialogFactory create_dialog_factory)
+    CreateDialogFactory create_dialog_factory,
+    favicon::FaviconService* favicon_service)
     : web_contents_(web_contents),
       view_(std::move(view)),
       create_dialog_factory_(create_dialog_factory),
+      favicon_service_(favicon_service),
       weak_factory_(this) {}
 
 PasswordAccessoryController::~PasswordAccessoryController() = default;
@@ -89,13 +106,14 @@ PasswordAccessoryController::~PasswordAccessoryController() = default;
 void PasswordAccessoryController::CreateForWebContentsForTesting(
     content::WebContents* web_contents,
     std::unique_ptr<PasswordAccessoryViewInterface> view,
-    CreateDialogFactory create_dialog_factory) {
+    CreateDialogFactory create_dialog_factory,
+    favicon::FaviconService* favicon_service) {
   DCHECK(web_contents) << "Need valid WebContents to attach controller to!";
   DCHECK(!FromWebContents(web_contents)) << "Controller already attached!";
   web_contents->SetUserData(
-      UserDataKey(),
-      base::WrapUnique(new PasswordAccessoryController(
-          web_contents, std::move(view), create_dialog_factory)));
+      UserDataKey(), base::WrapUnique(new PasswordAccessoryController(
+                         web_contents, std::move(view), create_dialog_factory,
+                         favicon_service)));
 }
 
 void PasswordAccessoryController::SavePasswordsForOrigin(
@@ -135,6 +153,67 @@ void PasswordAccessoryController::OnAutomaticGenerationStatusChanged(
   view_->OnAutomaticGenerationStatusChanged(available);
 }
 
+void PasswordAccessoryController::OnFilledIntoFocusedField(
+    autofill::FillingStatus status) {
+  // TODO(crbug/853766): Record success rate.
+  // TODO(fhorschig): Update UI by hiding the sheet or communicating the error.
+}
+
+void PasswordAccessoryController::RefreshSuggestionsForField(
+    const url::Origin& origin,
+    bool is_fillable,
+    bool is_password_field) {
+  // TODO(crbug/853766): Record CTR metric.
+  if (is_fillable) {
+    current_origin_ = origin;
+    view_->OnItemsAvailable(CreateViewItems(origin, origin_suggestions_[origin],
+                                            is_password_field));
+  } else {
+    // For unfillable fields, reset the origin and send the empty state message.
+    current_origin_ = url::Origin();
+    view_->OnItemsAvailable(CreateViewItems(
+        origin, std::vector<SuggestionElementData>(), is_password_field));
+  }
+}
+
+void PasswordAccessoryController::DidNavigateMainFrame() {
+  if (current_origin_.IsSameOriginWith(
+          web_contents_->GetMainFrame()->GetLastCommittedOrigin()))
+    return;  // Clean requests only if the navigation was across origins.
+  favicon_tracker_.TryCancelAll();  // If there is a request pending, cancel it.
+  current_origin_ = url::Origin();
+  icons_request_data_.clear();
+  origin_suggestions_.clear();
+}
+
+void PasswordAccessoryController::GetFavicon(
+    base::OnceCallback<void(const gfx::Image&)> icon_callback) {
+  url::Origin origin = current_origin_;  // Copy origin in case it changes.
+  // Check whether this request can be immediately answered with a cached icon.
+  // It is empty if there wasn't at least one request that found an icon yet.
+  FaviconRequestData* icon_request = &icons_request_data_[origin];
+  if (!icon_request->cached_icon.IsEmpty()) {
+    std::move(icon_callback).Run(icon_request->cached_icon);
+    return;
+  }
+  if (!favicon_service_) {  // This might happen in tests.
+    std::move(icon_callback).Run(gfx::Image());
+    return;
+  }
+
+  // The cache is empty. Queue the callback.
+  icon_request->pending_requests.emplace_back(std::move(icon_callback));
+  if (icon_request->pending_requests.size() > 1)
+    return;  // The favicon for this origin was already requested.
+
+  favicon_service_->GetFaviconImageForPageURL(
+      origin.GetURL(),
+      base::BindRepeating(  // FaviconService doesn't support BindOnce yet.
+          &PasswordAccessoryController::OnImageFetched,
+          weak_factory_.GetWeakPtr(), origin),
+      &favicon_tracker_);
+}
+
 void PasswordAccessoryController::OnFillingTriggered(
     bool is_password,
     const base::string16& textToFill) {
@@ -200,24 +279,6 @@ void PasswordAccessoryController::OnSavedPasswordsLinkClicked() {
   chrome::android::PreferencesLauncher::ShowPasswordSettings();
 }
 
-void PasswordAccessoryController::OnFilledIntoFocusedField(
-    autofill::FillingStatus status) {
-  // TODO(crbug/853766): Record success rate.
-  // TODO(fhorschig): Update UI by hiding the sheet or communicating the error.
-}
-
-void PasswordAccessoryController::RefreshSuggestionsForField(
-    const url::Origin& origin,
-    bool is_fillable,
-    bool is_password_field) {
-  // TODO(crbug/853766): Record CTR metric.
-  view_->OnItemsAvailable(
-      CreateViewItems(origin,
-                      is_fillable ? origin_suggestions_[origin]
-                                  : std::vector<SuggestionElementData>(),
-                      is_password_field));
-}
-
 gfx::NativeView PasswordAccessoryController::container_view() const {
   return web_contents_->GetNativeView();
 }
@@ -268,3 +329,17 @@ std::vector<Item> PasswordAccessoryController::CreateViewItems(
                      Item::Type::OPTION);
   return items;
 }
+
+void PasswordAccessoryController::OnImageFetched(
+    url::Origin origin,
+    const favicon_base::FaviconImageResult& image_result) {
+  FaviconRequestData* icon_request = &icons_request_data_[origin];
+  icon_request->cached_icon = image_result.image;
+  // Only trigger all the callbacks if they still affect a displayed origin.
+  if (origin == current_origin_) {
+    for (auto& callback : icon_request->pending_requests) {
+      std::move(callback).Run(icon_request->cached_icon);
+    }
+  }
+  icon_request->pending_requests.clear();
+}

chrome/browser/password_manager/password_accessory_controller.h

@@ -10,13 +10,16 @@
 #include <utility>
 #include <vector>
 
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "base/task/cancelable_task_tracker.h"
 #include "chrome/browser/password_manager/password_accessory_view_interface.h"
 #include "components/autofill/core/common/filling_status.h"
 #include "components/autofill/core/common/password_generation_util.h"
+#include "components/favicon_base/favicon_types.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "ui/gfx/native_widget_types.h"
 #include "url/gurl.h"
@@ -25,6 +28,10 @@ namespace autofill {
 struct PasswordForm;
 }  // namespace autofill
 
+namespace favicon {
+class FaviconService;
+}
+
 namespace password_manager {
 class PasswordManagerDriver;
 }  // namespace password_manager
@@ -52,6 +59,10 @@ class PasswordAccessoryController
       PasswordGenerationDialogViewInterface>(PasswordAccessoryController*)>;
   ~PasswordAccessoryController() override;
 
+  // -----------------------------
+  // Methods called by the client:
+  // -----------------------------
+
   // Saves credentials for an origin so that they can be used in the sheet.
   void SavePasswordsForOrigin(
       const std::map<base::string16, const autofill::PasswordForm*>&
@@ -65,6 +76,30 @@ class PasswordAccessoryController
           autofill::password_generation::PasswordGenerationUIData>& ui_data,
       const base::WeakPtr<password_manager::PasswordManagerDriver>& driver);
 
+  // Completes a filling attempt by recording metrics, giving feedback to the
+  // user and dismissing the accessory sheet.
+  void OnFilledIntoFocusedField(autofill::FillingStatus status);
+
+  // Makes sure, that all shown suggestions are appropriate for the currently
+  // focused field and for fields that lost the focus. If a field lost focus,
+  // |is_fillable| will be false.
+  void RefreshSuggestionsForField(const url::Origin& origin,
+                                  bool is_fillable,
+                                  bool is_password_field);
+
+  // Reacts to a navigation on the main frame, e.g. by clearing caches.
+  void DidNavigateMainFrame();
+
+  // --------------------------
+  // Methods called by UI code:
+  // --------------------------
+
+  // Uses the give |favicon_service| to get an icon for the currently focused
+  // frame. The given callback is called with an image unless an icon for a new
+  // origin was called. In the latter case, the callback is dropped.
+  // The callback is called with an |IsEmpty()| image if there is no favicon.
+  void GetFavicon(base::OnceCallback<void(const gfx::Image&)> icon_callback);
+
   // Called by the UI code to request that |textToFill| is to be filled into the
   // currently focused field.
   void OnFillingTriggered(bool is_password, const base::string16& textToFill);
@@ -86,16 +121,9 @@ class PasswordAccessoryController
   // in the explanation text that leads to the saved passwords.
   void OnSavedPasswordsLinkClicked();
 
-  // Compeletes a filling attempt by recording metrics, giving feedback to the
-  // user and dismissing the accessory sheet.
-  void OnFilledIntoFocusedField(autofill::FillingStatus status);
-
-  // Makes sure, that all shown suggestions are appropriate for the currently
-  // focused field and for fields that lost the focus. If a field lost focus,
-  // |is_fillable| will be false.
-  void RefreshSuggestionsForField(const url::Origin& origin,
-                                  bool is_fillable,
-                                  bool is_password_field);
+  // -----------------
+  // Member accessors:
+  // -----------------
 
   // The web page view containing the focused field.
   gfx::NativeView container_view() const;
@@ -107,7 +135,8 @@ class PasswordAccessoryController
   static void CreateForWebContentsForTesting(
       content::WebContents* web_contents,
       std::unique_ptr<PasswordAccessoryViewInterface> test_view,
-      CreateDialogFactory create_dialog_callback);
+      CreateDialogFactory create_dialog_callback,
+      favicon::FaviconService* favicon_service);
 
 #if defined(UNIT_TEST)
   // Returns the held view for testing.
@@ -122,6 +151,9 @@ class PasswordAccessoryController
   // Data for a credential pair that is transformed into a suggestion.
   struct SuggestionElementData;
 
+  // Data allowing to cache favicons and favicon-related requests.
+  struct FaviconRequestData;
+
   // Required for construction via |CreateForWebContents|:
   explicit PasswordAccessoryController(content::WebContents* contents);
   friend class content::WebContentsUserData<PasswordAccessoryController>;
@@ -130,7 +162,8 @@ class PasswordAccessoryController
   PasswordAccessoryController(
       content::WebContents* web_contents,
       std::unique_ptr<PasswordAccessoryViewInterface> view,
-      CreateDialogFactory create_dialog_callback);
+      CreateDialogFactory create_dialog_callback,
+      favicon::FaviconService* favicon_service);
 
   // Creates the view items based on the given |suggestions|.
   // If |is_password_field| is false, password suggestions won't be interactive.
@@ -139,6 +172,11 @@ class PasswordAccessoryController
                   const std::vector<SuggestionElementData>& suggestions,
                   bool is_password_field);
 
+  // Handles a favicon response requested by |GetFavicon| and calls the waiting
+  // last_icon_callback_ with a (possibly empty) icon bitmap.
+  void OnImageFetched(url::Origin origin,
+                      const favicon_base::FaviconImageResult& image_result);
+
   // Contains the last set of credentials by origin.
   std::map<url::Origin, std::vector<SuggestionElementData>> origin_suggestions_;
 
@@ -148,6 +186,19 @@ class PasswordAccessoryController
   // Data for the generation element used to generate the password.
   std::unique_ptr<GenerationElementData> generation_element_data_;
 
+  // The origin of the currently focused frame. It's used to ensure that
+  // favicons are not displayed across origins.
+  url::Origin current_origin_;
+
+  // TODO(fhorschig): Find a way to use unordered_map with origin keys.
+  // A cache for all favicons that were requested. This includes all iframes
+  // for which the accessory was displayed.
+  std::map<url::Origin, FaviconRequestData> icons_request_data_;
+
+  // Used to track a requested favicon. If the set of suggestion changes, this
+  // object aborts the request. Upon destruction, requests are cancelled, too.
+  base::CancelableTaskTracker favicon_tracker_;
+
   // Password manager driver for the target frame used for password generation.
   base::WeakPtr<password_manager::PasswordManagerDriver> target_frame_driver_;
 
@@ -166,6 +217,9 @@ class PasswordAccessoryController
   // Creation callback for the modal dialog view meant to facilitate testing.
   CreateDialogFactory create_dialog_factory_;
 
+  // The favicon service used to make retrieve icons for a given origin.
+  favicon::FaviconService* favicon_service_;
+
   base::WeakPtrFactory<PasswordAccessoryController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryController);

chrome/test/BUILD.gn

@@ -2922,6 +2922,7 @@ test("unit_tests") {
       "//base:base_java",
       "//chrome/android:app_hooks_java",
       "//chrome/android:chrome_java",
+      "//components/favicon/core/test:test_support",
       "//components/gcm_driver/instance_id/android:instance_id_driver_java",
       "//components/gcm_driver/instance_id/android:instance_id_driver_test_support_java",
       "//content/public/android:content_java",