[PwdCheckAndroid] Check is started automatically and on restart

With this CL, the check is automatically started if a user clicks the
restart button or navigates to the password check page from a
non-safety check origin.

The check is run to completion unless all observers unsubscribe before
the end of the check (e.g. because their surface is closed).

Promoted bugs:
 * List receives full update for each new credential
 * The "last run" date range does not seem sensible

Bug: 1109691, 1092444
Change-Id: Icfb25b91508d36b938e56cea2516922089963de2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346387
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796777}

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

@@ -90,6 +90,12 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi, LifecycleObs
         }
     }
 
+    @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
+    public void stopCheck() {
+        PasswordCheck check = PasswordCheckFactory.getPasswordCheckInstance();
+        if (check != null) check.stopCheck();
+    }
+
     // TODO(crbug.com/1101256): Move to view code.
     @Override
     public boolean handleHelp(MenuItem item) {

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

@@ -36,6 +36,7 @@ class PasswordCheckImpl implements PasswordCheck, PasswordCheckObserver {
         fragmentArgs.putInt(
                 PasswordCheckFragmentView.PASSWORD_CHECK_REFERRER, passwordCheckReferrer);
         launcher.launchSettingsActivity(context, PasswordCheckFragmentView.class, fragmentArgs);
+        if (passwordCheckReferrer != PasswordCheckReferrer.SAFETY_CHECK) startCheck();
     }
 
     @Override

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

@@ -12,6 +12,7 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckProperties
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.RESTART_BUTTON_ACTION;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
 
@@ -54,6 +55,7 @@ class PasswordCheckMediator
                         .with(CHECK_STATUS, PasswordCheckUIStatus.RUNNING)
                         .with(CHECK_TIMESTAMP, null)
                         .with(COMPROMISED_CREDENTIALS_COUNT, null)
+                        .with(RESTART_BUTTON_ACTION, this::runCheck)
                         .build()));
         getPasswordCheck().addObserver(this, true);
     }
@@ -71,6 +73,7 @@ class PasswordCheckMediator
             items.add(new ListItem(PasswordCheckProperties.ItemType.HEADER,
                     new PropertyModel.Builder(PasswordCheckProperties.HeaderProperties.ALL_KEYS)
                             .with(CHECK_STATUS, PasswordCheckUIStatus.RUNNING)
+                            .with(RESTART_BUTTON_ACTION, this::runCheck)
                             .build()));
         }
 
@@ -179,6 +182,10 @@ class PasswordCheckMediator
         mLaunchCctWithScript.accept(credential);
     }
 
