[PwdCheckAndroid] Add deletion confirmation dialog

With this CL, the deletion button triggers a confirmation dialog.

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

chrome/browser/password_check/android/BUILD.gn

@@ -78,6 +78,7 @@ junit_binary("password_check_junit_tests") {
     "//chrome/browser/password_check/android:password_check_java_enums",
     "//chrome/browser/password_check/android:public_java",
     "//chrome/test/android:chrome_java_test_support",
+    "//third_party/android_deps:androidx_appcompat_appcompat_java",
     "//third_party/hamcrest:hamcrest_java",
     "//third_party/junit",
     "//ui/android:ui_full_java",
@@ -111,6 +112,7 @@ android_library("test_java") {
     "//components/embedder_support/android:util_java",
     "//content/public/test/android:content_java_test_support",
     "//third_party/android_deps:androidx_annotation_annotation_java",
+    "//third_party/android_deps:androidx_appcompat_appcompat_java",
     "//third_party/android_deps:androidx_appcompat_appcompat_resources_java",
     "//third_party/android_deps:androidx_fragment_fragment_java",
     "//third_party/android_deps:androidx_recyclerview_recyclerview_java",

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

@@ -66,6 +66,7 @@ android_library("internal_java") {
     "//components/browser_ui/widget/android:java",
     "//components/embedder_support/android:util_java",
     "//third_party/android_deps:androidx_annotation_annotation_java",
+    "//third_party/android_deps:androidx_appcompat_appcompat_java",
     "//third_party/android_deps:androidx_fragment_fragment_java",
     "//third_party/android_deps:androidx_lifecycle_lifecycle_common_java",
     "//third_party/android_deps:androidx_preference_preference_java",
@@ -77,6 +78,7 @@ android_library("internal_java") {
   sources = [
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckBridge.java",
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckCoordinator.java",
+    "java/src/org/chromium/chrome/browser/password_check/PasswordCheckDeletionDialogFragment.java",
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckImpl.java",
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckMediator.java",
     "java/src/org/chromium/chrome/browser/password_check/PasswordCheckProperties.java",

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.password_check;
+
+import android.app.Dialog;
+import android.content.DialogInterface;
+import android.os.Bundle;
+
+import androidx.appcompat.app.AlertDialog;
+import androidx.fragment.app.DialogFragment;
+
+import org.chromium.chrome.browser.password_check.internal.R;
+
+/**
+ * Shows the dialog that confirms the user really wants to delete a credential.
+ */
+public class PasswordCheckDeletionDialogFragment extends DialogFragment {
+    /**
+     * This interface combines handling the clicks on the buttons and the general dismissal of the
+     * dialog.
+     */
+    interface Handler extends DialogInterface.OnClickListener {
+        /** Handle the dismissal of the dialog.*/
+        void onDismiss();
+    }
+
+    // This handler is used to answer the user actions on the dialog.
+    private final Handler mHandler;
+    private final String mOrigin;
+
+    PasswordCheckDeletionDialogFragment(Handler handler, String origin) {
+        mHandler = handler;
+        mOrigin = origin;
+    }
+
+    /**
+     * Opens the dialog with the confirmation and sets the button listener to a fragment identified
+     * by ID passed in arguments.
+     */
+    @Override
+    public Dialog onCreateDialog(Bundle savedInstanceState) {
+        return new AlertDialog
+                .Builder(getActivity(), R.style.Theme_Chromium_AlertDialog_NoActionBar)
+                .setTitle(R.string.password_check_delete_credential_dialog_title)
+                .setPositiveButton(
+                        R.string.password_check_delete_credential_dialog_confirm, mHandler)
+                .setNegativeButton(R.string.password_check_credential_dialog_cancel, mHandler)
+                .setMessage(
+                        getString(R.string.password_check_delete_credential_dialog_body, mOrigin))
+                .create();
+    }
+
+    @Override
+    public void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        if (savedInstanceState != null) {
+            dismiss();
+            return;
+        }
+    }
+
+    @Override
+    public void onDismiss(DialogInterface dialog) {
+        super.onDismiss(dialog);
+        if (mHandler != null) mHandler.onDismiss();
+    }
+}

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

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.password_check;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
@@ -13,8 +15,11 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckProperties
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
 
+import android.content.DialogInterface;
 import android.util.Pair;
 
+import androidx.appcompat.app.AlertDialog;
+
 import org.chromium.base.Consumer;
 import org.chromium.ui.modelutil.ListModel;
 import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
@@ -62,7 +67,12 @@ class PasswordCheckMediator
         CompromisedCredential[] credentials = getPasswordCheck().getCompromisedCredentials();
         assert credentials != null;
         ListModel<ListItem> items = mModel.get(ITEMS);
-        assert items.size() >= 1 : "Needs to initialize list with header before adding items!";
+        if (items.size() == 0) {
+            items.add(new ListItem(PasswordCheckProperties.ItemType.HEADER,
+                    new PropertyModel.Builder(PasswordCheckProperties.HeaderProperties.ALL_KEYS)
+                            .with(CHECK_STATUS, PasswordCheckUIStatus.RUNNING)
+                            .build()));
+        }
 
         for (CompromisedCredential credential : credentials) {
             items.add(new ListItem(credential.hasScript()
@@ -136,7 +146,22 @@ class PasswordCheckMediator
 
     @Override
     public void onRemove(CompromisedCredential credential) {
-        mDelegate.removeCredential(credential);
+        mModel.set(DELETION_ORIGIN, credential.getOriginUrl());
+        mModel.set(
+                DELETION_CONFIRMATION_HANDLER, new PasswordCheckDeletionDialogFragment.Handler() {
+                    @Override
+                    public void onClick(DialogInterface dialog, int which) {
+                        if (which != AlertDialog.BUTTON_POSITIVE) return;
+                        mDelegate.removeCredential(credential);
+                        mModel.set(DELETION_CONFIRMATION_HANDLER, null);
+                        mModel.set(DELETION_ORIGIN, null);
+                    }
+
+                    @Override
+                    public void onDismiss() {
+                        mModel.set(DELETION_CONFIRMATION_HANDLER, null);
+                    }
+                });
     }
 
     @Override

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

@@ -22,8 +22,13 @@ import java.lang.annotation.RetentionPolicy;
 class PasswordCheckProperties {
     static final PropertyModel.ReadableObjectPropertyKey<ListModel<MVCListAdapter.ListItem>> ITEMS =
             new PropertyModel.ReadableObjectPropertyKey<>("items");
+    static final PropertyModel.WritableObjectPropertyKey<
+            PasswordCheckDeletionDialogFragment.Handler> DELETION_CONFIRMATION_HANDLER =
+            new PropertyModel.WritableObjectPropertyKey<>("deletion_confirmation_handler");
+    static final PropertyModel.WritableObjectPropertyKey<String> DELETION_ORIGIN =
+            new PropertyModel.WritableObjectPropertyKey<>("deletion_origin");
 
-    static final PropertyKey[] ALL_KEYS = {ITEMS};
+    static final PropertyKey[] ALL_KEYS = {ITEMS, DELETION_CONFIRMATION_HANDLER, DELETION_ORIGIN};
 
     static PropertyModel createDefaultModel() {
         return new PropertyModel.Builder(ALL_KEYS).with(ITEMS, new ListModel<>()).build();

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

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.password_check;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
@@ -60,6 +62,11 @@ class PasswordCheckViewBinder {
                             PasswordCheckProperties::getItemType,
                             PasswordCheckViewBinder::connectPropertyModel),
                     PasswordCheckViewBinder::createViewHolder));
+        } else if (propertyKey == DELETION_CONFIRMATION_HANDLER) {
+            view.showDialogFragment(new PasswordCheckDeletionDialogFragment(
+                    model.get(DELETION_CONFIRMATION_HANDLER), model.get(DELETION_ORIGIN)));
+        } else if (propertyKey == DELETION_ORIGIN) {
+            // Binding not necessary (only used indirectly).
         } else {
             assert false : "Unhandled update to property:" + propertyKey;
         }

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings.grd

@@ -226,6 +226,20 @@
         Restart password check
       </message>
 
+      <!-- Password Check Dialog strings -->
+      <message name="IDS_PASSWORD_CHECK_CREDENTIAL_DIALOG_CANCEL" desc="The caption the button closing the prompt to delete a stored password.">
+        Cancel
+      </message>
+      <message name="IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_BODY" desc="The body of a prompt to confirm that the user intends to delete the stored password.">
+        Deleting this password will not delete your account on <ph name="SITE">%1$s<ex>example.com</ex></ph>. Change your password or delete your account on <ph name="SITE">%1$s<ex>example.com</ex></ph> to keep it safe from others.
+      </message>
+      <message name="IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_TITLE" desc="The title of a prompt to confirm that the user intends to delete the stored password.">
+        Delete password?
+      </message>
+      <message name="IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_CONFIRM" desc="The caption the button confirming that the user intends to delete the stored password.">
+        Delete password
+      </message>
+
       <!-- Utility string to show the timestamp -->
       <message name="IDS_PASSWORD_CHECK_JUST_NOW" desc="A time label for a check that happened just now. For example 'Checked passwords · Just now'">
         Just now

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings_grd/IDS_PASSWORD_CHECK_CREDENTIAL_DIALOG_CANCEL.png.sha1

+247a40688e167d85b78fc2bfc755ff73bacab2fe
\ No newline at end of file

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings_grd/IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_BODY.png.sha1

+19b07efe24ab4f8af6385168f1eb1a2209b850a6
\ No newline at end of file

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings_grd/IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_CONFIRM.png.sha1

+19b07efe24ab4f8af6385168f1eb1a2209b850a6
\ No newline at end of file

chrome/browser/password_check/android/internal/java/strings/android_password_check_strings_grd/IDS_PASSWORD_CHECK_DELETE_CREDENTIAL_DIALOG_TITLE.png.sha1

+19b07efe24ab4f8af6385168f1eb1a2209b850a6
\ No newline at end of file

chrome/browser/password_check/android/java/src/org/chromium/chrome/browser/password_check/PasswordCheckFragmentView.java

@@ -8,6 +8,7 @@ import android.content.Context;
 import android.os.Bundle;
 import android.view.MenuItem;
 
+import androidx.fragment.app.DialogFragment;
 import androidx.preference.PreferenceFragmentCompat;
 
 /**
@@ -74,4 +75,9 @@ public class PasswordCheckFragmentView extends PreferenceFragmentCompat {
     private Context getStyledContext() {
         return getPreferenceManager().getContext();
     }
+
+    void showDialogFragment(DialogFragment passwordCheckDeletionDialogFragment) {
+        if (passwordCheckDeletionDialogFragment == null) return;
+        passwordCheckDeletionDialogFragment.show(getParentFragmentManager(), null);
+    }
 }

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

@@ -15,12 +15,16 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_ORIGIN;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
@@ -51,6 +55,7 @@ import android.widget.ProgressBar;
 import android.widget.TextView;
 
 import androidx.annotation.IdRes;
+import androidx.appcompat.app.AlertDialog;
 import androidx.appcompat.content.res.AppCompatResources;
 import androidx.recyclerview.widget.RecyclerView;
 import androidx.test.filters.MediumTest;
@@ -445,6 +450,22 @@ public class PasswordCheckViewTest {
         assertThat(PasswordCheckViewBinder.getTimestamp(res, 315 * DAY_TO_MS), is("315 days ago"));
     }
 
+    @Test
+    @MediumTest
+    public void testConfirmingDeletionDialogTriggersHandler() {
+        PasswordCheckDeletionDialogFragment.Handler mockHandler =
+                mock(PasswordCheckDeletionDialogFragment.Handler.class);
+        mModel.set(DELETION_ORIGIN, ANA.getOriginUrl());
+        runOnUiThreadBlocking(() -> mModel.set(DELETION_CONFIRMATION_HANDLER, mockHandler));
+
+        onView(withText(R.string.password_check_delete_credential_dialog_confirm))
+                .inRoot(withDecorView(
+                        not(is(mPasswordCheckView.getActivity().getWindow().getDecorView()))))
+                .perform(click());
+
+        verify(mockHandler).onClick(any(), eq(AlertDialog.BUTTON_POSITIVE));
+    }
+
     private MVCListAdapter.ListItem buildHeader(@PasswordCheckUIStatus int status,
             Integer compromisedCredentialsCount, Long checkTimestamp) {
         return buildHeader(status, compromisedCredentialsCount, checkTimestamp, null);

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

@@ -10,11 +10,13 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.DELETION_CONFIRMATION_HANDLER;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
@@ -25,8 +27,11 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.E
 import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.IDLE;
 import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.RUNNING;
 
+import android.content.DialogInterface;
 import android.util.Pair;
 
+import androidx.appcompat.app.AlertDialog;
+
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -170,8 +175,15 @@ public class PasswordCheckControllerTest {
 
     @Test
     public void testRemovingElementTriggersDelegate() {
+        // Removing sets a valid handler:
         mMediator.onRemove(ANA);
+        assertNotNull(mModel.get(DELETION_CONFIRMATION_HANDLER));
+
+        // When the handler is triggered (because the dialog was confirmed), remove the credential:
+        mModel.get(DELETION_CONFIRMATION_HANDLER)
+                .onClick(mock(DialogInterface.class), AlertDialog.BUTTON_POSITIVE);
         verify(mDelegate).removeCredential(eq(ANA));
+        assertNull(mModel.get(DELETION_CONFIRMATION_HANDLER));
     }
 
     @Test