[PwdMessageDialog] Enable saving usernames in password edit dialog wi…

…th details view

Changes password edit dialog view in the way, that it's now possible to
enter a new username.
Enables username editing in PasswordEditDialogWithDetailsView.

Basically it just changes onDialogAccepted(int selectedUsernameIndex,
String password) to onDialogAccepted(String username, String password).

Bug: 1315916
Change-Id: I42901ca6a92558c5684fbb9f64ba6968f5ff92e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629367
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Commit-Queue: Anna Tsvirchkova <atsvirchkova@google.com>
Cr-Commit-Position: refs/heads/main@{#1001500}

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogBridge.java

@@ -42,10 +42,9 @@ void dismiss() {
     }
 
     @Override
-    public void onDialogAccepted(int selectedUsernameIndex, String password) {
+    public void onDialogAccepted(String username, String password) {
         assert mNativeDialog != 0;
-        PasswordEditDialogBridgeJni.get().onDialogAccepted(
-                mNativeDialog, selectedUsernameIndex, password);
+        PasswordEditDialogBridgeJni.get().onDialogAccepted(mNativeDialog, username, password);
     }
 
     @Override
@@ -58,7 +57,7 @@ public void onDialogDismissed(boolean dialogAccepted) {
     @NativeMethods
     interface Natives {
         void onDialogAccepted(
-                long nativePasswordEditDialogBridge, int selectedUsernameIndex, String password);
+                long nativePasswordEditDialogBridge, String username, String password);
         void onDialogDismissed(long nativePasswordEditDialogBridge, boolean dialogAccepted);
     }
 }

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogCoordinator.java

@@ -38,10 +38,10 @@ interface Delegate {
         /**
          * Called when the user taps the dialog positive button.
          *
-         * @param selectedUsernameIndex The index of the username selected by the user.
+         * @param username The username, whose password is to be updated or saved (if it's new)
          * @param password The password to be saved
          */
-        void onDialogAccepted(int selectedUsernameIndex, String password);
+        void onDialogAccepted(String username, String password);
 
         /**
          * Called when the dialog is dismissed.
@@ -127,10 +127,10 @@ private void createDialogViewModel(
         PropertyModel.Builder dialogViewModelBuilder =
                 new PropertyModel.Builder(PasswordEditDialogProperties.ALL_KEYS)
                         .with(PasswordEditDialogProperties.USERNAMES, Arrays.asList(usernames))
-                        .with(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX,
-                                selectedUsernameIndex)
-                        .with(PasswordEditDialogProperties.USERNAME_SELECTED_CALLBACK,
-                                this::handleUsernameSelected)
+                        .with(PasswordEditDialogProperties.USERNAME,
+                                usernames[selectedUsernameIndex])
+                        .with(PasswordEditDialogProperties.USERNAME_CHANGED_CALLBACK,
+                                this::handleUsernameChanged)
                         .with(PasswordEditDialogProperties.PASSWORD, password);
         if (ChromeFeatureList.isEnabled(ChromeFeatureList.PASSWORD_EDIT_DIALOG_WITH_DETAILS)) {
             dialogViewModelBuilder
@@ -180,9 +180,8 @@ void dismiss() {
         mModalDialogManager.dismissDialog(mDialogModel, DialogDismissalCause.DISMISSED_BY_NATIVE);
     }
 
-    private void handleUsernameSelected(int selectedUsernameIndex) {
-        mDialogViewModel.set(
-                PasswordEditDialogProperties.SELECTED_USERNAME_INDEX, selectedUsernameIndex);
+    private void handleUsernameChanged(String selectedUsername) {
+        mDialogViewModel.set(PasswordEditDialogProperties.USERNAME, selectedUsername);
     }
 
     private void handlePasswordChanged(String password) {
@@ -194,8 +193,7 @@ private void handlePasswordChanged(String password) {
     @Override
     public void onClick(PropertyModel model, @ButtonType int buttonType) {
         if (buttonType == ButtonType.POSITIVE) {
-            mDelegate.onDialogAccepted(
-                    mDialogViewModel.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX),
+            mDelegate.onDialogAccepted(mDialogViewModel.get(PasswordEditDialogProperties.USERNAME),
                     mDialogViewModel.get(PasswordEditDialogProperties.PASSWORD));
         }
         mModalDialogManager.dismissDialog(model,

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogProperties.java

@@ -19,14 +19,14 @@ class PasswordEditDialogProperties {
      * selected username.
      */
     static final PropertyModel
-            .ReadableObjectPropertyKey<Callback<Integer>> USERNAME_SELECTED_CALLBACK =
+            .ReadableObjectPropertyKey<Callback<String>> USERNAME_CHANGED_CALLBACK =
             new PropertyModel.ReadableObjectPropertyKey<>("username selected callback");
 
     static final PropertyModel.ReadableObjectPropertyKey<List<String>> USERNAMES =
             new PropertyModel.ReadableObjectPropertyKey<>("usernames");
 
-    static final PropertyModel.WritableIntPropertyKey SELECTED_USERNAME_INDEX =
-            new PropertyModel.WritableIntPropertyKey("selected username index");
+    static final PropertyModel.WritableObjectPropertyKey<String> USERNAME =
+            new PropertyModel.WritableObjectPropertyKey<>("username");
 
     static final PropertyModel.WritableObjectPropertyKey<String> PASSWORD =
             new PropertyModel.WritableObjectPropertyKey<>("password");
@@ -41,7 +41,6 @@ class PasswordEditDialogProperties {
     static final PropertyModel.WritableBooleanPropertyKey EMPTY_PASSWORD_ERROR =
             new PropertyModel.WritableBooleanPropertyKey("empty password error");
 
-    static final PropertyKey[] ALL_KEYS = {USERNAME_SELECTED_CALLBACK, USERNAMES,
-            SELECTED_USERNAME_INDEX, PASSWORD, PASSWORD_CHANGED_CALLBACK, EMPTY_PASSWORD_ERROR,
-            FOOTER};
+    static final PropertyKey[] ALL_KEYS = {USERNAME_CHANGED_CALLBACK, USERNAMES, USERNAME, PASSWORD,
+            PASSWORD_CHANGED_CALLBACK, EMPTY_PASSWORD_ERROR, FOOTER};
 }

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogTest.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.password_edit_dialog;
 
 import static org.hamcrest.Matchers.contains;
-import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.never;
 
@@ -42,7 +41,8 @@ public class PasswordEditDialogTest {
     private static final long NATIVE_PTR = 1;
     private static final String[] USERNAMES = {"user1", "user2", "user3"};
     private static final int INITIAL_USERNAME_INDEX = 1;
-    private static final int SELECTED_USERNAME_INDEX = 2;
+    private static final String INITIAL_USERNAME = USERNAMES[INITIAL_USERNAME_INDEX];
+    private static final String CHANGED_USERNAME = "user3";
     private static final String INITIAL_PASSWORD = "password";
     private static final String CHANGED_PASSWORD = "passwordChanged";
     private static final String ORIGIN = "example.com";
@@ -94,8 +94,8 @@ public void testDialogPropertiesFeatureDisabled() {
                 .showDialog(mModalDialogModel, ModalDialogManager.ModalDialogType.TAB);
         Assert.assertThat("Usernames don't match",
                 mDialogProperties.get(PasswordEditDialogProperties.USERNAMES), contains(USERNAMES));
-        Assert.assertEquals("Selected username doesn't match", INITIAL_USERNAME_INDEX,
-                mDialogProperties.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX));
+        Assert.assertEquals("Selected username doesn't match", INITIAL_USERNAME,
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME));
         Assert.assertEquals("Password doesn't match", INITIAL_PASSWORD,
                 mDialogProperties.get(PasswordEditDialogProperties.PASSWORD));
         Assert.assertNull(
@@ -108,15 +108,15 @@ public void testDialogPropertiesFeatureDisabled() {
     @Test
     @DisableFeatures(ChromeFeatureList.PASSWORD_EDIT_DIALOG_WITH_DETAILS)
     public void testUserSelection() {
-        Callback<Integer> usernameSelectedCallback =
-                mDialogProperties.get(PasswordEditDialogProperties.USERNAME_SELECTED_CALLBACK);
-        usernameSelectedCallback.onResult(SELECTED_USERNAME_INDEX);
-        Assert.assertEquals("Selected username doesn't match", SELECTED_USERNAME_INDEX,
-                mDialogProperties.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX));
+        Callback<String> usernameSelectedCallback =
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME_CHANGED_CALLBACK);
+        usernameSelectedCallback.onResult(CHANGED_USERNAME);
+        Assert.assertEquals("Selected username doesn't match", CHANGED_USERNAME,
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME));
         ModalDialogProperties.Controller dialogController =
                 mModalDialogModel.get(ModalDialogProperties.CONTROLLER);
         dialogController.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.POSITIVE);
-        Mockito.verify(mDelegateMock).onDialogAccepted(SELECTED_USERNAME_INDEX, INITIAL_PASSWORD);
+        Mockito.verify(mDelegateMock).onDialogAccepted(CHANGED_USERNAME, INITIAL_PASSWORD);
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.POSITIVE_BUTTON_CLICKED);
     }