+    private void runCheck() {
+        getPasswordCheck().startCheck();
+    }
+
     private PasswordCheck getPasswordCheck() {
         PasswordCheck passwordCheck = PasswordCheckFactory.getOrCreate();
         assert passwordCheck != null : "Password Check UI component needs native counterpart!";

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

@@ -64,9 +64,11 @@ class PasswordCheckProperties {
         static final PropertyModel
                 .WritableObjectPropertyKey<Integer> COMPROMISED_CREDENTIALS_COUNT =
                 new PropertyModel.WritableObjectPropertyKey<>("compromised_credentials_count");
+        static final PropertyModel.ReadableObjectPropertyKey<Runnable> RESTART_BUTTON_ACTION =
+                new PropertyModel.WritableObjectPropertyKey<>("restart_button_action");
 
-        static final PropertyKey[] ALL_KEYS = {
-                CHECK_PROGRESS, CHECK_STATUS, CHECK_TIMESTAMP, COMPROMISED_CREDENTIALS_COUNT};
+        static final PropertyKey[] ALL_KEYS = {CHECK_PROGRESS, CHECK_STATUS, CHECK_TIMESTAMP,
+                COMPROMISED_CREDENTIALS_COUNT, RESTART_BUTTON_ACTION};
 
         static final Pair<Integer, Integer> UNKNOWN_PROGRESS = new Pair<>(-1, -1);
 

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

@@ -12,6 +12,7 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckProperties
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.RESTART_BUTTON_ACTION;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
 import static org.chromium.components.embedder_support.util.UrlUtilities.stripScheme;
@@ -178,7 +179,7 @@ class PasswordCheckViewBinder {
         if (key == CHECK_PROGRESS) {
             updateStatusText(view, status, compromisedCredentialsCount, checkTimestamp, progress);
         } else if (key == CHECK_STATUS) {
-            updateActionButton(view, status);
+            updateActionButton(view, status, model.get(RESTART_BUTTON_ACTION));
             updateStatusIcon(view, status, compromisedCredentialsCount);
             updateStatusIllustration(view, status, compromisedCredentialsCount);
             updateStatusText(view, status, compromisedCredentialsCount, checkTimestamp, progress);
@@ -190,6 +191,8 @@ class PasswordCheckViewBinder {
             updateStatusIllustration(view, status, compromisedCredentialsCount);
             updateStatusText(view, status, compromisedCredentialsCount, checkTimestamp, progress);
             updateStatusSubtitle(view, status, compromisedCredentialsCount);
+        } else if (key == RESTART_BUTTON_ACTION) {
+            assert model.get(RESTART_BUTTON_ACTION) != null : "Restart action is always required.";
         } else {
             assert false : "Unhandled update to property:" + key;
         }
@@ -197,15 +200,13 @@ class PasswordCheckViewBinder {
 
     private PasswordCheckViewBinder() {}
 
-    private static void updateActionButton(View view, @PasswordCheckUIStatus int status) {
+    private static void updateActionButton(
+            View view, @PasswordCheckUIStatus int status, Runnable startCheck) {
         ImageButton restartButton = view.findViewById(R.id.check_status_restart_button);
         if (status != PasswordCheckUIStatus.RUNNING) {
             restartButton.setVisibility(View.VISIBLE);
             restartButton.setClickable(true);
-            restartButton.setOnClickListener(unusedView
-                    -> {
-                            // TODO(crbug.com/1109691): Add call to restart the check.
-                    });
+            restartButton.setOnClickListener(unusedView -> startCheck.run());
         } else {
             restartButton.setVisibility(View.GONE);
             restartButton.setClickable(false);

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

@@ -29,6 +29,7 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckProperties
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.RESTART_BUTTON_ACTION;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.ERROR_NO_PASSWORDS;
@@ -112,6 +113,8 @@ public class PasswordCheckViewTest {
     private PasswordCheckComponentUi mComponentUi;
     @Mock
     private PasswordCheckCoordinator.CredentialEventHandler mMockHandler;
+    @Mock
+    private Runnable mMockStartButtonCallback;
 
     @Rule
     public SettingsActivityTestRule<PasswordCheckFragmentView> mTestRule =
@@ -219,13 +222,15 @@ public class PasswordCheckViewTest {
 
     @Test
     @MediumTest
-    public void testStatusDisplaysRestartAction() {
+    public void testStatusDisplaysClickableRestartAction() {
         Long checkTimestamp = System.currentTimeMillis();
         runOnUiThreadBlocking(
                 () -> { mModel.get(ITEMS).add(buildHeader(IDLE, 0, checkTimestamp)); });
         waitForListViewToHaveLength(1);
         assertThat(getActionButton().getVisibility(), is(View.VISIBLE));
         assertTrue(getActionButton().isClickable());
+        getActionButton().callOnClick();
+        waitForEvent(mMockStartButtonCallback).run();
     }
 
     @Test
@@ -347,7 +352,7 @@ public class PasswordCheckViewTest {
 
     @Test
     @MediumTest
-    public void testStatusDysplaysSubtitleOnIdleNoLeaks() {
+    public void testStatusDisplaysSubtitleOnIdleNoLeaks() {
         Long checkTimestamp = System.currentTimeMillis();
         runOnUiThreadBlocking(
                 () -> { mModel.get(ITEMS).add(buildHeader(IDLE, 0, checkTimestamp)); });
@@ -359,7 +364,7 @@ public class PasswordCheckViewTest {
 
     @Test
     @MediumTest
-    public void testStatusDysplaysSubtitleOnIdleWithLeaks() {
+    public void testStatusDisplaysSubtitleOnIdleWithLeaks() {
         Long checkTimestamp = System.currentTimeMillis();
         runOnUiThreadBlocking(
                 () -> { mModel.get(ITEMS).add(buildHeader(IDLE, LEAKS_COUNT, checkTimestamp)); });
@@ -372,7 +377,7 @@ public class PasswordCheckViewTest {
 
     @Test
     @MediumTest
-    public void testStatusNotDysplaysSubtitle() {
+    public void testStatusNotDisplaysSubtitle() {
         runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(ERROR_UNKNOWN)); });
         waitForListViewToHaveLength(1);
         assertThat(getHeaderSubtitle().getVisibility(), is(View.GONE));
@@ -538,6 +543,7 @@ public class PasswordCheckViewTest {
                         .with(CHECK_STATUS, status)
                         .with(CHECK_TIMESTAMP, checkTimestamp)
                         .with(COMPROMISED_CREDENTIALS_COUNT, compromisedCredentialsCount)
+                        .with(RESTART_BUTTON_ACTION, mMockStartButtonCallback)
                         .build());
     }
 

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

@@ -21,6 +21,7 @@ import static org.chromium.chrome.browser.password_check.PasswordCheckProperties
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_TIMESTAMP;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
+import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.RESTART_BUTTON_ACTION;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
 import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.ERROR_OFFLINE;
@@ -113,6 +114,7 @@ public class PasswordCheckControllerTest {
         ListModel<MVCListAdapter.ListItem> itemList = mModel.get(ITEMS);
         assertThat(itemList.get(0).type, is(ItemType.HEADER));
         assertThat(itemList.get(0).model.get(CHECK_STATUS), is(IDLE));
+        assertNotNull(itemList.get(0).model.get(RESTART_BUTTON_ACTION));
     }
 
     @Test
@@ -203,6 +205,7 @@ public class PasswordCheckControllerTest {
         assertNull(header.model.get(CHECK_PROGRESS));
         assertNotNull(header.model.get(CHECK_TIMESTAMP));
         assertNotNull(header.model.get(COMPROMISED_CREDENTIALS_COUNT));
+        assertNotNull(header.model.get(RESTART_BUTTON_ACTION));
     }
 
     private void assertRunningHeader(
@@ -211,11 +214,13 @@ public class PasswordCheckControllerTest {
         assertThat(header.model.get(CHECK_PROGRESS), is(progress));
         assertNull(header.model.get(CHECK_TIMESTAMP));
         assertNull(header.model.get(COMPROMISED_CREDENTIALS_COUNT));
+        assertNotNull(header.model.get(RESTART_BUTTON_ACTION));
     }
 
     private void assertHeaderTypeWithStatus(
             MVCListAdapter.ListItem header, @PasswordCheckUIStatus int status) {
         assertThat(header.type, is(ItemType.HEADER));
         assertThat(header.model.get(CHECK_STATUS), is(status));
+        assertNotNull(header.model.get(RESTART_BUTTON_ACTION));
     }
 }

chrome/browser/password_check/android/password_check_manager.cc

@@ -138,7 +138,11 @@ void PasswordCheckManager::OnStateChanged(State state) {
 void PasswordCheckManager::OnCredentialDone(
     const password_manager::LeakCheckCredential& credential,
     password_manager::IsLeaked is_leaked) {
-  // TODO(crbug.com/1102025): implement this.
+  // TODO(crbug.com/1092444): Advance progress.
+  if (is_leaked) {
+    // TODO(crbug.com/1092444): Trigger single-credential update.
+    compromised_credentials_manager_.SaveCompromisedCredential(credential);
+  }
 }
 
 CompromisedCredentialForUI PasswordCheckManager::MakeUICredential(