Commit 8a47b09d authored by Reda Tawfik's avatar Reda Tawfik Committed by Commit Bot

[Android][Mfill] Add a list of credentials to the Model

1) Adds list all user credentials to the Model from the Mediator.
2) Adds the credential model to properties class.
3) Creates a ViewHolder for the credential view.
4) Implements the Adapter for the RecyclerView.
4) Implements |bindCredentialView()|.
5) Sends focused field type from native to java.
6) Adds a test to check if the model updates the
   view properly.

Screenshot: https://bugs.chromium.org/p/chromium/issues/detail?id=1104132#c11

Bug: 1104132
Change-Id: I4606a13214f5e815bce41207e72cde6caed60fd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2370542
Commit-Queue: Reda Tawfik <redatawfik@google.com>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805854}
parent 96c79582
......@@ -39,6 +39,7 @@ android_library("test_java") {
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/ManualFillingTestHelper.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/ManualFillingUiCaptureTest.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/PasswordGenerationIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetViewTest.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryModernViewTest.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryViewTest.java",
"javatests/src/org/chromium/chrome/browser/keyboard_accessory/sheet_component/AccessorySheetViewTest.java",
......@@ -69,6 +70,7 @@ android_library("test_java") {
"//chrome/test/android:chrome_java_test_support",
"//components/autofill/android:autofill_java",
"//components/autofill/core/common/mojom:mojo_types_java",
"//components/browser_ui/android/bottomsheet:java",
"//components/browser_ui/modaldialog/android:java",
"//components/browser_ui/widget/android:java",
"//components/browser_ui/widget/android:test_support_java",
......@@ -85,6 +87,7 @@ android_library("test_java") {
"//third_party/android_support_test_runner:runner_java",
"//third_party/hamcrest:hamcrest_java",
"//third_party/junit",
"//third_party/mockito:mockito_java",
"//ui/android:ui_full_java",
"//ui/android:ui_java_test_support",
"//ui/android:ui_utils_java",
......
......@@ -37,6 +37,7 @@ android_library("internal_java") {
"//third_party/android_deps:material_design_java",
"//ui/android:ui_java",
"//ui/android:ui_utils_java",
"//url:gurl_java",
]
sources = [
"java/src/org/chromium/chrome/browser/keyboard_accessory/AutofillKeyboardAccessoryViewBridge.java",
......@@ -53,6 +54,7 @@ android_library("internal_java") {
"java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetProperties.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetView.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetViewBinder.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetViewHolder.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/Credential.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryCoordinator.java",
"java/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryIPHUtils.java",
......
......@@ -8,5 +8,4 @@
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:layout_height="match_parent"
android:layout_width="match_parent"
android:orientation="vertical"/>
\ No newline at end of file
android:layout_width="match_parent" />
\ No newline at end of file
......@@ -52,8 +52,8 @@ class AllPasswordsBottomSheetBridge implements AllPasswordsBottomSheetCoordinato
}
@CalledByNative
private void showCredentials() {
mAllPasswordsBottomSheetCoordinator.showCredentials(mCredentials);
private void showCredentials(boolean isPasswordField) {
mAllPasswordsBottomSheetCoordinator.showCredentials(mCredentials, isPasswordField);
}
@Override
......
......@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_shee
import android.content.Context;
import androidx.annotation.VisibleForTesting;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
......@@ -51,11 +53,14 @@ class AllPasswordsBottomSheetCoordinator {
/**
* Displays the given credentials in a new bottom sheet.
* @param credentials An array of {@link Credential}s that will be displayed.
* @param isPasswordField True if the currently focused field is a password field and false for
* any other field type (e.g username, ...).
*/
public void showCredentials(Credential[] credentials) {
mMediator.showCredentials(credentials);
public void showCredentials(Credential[] credentials, boolean isPasswordField) {
mMediator.showCredentials(credentials, isPasswordField);
}
@VisibleForTesting
static void setUpModelChangeProcessor(PropertyModel model, AllPasswordsBottomSheetView view) {
PropertyModelChangeProcessor.create(
model, view, AllPasswordsBottomSheetViewBinder::bindAllPasswordsBottomSheet);
......
......@@ -4,8 +4,11 @@
package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyModel;
/**
......@@ -22,8 +25,20 @@ class AllPasswordsBottomSheetMediator {
mModel = model;
}
void showCredentials(Credential[] credentials) {
void showCredentials(Credential[] credentials, boolean isPasswordField) {
assert credentials != null;
ListModel<ListItem> sheetItems = mModel.get(SHEET_ITEMS);
sheetItems.clear();
for (Credential credential : credentials) {
final PropertyModel model =
AllPasswordsBottomSheetProperties.CredentialProperties.createCredentialModel(
credential, this::onCredentialSelected, isPasswordField);
sheetItems.add(
new ListItem(AllPasswordsBottomSheetProperties.ItemType.CREDENTIAL, model));
}
mModel.set(VISIBLE, true);
}
......
......@@ -4,12 +4,18 @@
package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet;
import androidx.annotation.IntDef;
import org.chromium.base.Callback;
import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
/**
* Properties defined here reflect the visible state of the AllPasswordsBottomSheet.
*/
......@@ -30,4 +36,45 @@ class AllPasswordsBottomSheetProperties {
.with(DISMISS_HANDLER, handler)
.build();
}
static class CredentialProperties {
static final PropertyModel.ReadableObjectPropertyKey<Credential> CREDENTIAL =
new PropertyModel.ReadableObjectPropertyKey<>("credential");
static final PropertyModel
.ReadableObjectPropertyKey<Callback<Credential>> ON_CLICK_LISTENER =
new PropertyModel.ReadableObjectPropertyKey<>("on_click_listener");
static final PropertyModel.ReadableBooleanPropertyKey IS_PASSWORD_FIELD =
new PropertyModel.ReadableBooleanPropertyKey("is_password_field");
static final PropertyKey[] ALL_KEYS = {CREDENTIAL, ON_CLICK_LISTENER, IS_PASSWORD_FIELD};
private CredentialProperties() {}
static PropertyModel createCredentialModel(Credential credential,
Callback<Credential> clickListener, boolean isPasswordField) {
return new PropertyModel
.Builder(AllPasswordsBottomSheetProperties.CredentialProperties.ALL_KEYS)
.with(CREDENTIAL, credential)
.with(ON_CLICK_LISTENER, clickListener)
.with(IS_PASSWORD_FIELD, isPasswordField)
.build();
}
}
@IntDef({ItemType.CREDENTIAL})
@Retention(RetentionPolicy.SOURCE)
@interface ItemType {
/**
* A section containing username, password and origin.
*/
int CREDENTIAL = 0;
}
/**
* Returns the sheet item type for a given item.
* @param item An {@link MVCListAdapter.ListItem}.
* @return The {@link ItemType} of the given list item.
*/
static @ItemType int getItemType(MVCListAdapter.ListItem item) {
return item.type;
}
}
......@@ -9,6 +9,7 @@ import android.view.LayoutInflater;
import android.view.View;
import androidx.annotation.Nullable;
import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView;
import org.chromium.base.Callback;
......@@ -26,7 +27,7 @@ import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
class AllPasswordsBottomSheetView implements BottomSheetContent {
private final BottomSheetController mBottomSheetController;
private Callback<Integer> mDismissHandler;
private final RecyclerView mContentView;
private final RecyclerView mSheetItemListView;
private final BottomSheetObserver mBottomSheetObserver = new EmptyBottomSheetObserver() {
@Override
......@@ -56,8 +57,11 @@ class AllPasswordsBottomSheetView implements BottomSheetContent {
public AllPasswordsBottomSheetView(
Context context, BottomSheetController bottomSheetController) {
mBottomSheetController = bottomSheetController;
mContentView = (RecyclerView) LayoutInflater.from(context).inflate(
mSheetItemListView = (RecyclerView) LayoutInflater.from(context).inflate(
R.layout.all_passwords_bottom_sheet, null);
mSheetItemListView.setLayoutManager(new LinearLayoutManager(
mSheetItemListView.getContext(), LinearLayoutManager.VERTICAL, false));
mSheetItemListView.setItemAnimator(null);
}
/**
......@@ -85,9 +89,13 @@ class AllPasswordsBottomSheetView implements BottomSheetContent {
}
}
void setSheetItemListAdapter(RecyclerView.Adapter adapter) {
mSheetItemListView.setAdapter(adapter);
}
@Override
public View getContentView() {
return mContentView;
return mSheetItemListView;
}
@Nullable
......
......@@ -4,11 +4,29 @@
package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.CredentialProperties.CREDENTIAL;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.CredentialProperties.IS_PASSWORD_FIELD;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.CredentialProperties.ON_CLICK_LISTENER;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.DISMISS_HANDLER;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
import android.text.method.PasswordTransformationMethod;
import android.view.View;
import android.view.ViewGroup;
import android.widget.TextView;
import org.chromium.chrome.browser.keyboard_accessory.R;
import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.ItemType;
import org.chromium.components.url_formatter.SchemeDisplay;
import org.chromium.components.url_formatter.UrlFormatter;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.RecyclerViewAdapter;
import org.chromium.ui.modelutil.SimpleRecyclerViewMcp;
import org.chromium.ui.widget.ChipView;
import org.chromium.url.GURL;
/**
* Provides functions that map {@link AllPasswordsBottomSheetProperties} changes in a {@link
......@@ -27,6 +45,96 @@ class AllPasswordsBottomSheetViewBinder {
view.setDismissHandler(model.get(DISMISS_HANDLER));
} else if (propertyKey == VISIBLE) {
view.setVisible(model.get(VISIBLE));
} else if (propertyKey == SHEET_ITEMS) {
view.setSheetItemListAdapter(new RecyclerViewAdapter<>(
new SimpleRecyclerViewMcp<>(model.get(SHEET_ITEMS),
AllPasswordsBottomSheetProperties::getItemType,
AllPasswordsBottomSheetViewBinder::connectPropertyModel),
AllPasswordsBottomSheetViewBinder::createViewHolder));
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
}
/**
* This method creates a model change processor for each recycler view item when it is created.
* @param holder A {@link AllPasswordsBottomSheetViewHolder} holding the view and view binder
* for the MCP.
* @param item A {@link MVCListAdapter.ListItem} holding the {@link PropertyModel} for the MCP.
*/
private static void connectPropertyModel(
AllPasswordsBottomSheetViewHolder holder, MVCListAdapter.ListItem item) {
holder.setupModelChangeProcessor(item.model);
}
/**
* Factory used to create a new View inside the ListView inside the AllPasswordsBottomSheetView.
* @param parent The parent {@link ViewGroup} of the new item.
* @param itemType The type of View to create.
*/
private static AllPasswordsBottomSheetViewHolder createViewHolder(
ViewGroup parent, @ItemType int itemType) {
switch (itemType) {
case ItemType.CREDENTIAL:
return new AllPasswordsBottomSheetViewHolder(parent,
R.layout.keyboard_accessory_sheet_tab_password_info,
AllPasswordsBottomSheetViewBinder::bindCredentialView);
}
assert false : "Cannot create view for ItemType: " + itemType;
return null;
}
/**
* Called whenever a credential is bound to this view holder. Please note that this method
* might be called on the same list entry repeatedly, so make sure to always set a default
* for unused fields.
* @param model The model containing the data for the view
* @param view The view to be bound
* @param propertyKey The key of the property to be bound
*/
private static void bindCredentialView(
PropertyModel model, View view, PropertyKey propertyKey) {
Credential credential = model.get(CREDENTIAL);
if (propertyKey == ON_CLICK_LISTENER) {
boolean isPasswordField = model.get(IS_PASSWORD_FIELD);
ChipView usernameView = view.findViewById(R.id.suggestion_text);
ChipView passwordView = view.findViewById(R.id.password_text);
if (isPasswordField) {
passwordView.setOnClickListener(
src -> model.get(ON_CLICK_LISTENER).onResult(credential));
usernameView.setOnClickListener(null);
} else {
usernameView.setOnClickListener(
src -> model.get(ON_CLICK_LISTENER).onResult(credential));
passwordView.setOnClickListener(null);
}
} else if (propertyKey == IS_PASSWORD_FIELD) {
boolean isPasswordField = model.get(IS_PASSWORD_FIELD);
ChipView usernameView = view.findViewById(R.id.suggestion_text);
String username = credential.getUsername();
usernameView.setEnabled(!isPasswordField && !username.isEmpty());
usernameView.setClickable(!isPasswordField && !username.isEmpty());
ChipView passwordView = view.findViewById(R.id.password_text);
passwordView.setEnabled(isPasswordField);
passwordView.setClickable(isPasswordField);
} else if (propertyKey == CREDENTIAL) {
TextView originView = view.findViewById(R.id.password_info_title);
String formattedOrigin = UrlFormatter.formatUrlForSecurityDisplay(
new GURL(credential.getOriginUrl()), SchemeDisplay.OMIT_CRYPTOGRAPHIC);
originView.setText(formattedOrigin);
ChipView usernameView = view.findViewById(R.id.suggestion_text);
usernameView.getPrimaryTextView().setText(credential.getFormattedUsername());
ChipView passwordView = view.findViewById(R.id.password_text);
passwordView.getPrimaryTextView().setTransformationMethod(
new PasswordTransformationMethod());
passwordView.getPrimaryTextView().setText(credential.getPassword());
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
}
}
// 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.keyboard_accessory.all_passwords_bottom_sheet;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.LayoutRes;
import androidx.recyclerview.widget.RecyclerView;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor.ViewBinder;
/**
* A {@link RecyclerView.ViewHolder} specifically meant to display a credential {@link Credential}.
*/
class AllPasswordsBottomSheetViewHolder extends RecyclerView.ViewHolder {
private final ViewBinder<PropertyModel, View, PropertyKey> mViewBinder;
AllPasswordsBottomSheetViewHolder(ViewGroup parent, @LayoutRes int layout,
ViewBinder<PropertyModel, View, PropertyKey> viewBinder) {
super(LayoutInflater.from(parent.getContext()).inflate(layout, parent, false));
mViewBinder = viewBinder;
}
/**
* Called whenever an item is bound to this view holder. Please note that this method
* might be called on the same list entry repeatedly, so make sure to always set a default
* for unused fields.
* @param model The {@link PropertyModel} whose data needs to be displayed.
*/
void setupModelChangeProcessor(PropertyModel model) {
PropertyModelChangeProcessor.create(model, itemView, mViewBinder, true);
}
}
// 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.keyboard_accessory.all_passwords_bottom_sheet;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.verify;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread;
import android.text.method.PasswordTransformationMethod;
import android.widget.TextView;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.filters.MediumTest;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.Callback;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.keyboard_accessory.R;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.SheetState;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.widget.ChipView;
/**
* View tests for the AllPasswordsBottomSheet ensure that model changes are reflected in the sheet.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@Features.EnableFeatures(ChromeFeatureList.FILLING_PASSWORDS_FROM_ANY_ORIGIN)
public class AllPasswordsBottomSheetViewTest {
private static final Credential ANA =
new Credential("Ana", "S3cr3t", "Ana", "https://example.com", false, false);
private static final Credential NO_ONE =
new Credential("", "***", "No Username", "https://m.example.xyz", true, false);
private static final Credential BOB =
new Credential("Bob", "***", "Bob", "http://mobile.example.xyz", true, false);
private static final boolean IS_PASSWORD_FIELD = true;
@Mock
private Callback<Integer> mDismissHandler;
@Mock
private Callback<Credential> mCredentialCallback;
private PropertyModel mModel;
private AllPasswordsBottomSheetView mAllPasswordsBottomSheetView;
private BottomSheetController mBottomSheetController;
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@Before
public void setUp() throws InterruptedException {
MockitoAnnotations.initMocks(this);
mActivityTestRule.startMainActivityOnBlankPage();
mModel = AllPasswordsBottomSheetProperties.createDefaultModel(mDismissHandler);
mBottomSheetController = mActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mAllPasswordsBottomSheetView =
new AllPasswordsBottomSheetView(getActivity(), mBottomSheetController);
AllPasswordsBottomSheetCoordinator.setUpModelChangeProcessor(
mModel, mAllPasswordsBottomSheetView);
});
}
@Test
@MediumTest
public void testVisibilityChangedByModel() {
// After setting the visibility to true, the view should exist and be visible.
TestThreadUtils.runOnUiThreadBlocking(() -> mModel.set(VISIBLE, true));
pollUiThread(() -> getBottomSheetState() == SheetState.FULL);
assertThat(mAllPasswordsBottomSheetView.getContentView().isShown(), is(true));
// After hiding the view, the view should still exist but be invisible.
TestThreadUtils.runOnUiThreadBlocking(() -> mModel.set(VISIBLE, false));
pollUiThread(() -> getBottomSheetState() == BottomSheetController.SheetState.HIDDEN);
assertThat(mAllPasswordsBottomSheetView.getContentView().isShown(), is(false));
}
@Test
@MediumTest
public void testCredentialsChangedByModel() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mAllPasswordsBottomSheetView.setVisible(true);
mModel.get(SHEET_ITEMS)
.add(new ListItem(AllPasswordsBottomSheetProperties.ItemType.CREDENTIAL,
AllPasswordsBottomSheetProperties.CredentialProperties
.createCredentialModel(
ANA, mCredentialCallback, IS_PASSWORD_FIELD)));
mModel.get(SHEET_ITEMS)
.add(new ListItem(AllPasswordsBottomSheetProperties.ItemType.CREDENTIAL,
AllPasswordsBottomSheetProperties.CredentialProperties
.createCredentialModel(
NO_ONE, mCredentialCallback, IS_PASSWORD_FIELD)));
mModel.get(SHEET_ITEMS)
.add(new ListItem(AllPasswordsBottomSheetProperties.ItemType.CREDENTIAL,
AllPasswordsBottomSheetProperties.CredentialProperties
.createCredentialModel(
BOB, mCredentialCallback, IS_PASSWORD_FIELD)));
});
pollUiThread(() -> getBottomSheetState() == SheetState.FULL);
assertThat(getCredentials().getChildCount(), is(3));
assertThat(getCredentialOriginAt(0).getText(), is("example.com"));
assertThat(getCredentialNameAt(0).getPrimaryTextView().getText(),
is(ANA.getFormattedUsername()));
assertThat(
getCredentialPasswordAt(0).getPrimaryTextView().getText(), is(ANA.getPassword()));
assertThat(getCredentialPasswordAt(0).getPrimaryTextView().getTransformationMethod(),
instanceOf(PasswordTransformationMethod.class));
assertThat(getCredentialNameAt(0).isEnabled(), is(false));
assertThat(getCredentialNameAt(0).isClickable(), is(false));
assertThat(getCredentialPasswordAt(0).isEnabled(), is(true));
assertThat(getCredentialPasswordAt(0).isClickable(), is(true));
assertThat(getCredentialOriginAt(1).getText(), is("m.example.xyz"));
assertThat(getCredentialNameAt(1).getPrimaryTextView().getText(),
is(NO_ONE.getFormattedUsername()));
assertThat(getCredentialPasswordAt(1).getPrimaryTextView().getText(),
is(NO_ONE.getPassword()));
assertThat(getCredentialPasswordAt(1).getPrimaryTextView().getTransformationMethod(),
instanceOf(PasswordTransformationMethod.class));
assertThat(getCredentialNameAt(1).isEnabled(), is(false));
assertThat(getCredentialNameAt(1).isClickable(), is(false));
assertThat(getCredentialPasswordAt(1).isEnabled(), is(true));
assertThat(getCredentialPasswordAt(1).isClickable(), is(true));
assertThat(getCredentialOriginAt(2).getText(), is("http://mobile.example.xyz"));
assertThat(getCredentialNameAt(2).getPrimaryTextView().getText(),
is(BOB.getFormattedUsername()));
assertThat(
getCredentialPasswordAt(2).getPrimaryTextView().getText(), is(BOB.getPassword()));
assertThat(getCredentialPasswordAt(2).getPrimaryTextView().getTransformationMethod(),
instanceOf(PasswordTransformationMethod.class));
assertThat(getCredentialNameAt(2).isEnabled(), is(false));
assertThat(getCredentialNameAt(2).isClickable(), is(false));
assertThat(getCredentialPasswordAt(2).isEnabled(), is(true));
assertThat(getCredentialPasswordAt(2).isClickable(), is(true));
}
@Test
@MediumTest
public void testDismissesWhenHidden() {
TestThreadUtils.runOnUiThreadBlocking(() -> mModel.set(VISIBLE, true));
pollUiThread(() -> getBottomSheetState() == SheetState.FULL);
TestThreadUtils.runOnUiThreadBlocking(() -> mModel.set(VISIBLE, false));
pollUiThread(() -> getBottomSheetState() == BottomSheetController.SheetState.HIDDEN);
verify(mDismissHandler).onResult(BottomSheetController.StateChangeReason.NONE);
}
private ChromeActivity getActivity() {
return mActivityTestRule.getActivity();
}
private @SheetState int getBottomSheetState() {
return mBottomSheetController.getSheetState();
}
private RecyclerView getCredentials() {
return (RecyclerView) mAllPasswordsBottomSheetView.getContentView();
}
private TextView getCredentialOriginAt(int index) {
return getCredentials().getChildAt(index).findViewById(R.id.password_info_title);
}
private ChipView getCredentialNameAt(int index) {
return ((ChipView) getCredentials().getChildAt(index).findViewById(R.id.suggestion_text));
}
private ChipView getCredentialPasswordAt(int index) {
return ((ChipView) getCredentials().getChildAt(index).findViewById(R.id.password_text));
}
}
......@@ -9,6 +9,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.verify;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.CredentialProperties.CREDENTIAL;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.DISMISS_HANDLER;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
......@@ -22,8 +23,11 @@ import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.ItemType;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyModel;
/**
......@@ -33,8 +37,15 @@ import org.chromium.ui.modelutil.PropertyModel;
@Config(manifest = Config.NONE)
@Features.EnableFeatures(ChromeFeatureList.FILLING_PASSWORDS_FROM_ANY_ORIGIN)
public class AllPasswordsBottomSheetControllerTest {
private static final Credential[] TEST_CREDENTIALS = new Credential[] {
new Credential("user1", "password123", "user1", "origin.com", false, false)};
private static final Credential ANA =
new Credential("Ana", "S3cr3t", "Ana", "https://m.a.xyz/", true, false);
private static final Credential BOB =
new Credential("Bob", "*****", "Bob", "https://subdomain.example.xyz", true, false);
private static final Credential CARL =
new Credential("Carl", "G3h3!m", "Carl", "https://www.example.xyz", false, false);
private static final Credential[] TEST_CREDENTIALS = new Credential[] {ANA, BOB, CARL};
private static final boolean IS_PASSWORD_FIELD = true;
@Mock
private AllPasswordsBottomSheetCoordinator.Delegate mMockDelegate;
......@@ -58,21 +69,35 @@ public class AllPasswordsBottomSheetControllerTest {
@Test
public void testShowCredentialsSetsVisibile() {
mMediator.showCredentials(TEST_CREDENTIALS);
mMediator.showCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
assertThat(mModel.get(VISIBLE), is(true));
}
@Test
public void testShowCredentialSetsCredentialListModel() {
mMediator.showCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
ListModel<ListItem> itemList = mModel.get(SHEET_ITEMS);
assertThat(itemList.size(), is(3));
assertThat(itemList.get(0).type, is(ItemType.CREDENTIAL));
assertThat(itemList.get(0).model.get(CREDENTIAL), is(ANA));
assertThat(itemList.get(1).type, is(ItemType.CREDENTIAL));
assertThat(itemList.get(1).model.get(CREDENTIAL), is(BOB));
assertThat(itemList.get(2).type, is(ItemType.CREDENTIAL));
assertThat(itemList.get(2).model.get(CREDENTIAL), is(CARL));
}
@Test
public void testOnCredentialSelected() {
mMediator.showCredentials(TEST_CREDENTIALS);
mMediator.onCredentialSelected(TEST_CREDENTIALS[0]);
mMediator.showCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
mMediator.onCredentialSelected(TEST_CREDENTIALS[1]);
assertThat(mModel.get(VISIBLE), is(false));
verify(mMockDelegate).onCredentialSelected(TEST_CREDENTIALS[0]);
verify(mMockDelegate).onCredentialSelected(TEST_CREDENTIALS[1]);
}
@Test
public void testOnDismiss() {
mMediator.showCredentials(TEST_CREDENTIALS);
mMediator.showCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
mMediator.onDismissed(BottomSheetController.StateChangeReason.BACK_PRESS);
assertThat(mModel.get(VISIBLE), is(false));
verify(mMockDelegate).onDismissed();
......
......@@ -13,6 +13,7 @@
#include "components/password_manager/core/browser/password_store.h"
#include "content/public/browser/web_contents.h"
using autofill::mojom::FocusedFieldType;
using password_manager::UiCredential;
// No-op constructor for tests.
......@@ -21,20 +22,24 @@ AllPasswordsBottomSheetController::AllPasswordsBottomSheetController(
std::unique_ptr<AllPasswordsBottomSheetView> view,
base::WeakPtr<password_manager::PasswordManagerDriver> driver,
password_manager::PasswordStore* store,
base::OnceCallback<void()> dismissal_callback)
base::OnceCallback<void()> dismissal_callback,
FocusedFieldType focused_field_type)
: view_(std::move(view)),
store_(store),
dismissal_callback_(std::move(dismissal_callback)),
driver_(std::move(driver)) {}
driver_(std::move(driver)),
focused_field_type_(focused_field_type) {}
AllPasswordsBottomSheetController::AllPasswordsBottomSheetController(
content::WebContents* web_contents,
password_manager::PasswordStore* store,
base::OnceCallback<void()> dismissal_callback)
base::OnceCallback<void()> dismissal_callback,
FocusedFieldType focused_field_type)
: view_(std::make_unique<AllPasswordsBottomSheetViewImpl>(this)),
web_contents_(web_contents),
store_(store),
dismissal_callback_(std::move(dismissal_callback)) {
dismissal_callback_(std::move(dismissal_callback)),
focused_field_type_(focused_field_type) {
DCHECK(web_contents_);
DCHECK(store_);
DCHECK(dismissal_callback_);
......@@ -56,7 +61,7 @@ void AllPasswordsBottomSheetController::Show() {
void AllPasswordsBottomSheetController::OnGetPasswordStoreResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
// TODO(crbug.com/1104132): Handle empty credentials case.
view_->Show(std::move(results));
view_->Show(std::move(results), focused_field_type_);
}
gfx::NativeView AllPasswordsBottomSheetController::GetNativeView() {
......@@ -65,7 +70,15 @@ gfx::NativeView AllPasswordsBottomSheetController::GetNativeView() {
void AllPasswordsBottomSheetController::OnCredentialSelected(
const UiCredential& credential) {
driver_->FillSuggestion(credential.username(), credential.password());
const bool is_password_field =
focused_field_type_ == FocusedFieldType::kFillablePasswordField;
DCHECK(driver_);
if (is_password_field) {
driver_->FillIntoFocusedField(is_password_field, credential.password());
} else {
driver_->FillIntoFocusedField(is_password_field, credential.username());
}
// Consumes the dismissal callback to destroy the native controller and java
// controller after the user selects a credential.
OnDismiss();
......
......@@ -7,6 +7,7 @@
#include "base/callback.h"
#include "base/util/type_safety/pass_key.h"
#include "components/autofill/core/common/mojom/autofill_types.mojom-forward.h"
#include "components/password_manager/core/browser/password_store_consumer.h"
#include "ui/gfx/native_widget_types.h"
......@@ -31,11 +32,14 @@ class AllPasswordsBottomSheetController
std::unique_ptr<AllPasswordsBottomSheetView> view,
base::WeakPtr<password_manager::PasswordManagerDriver> driver,
password_manager::PasswordStore* store,
base::OnceCallback<void()> dismissal_callback);
base::OnceCallback<void()> dismissal_callback,
autofill::mojom::FocusedFieldType focused_field_type);
AllPasswordsBottomSheetController(
content::WebContents* web_contents,
password_manager::PasswordStore* store,
base::OnceCallback<void()> dismissal_callback);
base::OnceCallback<void()> dismissal_callback,
autofill::mojom::FocusedFieldType focused_field_type);
~AllPasswordsBottomSheetController() override;
AllPasswordsBottomSheetController(const AllPasswordsBottomSheetController&) =
delete;
......@@ -77,6 +81,9 @@ class AllPasswordsBottomSheetController
// Either |driver_| is created and owned by this controller or received in
// constructor specified for tests.
base::WeakPtr<password_manager::PasswordManagerDriver> driver_;
// The type of field on which the user is focused, e.g. PASSWORD.
autofill::mojom::FocusedFieldType focused_field_type_;
};
#endif // CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_ALL_PASSWORDS_BOTTOM_SHEET_CONTROLLER_H_
......@@ -10,6 +10,7 @@
#include "chrome/browser/password_manager/password_store_factory.h"
#include "chrome/browser/ui/android/passwords/all_passwords_bottom_sheet_view.h"
#include "chrome/test/base/testing_profile.h"
#include "components/autofill/core/common/mojom/autofill_types.mojom-forward.h"
#include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/origin_credential_store.h"
#include "components/password_manager/core/browser/password_manager_test_utils.h"
......@@ -28,6 +29,7 @@ using autofill::PasswordForm;
using password_manager::TestPasswordStore;
using password_manager::UiCredential;
using CallbackFunctionMock = testing::MockFunction<void()>;
using autofill::mojom::FocusedFieldType;
using DismissCallback = base::MockCallback<base::OnceCallback<void()>>;
......@@ -46,8 +48,8 @@ class MockPasswordManagerDriver
: public password_manager::StubPasswordManagerDriver {
public:
MOCK_METHOD(void,
FillSuggestion,
(const base::string16&, const base::string16&),
FillIntoFocusedField,
(bool, const base::string16&),
(override));
};
......@@ -55,7 +57,8 @@ class MockAllPasswordsBottomSheetView : public AllPasswordsBottomSheetView {
public:
MOCK_METHOD(void,
Show,
(const std::vector<std::unique_ptr<PasswordForm>>&),
(const std::vector<std::unique_ptr<PasswordForm>>&,
FocusedFieldType),
(override));
};
......@@ -100,7 +103,8 @@ class AllPasswordsBottomSheetControllerTest : public testing::Test {
std::make_unique<AllPasswordsBottomSheetController>(
util::PassKey<AllPasswordsBottomSheetControllerTest>(),
std::move(mock_view_unique_ptr), driver_.AsWeakPtr(), store_.get(),
dissmissal_callback_.Get());
dissmissal_callback_.Get(),
FocusedFieldType::kFillablePasswordField);
}
MockPasswordManagerDriver& driver() { return driver_; }
......@@ -139,9 +143,10 @@ TEST_F(AllPasswordsBottomSheetControllerTest, Show) {
store().AddLogin(form3);
store().AddLogin(form4);
EXPECT_CALL(view(), Show(UnorderedElementsAre(
Pointee(Eq(form1)), Pointee(Eq(form2)),
Pointee(Eq(form3)), Pointee(Eq(form4)))));
EXPECT_CALL(view(),
Show(UnorderedElementsAre(Pointee(Eq(form1)), Pointee(Eq(form2)),
Pointee(Eq(form3)), Pointee(Eq(form4))),
FocusedFieldType::kFillablePasswordField));
all_passwords_controller()->Show();
// Show method uses the store which has async work.
......@@ -151,8 +156,8 @@ TEST_F(AllPasswordsBottomSheetControllerTest, Show) {
TEST_F(AllPasswordsBottomSheetControllerTest, OnCredentialSelected) {
UiCredential credential = MakeUiCredential(kUsername1, kPassword);
EXPECT_CALL(driver(), FillSuggestion(base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword)));
EXPECT_CALL(driver(),
FillIntoFocusedField(true, base::ASCIIToUTF16(kPassword)));
all_passwords_controller()->OnCredentialSelected(credential);
}
......
......@@ -239,6 +239,7 @@ void PasswordAccessoryControllerImpl::OnToggleChanged(
void PasswordAccessoryControllerImpl::RefreshSuggestionsForField(
FocusedFieldType focused_field_type,
bool is_manual_generation_available) {
last_focused_field_type_ = focused_field_type;
// Prevent crashing by not acting at all if frame became unfocused at any
// point. The next time a focus event happens, this will be called again and
// ensure we show correct data.
......@@ -408,12 +409,14 @@ void PasswordAccessoryControllerImpl::ShowAllPasswords() {
// |AllPasswordsSheetDismissed| we are sure that this controller is alive as
// it owns |AllPasswordsBottomSheetController| from which the method is
// called.
// TODO(crbug.com/1104132): Update the controller with the last focused field.
all_passords_bottom_sheet_controller_ =
std::make_unique<AllPasswordsBottomSheetController>(
web_contents_, password_client_->GetProfilePasswordStore(),
base::BindOnce(
&PasswordAccessoryControllerImpl::AllPasswordsSheetDismissed,
base::Unretained(this)));
base::Unretained(this)),
last_focused_field_type_);
all_passords_bottom_sheet_controller_->Show();
}
......
......@@ -135,6 +135,10 @@ class PasswordAccessoryControllerImpl
std::unique_ptr<AllPasswordsBottomSheetController>
all_passords_bottom_sheet_controller_;
// Records the last focused field type that `RefreshSuggestionsForField()` was
// called with.
autofill::mojom::FocusedFieldType last_focused_field_type_;
WEB_CONTENTS_USER_DATA_KEY_DECL();
DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryControllerImpl);
......
......@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_UI_ANDROID_PASSWORDS_ALL_PASSWORDS_BOTTOM_SHEET_VIEW_H_
#define CHROME_BROWSER_UI_ANDROID_PASSWORDS_ALL_PASSWORDS_BOTTOM_SHEET_VIEW_H_
#include "components/autofill/core/common/mojom/autofill_types.mojom-forward.h"
namespace autofill {
struct PasswordForm;
} // namespace autofill
......@@ -21,8 +23,9 @@ class AllPasswordsBottomSheetView {
// Instructs All Passwords Sheet to show the provided |credentials| to the
// user.
virtual void Show(const std::vector<std::unique_ptr<autofill::PasswordForm>>&
credentials) = 0;
virtual void Show(
const std::vector<std::unique_ptr<autofill::PasswordForm>>& credentials,
autofill::mojom::FocusedFieldType focused_field_type) = 0;
};
#endif // CHROME_BROWSER_UI_ANDROID_PASSWORDS_ALL_PASSWORDS_BOTTOM_SHEET_VIEW_H_
......@@ -16,6 +16,7 @@
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "ui/android/window_android.h"
using autofill::mojom::FocusedFieldType;
using base::android::AttachCurrentThread;
using base::android::ConvertJavaStringToUTF16;
using base::android::ConvertJavaStringToUTF8;
......@@ -56,7 +57,8 @@ AllPasswordsBottomSheetViewImpl::~AllPasswordsBottomSheetViewImpl() {
}
void AllPasswordsBottomSheetViewImpl::Show(
const std::vector<std::unique_ptr<autofill::PasswordForm>>& credentials) {
const std::vector<std::unique_ptr<autofill::PasswordForm>>& credentials,
FocusedFieldType focused_field_type) {
auto java_object = GetOrCreateJavaObject();
if (!java_object)
return;
......@@ -78,7 +80,10 @@ void AllPasswordsBottomSheetViewImpl::Show(
credential->is_affiliation_based_match);
}
Java_AllPasswordsBottomSheetBridge_showCredentials(env, java_object);
const bool is_password_field =
focused_field_type == FocusedFieldType::kFillablePasswordField;
Java_AllPasswordsBottomSheetBridge_showCredentials(env, java_object,
is_password_field);
}
void AllPasswordsBottomSheetViewImpl::OnCredentialSelected(
......
......@@ -8,6 +8,7 @@
#include <memory>
#include "base/android/scoped_java_ref.h"
#include "chrome/browser/ui/android/passwords/all_passwords_bottom_sheet_view.h"
#include "components/autofill/core/common/mojom/autofill_types.mojom-forward.h"
namespace autofill {
struct PasswordForm;
......@@ -28,8 +29,9 @@ class AllPasswordsBottomSheetViewImpl : public AllPasswordsBottomSheetView {
~AllPasswordsBottomSheetViewImpl() override;
// AllPasswordsBottomSheetView:
void Show(const std::vector<std::unique_ptr<autofill::PasswordForm>>&
credentials) override;
void Show(
const std::vector<std::unique_ptr<autofill::PasswordForm>>& credentials,
autofill::mojom::FocusedFieldType focused_field_type) override;
// Invoked in case the user chooses an entry from the credential list
// presented to them.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment