Commit b9184f39 authored by Reda Tawfik's avatar Reda Tawfik Committed by Commit Bot

[Android][Mfill] Sort credentials in AllPasswordsBottomSheet

This CL sorts the credentials in AllPasswordsBottomSheet alphabetically
according to second-level domain name.

Bug: 1133326, 1104132
Change-Id: I31bd63c51a0e842a3a8f92470a30f56342886471
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440889Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Reda Tawfik <redatawfik@google.com>
Cr-Commit-Position: refs/heads/master@{#812663}
parent f6ff2b0d
...@@ -122,6 +122,7 @@ junit_binary("keyboard_accessory_junit_tests") { ...@@ -122,6 +122,7 @@ junit_binary("keyboard_accessory_junit_tests") {
"//components/autofill/android:autofill_java", "//components/autofill/android:autofill_java",
"//components/browser_ui/android/bottomsheet:java", "//components/browser_ui/android/bottomsheet:java",
"//components/embedder_support/android:content_view_java", "//components/embedder_support/android:content_view_java",
"//components/embedder_support/android:util_java",
"//components/feature_engagement/public:public_java", "//components/feature_engagement/public:public_java",
"//components/module_installer/android:module_installer_java", "//components/module_installer/android:module_installer_java",
"//content/public/android:content_java", "//content/public/android:content_java",
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_shee ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_shee
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.SHEET_ITEMS;
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE; import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.ui.modaldialog.DialogDismissalCause; import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
...@@ -14,6 +15,7 @@ import org.chromium.ui.modelutil.ListModel; ...@@ -14,6 +15,7 @@ import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem; import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
import java.util.Arrays;
import java.util.Locale; import java.util.Locale;
/** /**
...@@ -57,6 +59,8 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle ...@@ -57,6 +59,8 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
void setCredentials(Credential[] credentials, boolean isPasswordField) { void setCredentials(Credential[] credentials, boolean isPasswordField) {
assert credentials != null; assert credentials != null;
Arrays.sort(credentials, AllPasswordsBottomSheetMediator::compareCredentials);
mCredentials = credentials; mCredentials = credentials;
mIsPasswordField = isPasswordField; mIsPasswordField = isPasswordField;
...@@ -135,4 +139,14 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle ...@@ -135,4 +139,14 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
mModel.set(VISIBLE, false); mModel.set(VISIBLE, false);
mDelegate.onDismissed(); mDelegate.onDismissed();
} }
private static int compareCredentials(Credential credential1, Credential credential2) {
String displayOrigin1 = credential1.isAndroidCredential()
? credential1.getAppDisplayName().toLowerCase(Locale.ENGLISH)
: UrlUtilities.getDomainAndRegistry(credential1.getOriginUrl(), false);
String displayOrigin2 = credential2.isAndroidCredential()
? credential2.getAppDisplayName().toLowerCase(Locale.ENGLISH)
: UrlUtilities.getDomainAndRegistry(credential2.getOriginUrl(), false);
return displayOrigin1.compareTo(displayOrigin2);
}
} }
...@@ -7,7 +7,10 @@ package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_shee ...@@ -7,7 +7,10 @@ package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_shee
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
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.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.DISMISS_HANDLER;
...@@ -15,17 +18,22 @@ import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_botto ...@@ -15,17 +18,22 @@ import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_botto
import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE; import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.TestRule;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.ItemType; import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.ItemType;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.components.embedder_support.util.UrlUtilitiesJni;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
import org.chromium.ui.modaldialog.ModalDialogProperties.ButtonType; import org.chromium.ui.modaldialog.ModalDialogProperties.ButtonType;
...@@ -41,14 +49,20 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -41,14 +49,20 @@ import org.chromium.ui.modelutil.PropertyModel;
@Features.EnableFeatures(ChromeFeatureList.FILLING_PASSWORDS_FROM_ANY_ORIGIN) @Features.EnableFeatures(ChromeFeatureList.FILLING_PASSWORDS_FROM_ANY_ORIGIN)
public class AllPasswordsBottomSheetControllerTest { public class AllPasswordsBottomSheetControllerTest {
private static final Credential ANA = private static final Credential ANA =
new Credential("Ana", "S3cr3t", "Ana", "https://m.a.xyz/", false, ""); new Credential("Ana", "S3cr3t", "Ana", "https://m.domain.xyz/", false, "");
private static final Credential BOB = private static final Credential BOB =
new Credential("Bob", "*****", "Bob", "https://subdomain.example.xyz", false, ""); new Credential("Bob", "*****", "Bob", "https://subdomain.example.xyz", false, "");
private static final Credential CARL = private static final Credential CARL =
new Credential("Carl", "G3h3!m", "Carl", "https://www.example.xyz", false, ""); new Credential("Carl", "G3h3!m", "Carl", "https://www.origin.xyz", false, "");
private static final Credential[] TEST_CREDENTIALS = new Credential[] {ANA, BOB, CARL}; private static final Credential[] TEST_CREDENTIALS = new Credential[] {BOB, CARL, ANA};
private static final boolean IS_PASSWORD_FIELD = true; private static final boolean IS_PASSWORD_FIELD = true;
@Rule
public JniMocker mJniMocker = new JniMocker();
@Rule
public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();
@Mock
private UrlUtilities.Natives mUrlUtilitiesJniMock;
@Mock @Mock
private AllPasswordsBottomSheetCoordinator.Delegate mMockDelegate; private AllPasswordsBottomSheetCoordinator.Delegate mMockDelegate;
...@@ -62,11 +76,15 @@ public class AllPasswordsBottomSheetControllerTest { ...@@ -62,11 +76,15 @@ public class AllPasswordsBottomSheetControllerTest {
@Before @Before
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
mJniMocker.mock(UrlUtilitiesJni.TEST_HOOKS, mUrlUtilitiesJniMock);
mMediator = new AllPasswordsBottomSheetMediator(); mMediator = new AllPasswordsBottomSheetMediator();
mModalDialogModel = getModalDialogModel(mMediator); mModalDialogModel = getModalDialogModel(mMediator);
mModel = AllPasswordsBottomSheetProperties.createDefaultModel( mModel = AllPasswordsBottomSheetProperties.createDefaultModel(
mMediator::onDismissed, mMediator::onQueryTextChange); mMediator::onDismissed, mMediator::onQueryTextChange);
mMediator.initialize(mModalDialogManager, mModalDialogModel, mMockDelegate, mModel); mMediator.initialize(mModalDialogManager, mModalDialogModel, mMockDelegate, mModel);
when(mUrlUtilitiesJniMock.getDomainAndRegistry(anyString(), anyBoolean()))
.then(inv -> getDomainAndRegistry(inv.getArgument(0)));
} }
@Test @Test
...@@ -148,10 +166,32 @@ public class AllPasswordsBottomSheetControllerTest { ...@@ -148,10 +166,32 @@ public class AllPasswordsBottomSheetControllerTest {
assertThat(mModel.get(SHEET_ITEMS).size(), is(1)); assertThat(mModel.get(SHEET_ITEMS).size(), is(1));
} }
@Test
public void testCredentialSortedByOrigin() {
mMediator.setCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
ListModel<ListItem> itemList = mModel.get(SHEET_ITEMS);
assertThat(itemList.get(0).model.get(CREDENTIAL), is(ANA));
assertThat(itemList.get(1).model.get(CREDENTIAL), is(BOB));
assertThat(itemList.get(2).model.get(CREDENTIAL), is(CARL));
}
private PropertyModel getModalDialogModel(AllPasswordsBottomSheetMediator mediator) { private PropertyModel getModalDialogModel(AllPasswordsBottomSheetMediator mediator) {
return new PropertyModel.Builder(ModalDialogProperties.ALL_KEYS) return new PropertyModel.Builder(ModalDialogProperties.ALL_KEYS)
.with(ModalDialogProperties.CONTROLLER, mediator) .with(ModalDialogProperties.CONTROLLER, mediator)
.with(ModalDialogProperties.FILTER_TOUCH_FOR_SECURITY, true) .with(ModalDialogProperties.FILTER_TOUCH_FOR_SECURITY, true)
.build(); .build();
} }
/**
* Helper to get organization-identifying host from URLs. The real implementation calls
* {@link UrlUtilities}. It's not useful to actually reimplement it, so just return a string in
* a trivial way.
* @param origin A URL.
* @return The organization-identifying host from the given URL.
*/
private String getDomainAndRegistry(String origin) {
return origin.replaceAll(".*\\.(.+\\.[^.]+$)", "$1");
}
} }
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