@@ -129,7 +129,7 @@ public void testUserSelection() {
     @DisableFeatures(ChromeFeatureList.PASSWORD_EDIT_DIALOG_WITH_DETAILS)
     public void testDialogDismissedFromNative() {
         mDialogCoordinator.dismiss();
-        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyInt(), anyString());
+        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyString(), anyString());
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.DISMISSED_BY_NATIVE);
     }
@@ -144,7 +144,7 @@ public void testDialogDismissedWithNegativeButton() {
         ModalDialogProperties.Controller dialogController =
                 mModalDialogModel.get(ModalDialogProperties.CONTROLLER);
         dialogController.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.NEGATIVE);
-        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyInt(), anyString());
+        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyString(), anyString());
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
     }
@@ -160,8 +160,8 @@ public void testDialogPropertiesFeatureEnabled() {
                 .showDialog(mModalDialogModel, ModalDialogManager.ModalDialogType.TAB);
         Assert.assertThat("Usernames don't match",
                 mDialogProperties.get(PasswordEditDialogProperties.USERNAMES), contains(USERNAMES));
-        Assert.assertEquals("Selected username doesn't match", INITIAL_USERNAME_INDEX,
-                mDialogProperties.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX));
+        Assert.assertEquals("Selected username doesn't match", INITIAL_USERNAME,
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME));
         Assert.assertEquals("Password doesn't match", INITIAL_PASSWORD,
                 mDialogProperties.get(PasswordEditDialogProperties.PASSWORD));
         Assert.assertNotNull(
@@ -187,7 +187,7 @@ public void testPasswordChanging() {
         ModalDialogProperties.Controller dialogController =
                 mModalDialogModel.get(ModalDialogProperties.CONTROLLER);
         dialogController.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.POSITIVE);
-        Mockito.verify(mDelegateMock).onDialogAccepted(INITIAL_USERNAME_INDEX, CHANGED_PASSWORD);
+        Mockito.verify(mDelegateMock).onDialogAccepted(INITIAL_USERNAME, CHANGED_PASSWORD);
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.POSITIVE_BUTTON_CLICKED);
     }
