Commit e908633d authored by Eleonora Rocchi's avatar Eleonora Rocchi Committed by Commit Bot

[PwdCheckAndroid] Add mediator logic to handle the check status update.

This CL adds to the mediator the logic to handle updates of the status
needed for the UI of the status header. Two new properties are added to
the model:
- Timestamp of the last check completed
- Progress of the current check (if running)
It also adds tests to check that the model is being correctly updated.

Bug: 1109691, 1092444
Change-Id: I07daefc8df1dff609a86505618483f4eefa2646a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2320835
Commit-Queue: Eleonora Rocchi <erocchi@google.com>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795871}
parent 16c01bbd
......@@ -6,10 +6,15 @@ 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.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;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.UNKNOWN_PROGRESS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import android.util.Pair;
import org.chromium.base.Consumer;
import org.chromium.ui.modelutil.ListModel;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
......@@ -40,7 +45,9 @@ class PasswordCheckMediator
items.add(new ListItem(PasswordCheckProperties.ItemType.HEADER,
new PropertyModel.Builder(PasswordCheckProperties.HeaderProperties.ALL_KEYS)
.with(CHECK_PROGRESS, UNKNOWN_PROGRESS)
.with(CHECK_STATUS, PasswordCheckUIStatus.RUNNING)
.with(CHECK_TIMESTAMP, null)
.with(COMPROMISED_CREDENTIALS_COUNT, null)
.build()));
getPasswordCheck().addObserver(this, true);
......@@ -80,9 +87,30 @@ class PasswordCheckMediator
PropertyModel header = items.get(0).model;
header.set(CHECK_STATUS, status);
// TODO(crbug.com/1109691): Retrieve the number of compromised credentials from the
// bridge.
header.set(COMPROMISED_CREDENTIALS_COUNT, 0);
header.set(
CHECK_PROGRESS, status == PasswordCheckUIStatus.RUNNING ? UNKNOWN_PROGRESS : null);
Long checkTimestamp = null;
Integer compromisedCredentialCount = null;
if (status == PasswordCheckUIStatus.IDLE) {
compromisedCredentialCount = getPasswordCheck().getCompromisedCredentialsCount();
// TODO(crbug.com/1109691): Retrieve the timestamp of the last check from the bridge.
checkTimestamp = 0L;
}
header.set(CHECK_TIMESTAMP, checkTimestamp);
header.set(COMPROMISED_CREDENTIALS_COUNT, compromisedCredentialCount);
}
void onPasswordCheckProgressChanged(Pair<Integer, Integer> progress) {
ListModel<ListItem> items = mModel.get(ITEMS);
assert items.size() >= 1;
assert progress.first >= 0;
assert progress.second >= progress.first;
PropertyModel header = items.get(0).model;
header.set(CHECK_STATUS, PasswordCheckUIStatus.RUNNING);
header.set(CHECK_PROGRESS, progress);
header.set(CHECK_TIMESTAMP, null);
header.set(COMPROMISED_CREDENTIALS_COUNT, null);
}
@Override
......
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.password_check;
import android.util.Pair;
import androidx.annotation.IntDef;
import org.chromium.ui.modelutil.ListModel;
......@@ -47,13 +49,21 @@ class PasswordCheckProperties {
* Properties defining the header (banner logo and status line).
*/
static class HeaderProperties {
static final PropertyModel
.WritableObjectPropertyKey<Pair<Integer, Integer>> CHECK_PROGRESS =
new PropertyModel.WritableObjectPropertyKey<>("check_progress");
static final PropertyModel.WritableIntPropertyKey CHECK_STATUS =
new PropertyModel.WritableIntPropertyKey("check_status");
static final PropertyModel.WritableObjectPropertyKey<Long> CHECK_TIMESTAMP =
new PropertyModel.WritableObjectPropertyKey<>("check_timestamp");
static final PropertyModel
.WritableObjectPropertyKey<Integer> COMPROMISED_CREDENTIALS_COUNT =
new PropertyModel.WritableObjectPropertyKey<>("compromised_credentials_count");
static final PropertyKey[] ALL_KEYS = {CHECK_STATUS, COMPROMISED_CREDENTIALS_COUNT};
static final PropertyKey[] ALL_KEYS = {
CHECK_PROGRESS, CHECK_STATUS, CHECK_TIMESTAMP, COMPROMISED_CREDENTIALS_COUNT};
static final Pair<Integer, Integer> UNKNOWN_PROGRESS = new Pair<>(-1, -1);
private HeaderProperties() {}
}
......
......@@ -6,7 +6,9 @@ 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.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;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import static org.chromium.components.embedder_support.util.UrlUtilities.stripScheme;
......@@ -157,12 +159,18 @@ class PasswordCheckViewBinder {
@PasswordCheckUIStatus
int status = model.get(CHECK_STATUS);
Integer compromisedCredentialsCount = model.get(COMPROMISED_CREDENTIALS_COUNT);
if (key == CHECK_STATUS) {
if (key == CHECK_PROGRESS) {
// TODO(crbug.com/1109691): Set text based on progress.
} else if (key == CHECK_STATUS) {
// TODO(crbug.com/1109691): Set text and illustration based on status.
updateActionButton(view, status);
updateStatusIcon(view, status, compromisedCredentialsCount);
} else if (key == COMPROMISED_CREDENTIALS_COUNT) {
// TODO(crbug.com/1109691): Set text and illustration based on compromised credential
// count.
updateStatusIcon(view, status, compromisedCredentialsCount);
} else if (key == CHECK_TIMESTAMP) {
// TODO(crbug.com/1109691): Set text description based on timestamp.
} else {
assert false : "Unhandled update to property:" + key;
}
......@@ -177,7 +185,7 @@ class PasswordCheckViewBinder {
restartButton.setClickable(true);
restartButton.setOnClickListener(unusedView
-> {
// TODO(crbug.com/1092444): Add call to restart the check.
// TODO(crbug.com/1109691): Add call to restart the check.
});
} else {
restartButton.setVisibility(View.GONE);
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.password_check;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
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.verify;
......@@ -14,9 +15,17 @@ 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.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;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
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;
import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.IDLE;
import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.RUNNING;
import android.util.Pair;
import org.junit.Before;
import org.junit.Rule;
......@@ -51,6 +60,8 @@ public class PasswordCheckControllerTest {
@Rule
public TestRule mFeaturesProcessorRule = new Features.JUnitProcessor();
private static final Pair<Integer, Integer> PROGRESS_UPDATE = new Pair<>(2, 19);
@Mock
private PasswordCheckComponentUi.Delegate mDelegate;
@Mock
......@@ -86,6 +97,11 @@ public class PasswordCheckControllerTest {
verify(mPasswordCheck).removeObserver(mMediator);
}
@Test
public void testInitializeRunningHeader() {
assertRunningHeader(mModel.get(ITEMS).get(0), UNKNOWN_PROGRESS);
}
@Test
public void testCreatesHeaderForStatus() {
mMediator.onPasswordCheckStatusChanged(IDLE);
......@@ -94,6 +110,35 @@ public class PasswordCheckControllerTest {
assertThat(itemList.get(0).model.get(CHECK_STATUS), is(IDLE));
}
@Test
public void testUpdateStatusHeaderOnError() {
assertRunningHeader(mModel.get(ITEMS).get(0), UNKNOWN_PROGRESS);
mMediator.onPasswordCheckStatusChanged(ERROR_OFFLINE);
ListModel<MVCListAdapter.ListItem> itemList = mModel.get(ITEMS);
assertThat(itemList.size(), is(1));
MVCListAdapter.ListItem header = itemList.get(0);
assertHeaderTypeWithStatus(header, ERROR_OFFLINE);
assertNull(header.model.get(CHECK_PROGRESS));
assertNull(header.model.get(CHECK_TIMESTAMP));
assertNull(header.model.get(COMPROMISED_CREDENTIALS_COUNT));
}
@Test
public void testUpdateStatusHeaderOnIdle() {
assertRunningHeader(mModel.get(ITEMS).get(0), UNKNOWN_PROGRESS);
mMediator.onPasswordCheckStatusChanged(IDLE);
ListModel<MVCListAdapter.ListItem> itemList = mModel.get(ITEMS);
assertThat(itemList.size(), is(1));
assertIdleHeader(itemList.get(0));
}
@Test
public void testUpdateProgressHeader() {
assertRunningHeader(mModel.get(ITEMS).get(0), UNKNOWN_PROGRESS);
mMediator.onPasswordCheckProgressChanged(PROGRESS_UPDATE);
assertRunningHeader(mModel.get(ITEMS).get(0), PROGRESS_UPDATE);
}
@Test
public void testCreatesEntryForExistingCredentials() {
when(mPasswordCheck.getCompromisedCredentials())
......@@ -140,4 +185,25 @@ public class PasswordCheckControllerTest {
mMediator.onChangePasswordWithScriptButtonClick(BOB);
verify(mLaunchCctWithScriptConsumer).accept(eq(BOB));
}
private void assertIdleHeader(MVCListAdapter.ListItem header) {
assertHeaderTypeWithStatus(header, IDLE);
assertNull(header.model.get(CHECK_PROGRESS));
assertNotNull(header.model.get(CHECK_TIMESTAMP));
assertNotNull(header.model.get(COMPROMISED_CREDENTIALS_COUNT));
}
private void assertRunningHeader(
MVCListAdapter.ListItem header, Pair<Integer, Integer> progress) {
assertHeaderTypeWithStatus(header, RUNNING);
assertThat(header.model.get(CHECK_PROGRESS), is(progress));
assertNull(header.model.get(CHECK_TIMESTAMP));
assertNull(header.model.get(COMPROMISED_CREDENTIALS_COUNT));
}
private void assertHeaderTypeWithStatus(
MVCListAdapter.ListItem header, @PasswordCheckUIStatus int status) {
assertThat(header.type, is(ItemType.HEADER));
assertThat(header.model.get(CHECK_STATUS), is(status));
}
}
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