@@ -196,15 +196,15 @@ public void testPasswordChanging() {
     @Test
     @EnableFeatures(ChromeFeatureList.PASSWORD_EDIT_DIALOG_WITH_DETAILS)
     public void testUserSelectionFeatureEnabled() {
-        Callback<Integer> usernameSelectedCallback =
-                mDialogProperties.get(PasswordEditDialogProperties.USERNAME_SELECTED_CALLBACK);
-        usernameSelectedCallback.onResult(SELECTED_USERNAME_INDEX);
-        Assert.assertEquals("Selected username doesn't match", SELECTED_USERNAME_INDEX,
-                mDialogProperties.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX));
+        Callback<String> usernameSelectedCallback =
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME_CHANGED_CALLBACK);
+        usernameSelectedCallback.onResult(CHANGED_USERNAME);
+        Assert.assertEquals("Selected username doesn't match", CHANGED_USERNAME,
+                mDialogProperties.get(PasswordEditDialogProperties.USERNAME));
         ModalDialogProperties.Controller dialogController =
                 mModalDialogModel.get(ModalDialogProperties.CONTROLLER);
         dialogController.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.POSITIVE);
-        Mockito.verify(mDelegateMock).onDialogAccepted(SELECTED_USERNAME_INDEX, INITIAL_PASSWORD);
+        Mockito.verify(mDelegateMock).onDialogAccepted(CHANGED_USERNAME, INITIAL_PASSWORD);
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.POSITIVE_BUTTON_CLICKED);
     }
@@ -217,7 +217,7 @@ public void testUserSelectionFeatureEnabled() {
     @EnableFeatures(ChromeFeatureList.PASSWORD_EDIT_DIALOG_WITH_DETAILS)
     public void testDialogDismissedFromNativeFeatureEnabled() {
         mDialogCoordinator.dismiss();
-        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyInt(), anyString());
+        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyString(), anyString());
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.DISMISSED_BY_NATIVE);
     }
@@ -232,7 +232,7 @@ public void testDialogDismissedWithNegativeButtonFeatureEnabled() {
         ModalDialogProperties.Controller dialogController =
                 mModalDialogModel.get(ModalDialogProperties.CONTROLLER);
         dialogController.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.NEGATIVE);
-        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyInt(), anyString());
+        Mockito.verify(mDelegateMock, never()).onDialogAccepted(anyString(), anyString());
         Mockito.verify(mModalDialogManagerMock)
                 .dismissDialog(mModalDialogModel, DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
     }

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogView.java

@@ -45,16 +45,16 @@ void setFooter(String footer) {
      * Sets list of known usernames which can be selected from the list by user
      *
      * @param usernames Known usernames list
-     * @param selectedUsernameIndex Initially selected username
+     * @param initialUsername Initially typed username
      */
-    abstract void setUsernames(List<String> usernames, int selectedUsernameIndex);
+    abstract void setUsernames(List<String> usernames, String initialUsername);
 
     /**
-     * Sets callback for handling username selection change
+     * Sets callback for handling username change
      *
-     * @param callback The callback to be called with the index of the selected username
+     * @param callback The callback to be called with new user typed username
      */
-    abstract void setUsernameSelectedCallback(Callback<Integer> callback);
+    abstract void setUsernameChangedCallback(Callback<String> callback);
 
     /**
      * Sets initial password

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogViewBinder.java

@@ -16,20 +16,20 @@ static void bind(
             PropertyModel model, PasswordEditDialogView dialogView, PropertyKey propertyKey) {
         if (propertyKey == PasswordEditDialogProperties.USERNAMES) {
             dialogView.setUsernames(model.get(PasswordEditDialogProperties.USERNAMES),
-                    model.get(PasswordEditDialogProperties.SELECTED_USERNAME_INDEX));
-        } else if (propertyKey == PasswordEditDialogProperties.SELECTED_USERNAME_INDEX) {
+                    model.get(PasswordEditDialogProperties.USERNAME));
+        } else if (propertyKey == PasswordEditDialogProperties.USERNAME) {
             // Propagation of USERNAMES property triggers passing both USERNAMES and
-            // SELECTED_USERNAME_INDEX properties to the view. This is safe because both properties
+            // USERNAME properties to the view. This is safe because both properties
             // are set through property model builder and available by the time the property model
-            // is bound to the view. The SELECTED_USERNAME_INDEX property is writable since it
-            // maintains username index of the user, currently selected in UI. Updating the property
+            // is bound to the view. The USERNAME property is writable since it
+            // maintains username, currently typed or selected in UI. Updating the property
             // by itself doesn't get propagated to the view as the value originates in the view and
-            // gets routed to coordinator through USERNAME_SELECTED_CALLBACK.
+            // gets routed to coordinator through USERNAME_CHANGED_CALLBACK.
         } else if (propertyKey == PasswordEditDialogProperties.FOOTER) {
             dialogView.setFooter(model.get(PasswordEditDialogProperties.FOOTER));
-        } else if (propertyKey == PasswordEditDialogProperties.USERNAME_SELECTED_CALLBACK) {
-            dialogView.setUsernameSelectedCallback(
-                    model.get(PasswordEditDialogProperties.USERNAME_SELECTED_CALLBACK));
+        } else if (propertyKey == PasswordEditDialogProperties.USERNAME_CHANGED_CALLBACK) {
+            dialogView.setUsernameChangedCallback(
+                    model.get(PasswordEditDialogProperties.USERNAME_CHANGED_CALLBACK));
         } else if (propertyKey == PasswordEditDialogProperties.PASSWORD_CHANGED_CALLBACK) {
             dialogView.setPasswordChangedCallback(
                     model.get(PasswordEditDialogProperties.PASSWORD_CHANGED_CALLBACK));

chrome/browser/password_edit_dialog/android/java/src/org/chromium/chrome/browser/password_edit_dialog/PasswordEditDialogWithDetailsView.java

@@ -27,6 +27,7 @@
 public class PasswordEditDialogWithDetailsView extends PasswordEditDialogView {
     private AutoCompleteTextView mUsernameView;
     private TextInputEditText mPasswordView;
+    private Callback<String> mUsernameChangedCallback;
     private Callback<String> mPasswordChangedCallback;
 
     public PasswordEditDialogWithDetailsView(Context context, AttributeSet attrs) {
@@ -40,6 +41,19 @@ public PasswordEditDialogWithDetailsView(Context context, AttributeSet attrs) {
     protected void onFinishInflate() {
         super.onFinishInflate();
         mUsernameView = findViewById(R.id.username_view);
+        mUsernameView.addTextChangedListener(new TextWatcher() {
+            @Override
+            public void beforeTextChanged(CharSequence charSequence, int i, int i1, int i2) {}
+
+            @Override
+            public void onTextChanged(CharSequence charSequence, int i, int i1, int i2) {
+                if (mUsernameChangedCallback == null) return;
+                mUsernameChangedCallback.onResult(charSequence.toString());
+            }
+
+            @Override
+            public void afterTextChanged(Editable editable) {}
+        });
         TextInputLayout usernameInput = findViewById(R.id.username_input_layout);
         usernameInput.setEndIconOnClickListener(view -> mUsernameView.showDropDown());
 
@@ -62,18 +76,17 @@ public void afterTextChanged(Editable editable) {}
     }
 
     @Override
-    public void setUsernames(List<String> usernames, int selectedUsernameIndex) {
+    public void setUsernames(List<String> usernames, String initialUsername) {
         ArrayAdapter<String> usernamesAdapter = new NoFilterArrayAdapter<>(
                 getContext(), android.R.layout.simple_spinner_item, usernames);
         usernamesAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
         mUsernameView.setAdapter(usernamesAdapter);
-        mUsernameView.setText(usernames.get(selectedUsernameIndex));
+        mUsernameView.setText(initialUsername);
     }
 
     @Override
-    public void setUsernameSelectedCallback(Callback<Integer> callback) {
-        // TODO(crbug.com/1315916): Transform this method into
-        // setUsername(Callback<String> callback) format and implement
+    public void setUsernameChangedCallback(Callback<String> callback) {
+        mUsernameChangedCallback = callback;
     }
 
     @Override