Commit aa990afd authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[PwdCheckAndroid] Add view binder and RecyclerView for leak check

This CL connects the PasswordCheckFragmentView with the controller code
by introducing a ViewBinder to set properties.
Additionally, a view test covers that model data is rendered as
expected.

Bug: 1101256,1092444
Change-Id: Id3afbb6a559bbaf6c0c8e267ecec322f7a4b75a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279117
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785751}
parent 10be899b
......@@ -2285,6 +2285,7 @@ chrome_test_apk_tmpl("chrome_public_test_apk") {
"//chrome/android/webapk/shell_apk:shell_apk_javatests",
"//chrome/browser/download/android:download_java_tests",
"//chrome/browser/flags:javatests",
"//chrome/browser/password_check/android:test_java",
"//chrome/browser/subresource_filter:subresource_filter_javatests",
"//chrome/browser/touch_to_fill/android:test_java",
"//chrome/browser/ui/android/appmenu/internal:javatests",
......
......@@ -30,6 +30,7 @@ android_library("public_java") {
"//base:base_java",
"//third_party/android_deps:androidx_fragment_fragment_java",
"//third_party/android_deps:androidx_preference_preference_java",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
]
sources = [
"java/src/org/chromium/chrome/browser/password_check/CompromisedCredential.java",
......@@ -59,6 +60,31 @@ junit_binary("password_check_junit_tests") {
]
}
android_library("test_java") {
testonly = true
sources = [ "javatests/src/org/chromium/chrome/browser/password_check/PasswordCheckViewTest.java" ]
deps = [
":public_java",
"//base:base_java",
"//base:base_java_test_support",
"//chrome/android:chrome_java",
"//chrome/android:chrome_test_java",
"//chrome/android:chrome_test_util_java",
"//chrome/browser/password_check/android/internal:internal_java",
"//chrome/browser/settings:test_support_java",
"//chrome/test/android:chrome_java_test_support",
"//components/embedder_support/android:util_java",
"//content/public/test/android:content_java_test_support",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
"//third_party/hamcrest:hamcrest_java",
"//third_party/junit",
"//third_party/mockito:mockito_java",
"//ui/android:ui_full_java",
"//ui/android:ui_utils_java",
]
}
android_resources("java_resources") {
sources = []
deps = [ "//chrome/browser/ui/android/strings:ui_strings_grd" ]
......
......@@ -52,6 +52,16 @@ The public build and run target is `password_check_junit_tests`. Run them with:
Although the entire suite should run in seconds, you can filter tests with `-f`.
#### javatests/
Contains View and Integration tests. These tests need an emulator which means
that they run slowly but can test rendered Views.
The public build and run target is `chrome_public_test_apk`. Run them with:
``` bash
./out/<OutDirectory>/bin/run_chrome_public_test_apk -f *PasswordCheck*
```
## Example usage
``` java
......
......@@ -44,14 +44,17 @@ android_library("internal_ui_factory_java") {
android_library("internal_java") {
deps = [
":java_resources",
"//chrome/android:chrome_app_java_resources",
"//chrome/android:chrome_java",
"//chrome/browser/password_check/android:public_ui_java",
"//chrome/browser/profiles/android:java",
"//chrome/browser/settings:java",
"//chrome/browser/ui/android/strings:ui_strings_grd",
"//components/embedder_support/android:util_java",
"//third_party/android_deps:androidx_fragment_fragment_java",
"//third_party/android_deps:androidx_preference_preference_java",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
"//ui/android:ui_full_java",
_public_target,
]
......@@ -60,5 +63,15 @@ android_library("internal_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",
"java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewBinder.java",
"java/src/org/chromium/chrome/browser/password_check/PasswordCheckViewHolder.java",
]
}
android_resources("java_resources") {
sources = [
"java/res/layout/password_check_compromised_credential_item.xml",
"java/res/layout/password_check_header_item.xml",
]
custom_package = "org.chromium.chrome.browser.password_check.internal"
}
......@@ -34,9 +34,9 @@ requests to show compromosed credentials:
`credentials`.
1. The controller prepares and filters the `credentials` for display.
2. The controller writes the prepared credentials into the [model](#Model)
as `SETTINGS_ITEMS`.
as `ITEMS`.
2. The MCP picks up model changes.
1. The MCP identifies that the `SETTINGS_ITEMS` property was changed.
1. The MCP identifies that the `ITEMS` property was changed.
2. The MCP uses a ViewBinder to bind each settings item to the
corresponding view (e.g. a `COMPROMISED_CREDENTIAL` item id would create
or reuse a TextView with a button of options).
......@@ -56,10 +56,12 @@ static `PasswordCheckProperties` class.
The model contains writable and readable properties. The readable properties are
guaranteed to never change for the lifetime of the Password Check component:
* **SETTINGS_ITEMS** which is the set of displayed credentials. The list
* **ITEMS** which is the set of displayed credentials. The list
itself will be modified (credentials will be added and removed) but the
object remains the same which allows to permanently bind it to a list
adapter.
* **FRAGMET_EVENT_LISTENER** which is the listener that reacts to view events.
The mediator implements this interface.
The writable properties change over the course of the components lifetime:
......@@ -81,4 +83,12 @@ appearance. The controller consists of two parts:
## View
The view contains all parts that are necessary to display the bottom sheet.
The view contains all parts that are necessary to display the check UI. It
consists of two parts:
* **PasswordCheckViewBinder** which maps model changes to the view. For the
Password Check component, the ViewBinder also supports an Adapter that binds
changes to the `ITEMS` property to the RecyclerView inside the
bottom sheet.
* **PasswordCheckSettingsView** which displays all visible elements. Since it
is a fragment that the SettignsLauncher needs to access, it is public.
<?xml version="1.0" encoding="utf-8"?>
<!-- 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. -->
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/credential_origin"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary" />
<?xml version="1.0" encoding="utf-8"?>
<!-- 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. -->
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/status_message"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary" />
......@@ -6,9 +6,12 @@ package org.chromium.chrome.browser.password_check;
import android.view.MenuItem;
import org.chromium.chrome.R;
import androidx.annotation.VisibleForTesting;
import org.chromium.chrome.browser.help.HelpAndFeedback;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
/**
* Creates the PasswordCheckComponentUi. This class is responsible for managing the UI for the check
......@@ -19,17 +22,33 @@ class PasswordCheckCoordinator implements PasswordCheckComponentUi {
PasswordCheckCoordinator(PasswordCheckFragmentView fragmentView) {
mFragmentView = fragmentView;
PropertyModel model = new PropertyModel.Builder(PasswordCheckProperties.ALL_KEYS).build();
PasswordCheckMediator mediator = new PasswordCheckMediator();
mediator.initialize(model);
PasswordCheckCoordinator.setUpModelChangeProcessors(model, mFragmentView);
}
// TODO(crbug.com/1101256): Move to view code.
@Override
public boolean handleHelp(MenuItem item) {
if (item.getItemId() == R.id.menu_id_targeted_help) {
if (item.getItemId() == org.chromium.chrome.R.id.menu_id_targeted_help) {
HelpAndFeedback.getInstance().show(mFragmentView.getActivity(),
mFragmentView.getActivity().getString(R.string.help_context_check_passwords),
mFragmentView.getActivity().getString(
org.chromium.chrome.R.string.help_context_check_passwords),
Profile.getLastUsedRegularProfile(), null);
return true;
}
return false;
}
/**
* Connects the given model with the given view using Model Change Processors.
* @param model A {@link PropertyModel} built with {@link PasswordCheckProperties}.
* @param view A {@link PasswordCheckFragmentView}.
*/
@VisibleForTesting
static void setUpModelChangeProcessors(PropertyModel model, PasswordCheckFragmentView view) {
PropertyModelChangeProcessor.create(
model, view, PasswordCheckViewBinder::bindPasswordCheckView);
}
}
\ No newline at end of file
......@@ -85,5 +85,14 @@ class PasswordCheckProperties {
int ERROR_UNKNOWN = 6;
}
/**
* Returns the sheet item type for a given item.
* @param item An {@link MVCListAdapter.ListItem}.
* @return The {@link ItemType} of the given list item.
*/
static @ItemType int getItemType(MVCListAdapter.ListItem item) {
return item.type;
}
private PasswordCheckProperties() {}
}
// 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 static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import static org.chromium.components.embedder_support.util.UrlUtilities.stripScheme;
import android.view.View;
import android.view.ViewGroup;
import android.widget.TextView;
import org.chromium.chrome.browser.password_check.PasswordCheckProperties.ItemType;
import org.chromium.chrome.browser.password_check.internal.R;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.RecyclerViewAdapter;
import org.chromium.ui.modelutil.SimpleRecyclerViewMcp;
/**
* Provides functions that map {@link PasswordCheckProperties} changes in a {@link PropertyModel} to
* the suitable method in {@link PasswordCheckFragmentView}.
*/
class PasswordCheckViewBinder {
/**
* Called whenever a property in the given model changes. It updates the given view accordingly.
* @param model The observed {@link PropertyModel}. Its data need to be reflected in the view.
* @param view The {@link PasswordCheckFragmentView} to update.
* @param propertyKey The {@link PropertyKey} which changed.
*/
static void bindPasswordCheckView(
PropertyModel model, PasswordCheckFragmentView view, PropertyKey propertyKey) {
if (propertyKey == ITEMS) {
view.getListView().setAdapter(new RecyclerViewAdapter<>(
new SimpleRecyclerViewMcp<>(model.get(ITEMS),
PasswordCheckProperties::getItemType,
PasswordCheckViewBinder::connectPropertyModel),
PasswordCheckViewBinder::createViewHolder));
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
}
/**
* Factory used to create a new View inside the list inside the PasswordCheckFragmentView.
* @param parent The parent {@link ViewGroup} of the new item.
* @param itemType The type of View to create.
*/
private static PasswordCheckViewHolder createViewHolder(
ViewGroup parent, @ItemType int itemType) {
switch (itemType) {
case ItemType.HEADER:
return new PasswordCheckViewHolder(parent, R.layout.password_check_header_item,
PasswordCheckViewBinder::bindHeaderView);
case ItemType.COMPROMISED_CREDENTIAL:
return new PasswordCheckViewHolder(parent,
R.layout.password_check_compromised_credential_item,
PasswordCheckViewBinder::bindCredentialView);
}
assert false : "Cannot create view for ItemType: " + itemType;
return null;
}
/**
* This method creates a model change processor for each recycler view item when it is created.
* @param holder A {@link PasswordCheckViewHolder} holding the view and view binder for the MCP.
* @param item A {@link MVCListAdapter.ListItem} holding the {@link PropertyModel} for the MCP.
*/
private static void connectPropertyModel(
PasswordCheckViewHolder holder, MVCListAdapter.ListItem item) {
holder.setupModelChangeProcessor(item.model);
}
/**
* Called whenever a credential is bound to this view holder. Please note that this method
* might be called on a recycled view with old data, so make sure to always reset unused
* properties to default values.
* @param model The model containing the data for the view
* @param view The view to be bound
* @param propertyKey The key of the property to be bound
*/
private static void bindCredentialView(
PropertyModel model, View view, PropertyKey propertyKey) {
CompromisedCredential credential = model.get(COMPROMISED_CREDENTIAL);
if (propertyKey == COMPROMISED_CREDENTIAL) {
TextView pslOriginText = view.findViewById(R.id.credential_origin);
String formattedOrigin = stripScheme(credential.getOriginUrl());
formattedOrigin =
formattedOrigin.replaceFirst("/$", ""); // Strip possibly trailing slash.
pslOriginText.setText(formattedOrigin);
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
}
/**
* Called whenever a property in the given model changes. It updates the given view accordingly.
* @param model The observed {@link PropertyModel}. Its data need to be reflected in the view.
* @param view The {@link View} of the header to update.
* @param key The {@link PropertyKey} which changed.
*/
private static void bindHeaderView(PropertyModel model, View view, PropertyKey key) {
if (key == CHECK_STATUS) {
// TODO(crbug.com/1101256): Set text and illustration based on status.
} else {
assert false : "Unhandled update to property:" + key;
}
}
private PasswordCheckViewBinder() {}
}
// 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.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.LayoutRes;
import androidx.recyclerview.widget.RecyclerView;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor.ViewBinder;
class PasswordCheckViewHolder extends RecyclerView.ViewHolder {
private final ViewBinder<PropertyModel, View, PropertyKey> mViewBinder;
PasswordCheckViewHolder(ViewGroup parent, @LayoutRes int layout,
ViewBinder<PropertyModel, View, PropertyKey> viewBinder) {
super(LayoutInflater.from(parent.getContext()).inflate(layout, parent, false));
mViewBinder = viewBinder;
}
/**
* Called whenever an item is bound to this view holder. Please note that this method
* might be called on the same list entry repeatedly, so make sure to always set a default
* for unused fields.
* @param model The {@link PropertyModel} whose data needs to be displayed.
*/
void setupModelChangeProcessor(PropertyModel model) {
PropertyModelChangeProcessor.create(model, itemView, mViewBinder, true);
}
}
\ No newline at end of file
......@@ -8,6 +8,7 @@ import android.content.Context;
import android.os.Bundle;
import android.view.MenuItem;
import androidx.annotation.VisibleForTesting;
import androidx.preference.PreferenceFragmentCompat;
/**
......@@ -16,11 +17,25 @@ import androidx.preference.PreferenceFragmentCompat;
public class PasswordCheckFragmentView extends PreferenceFragmentCompat {
private PasswordCheckComponentUi mComponentDelegate;
/**
* The factory used to create components that connect to this fragment and provide data. It
* defaults to the {@link PasswordCheckComponentUiFactory} but can be replaced in tests.
*/
interface ComponentFactory {
/**
* Returns a component that is connected to the given fragment and manipulates its data.
* @param fragmentView The fragment (usually {@code this}).
* @return A non-null {@link PasswordCheckComponentUi}.
*/
PasswordCheckComponentUi create(PreferenceFragmentCompat fragmentView);
}
private static ComponentFactory sComponentFactory = PasswordCheckComponentUiFactory::create;
@Override
public void onCreatePreferences(Bundle savedInstanceState, String rootKey) {
getActivity().setTitle(R.string.passwords_check_title);
setPreferenceScreen(getPreferenceManager().createPreferenceScreen(getStyledContext()));
mComponentDelegate = PasswordCheckComponentUiFactory.create(this);
mComponentDelegate = sComponentFactory.create(this);
}
@Override
......@@ -31,4 +46,9 @@ public class PasswordCheckFragmentView extends PreferenceFragmentCompat {
private Context getStyledContext() {
return getPreferenceManager().getContext();
}
@VisibleForTesting
static void setComponentFactory(ComponentFactory factory) {
sComponentFactory = factory;
}
}
\ No newline at end of file
// 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 static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertThat;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import static org.chromium.components.embedder_support.util.UrlUtilities.stripScheme;
import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread;
import android.widget.TextView;
import androidx.test.filters.MediumTest;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties;
import org.chromium.chrome.browser.settings.SettingsActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyModel;
/**
* View tests for the Password Check component ensure model changes are reflected in the check UI.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class PasswordCheckViewTest {
private static final CompromisedCredential ANA =
new CompromisedCredential("Ana", "https://some-url.com");
private PropertyModel mModel = PasswordCheckProperties.createDefaultModel();
private PasswordCheckFragmentView mPasswordCheckView;
@Mock
private PasswordCheckComponentUi mComponentUi;
@Rule
public SettingsActivityTestRule<PasswordCheckFragmentView> mTestRule =
new SettingsActivityTestRule<>(PasswordCheckFragmentView.class);
@Before
public void setUp() throws InterruptedException {
MockitoAnnotations.initMocks(this);
PasswordCheckFragmentView.setComponentFactory(fragmentView -> {
mPasswordCheckView = (PasswordCheckFragmentView) fragmentView;
return mComponentUi;
});
mTestRule.startSettingsActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
PasswordCheckCoordinator.setUpModelChangeProcessors(mModel, mPasswordCheckView);
});
}
@Test
@MediumTest
public void testDisplaysHeaderAndCredential() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mModel.get(ITEMS).add(
new MVCListAdapter.ListItem(PasswordCheckProperties.ItemType.HEADER,
new PropertyModel.Builder(HeaderProperties.ALL_KEYS)
.with(CHECK_STATUS, PasswordCheckProperties.CheckStatus.SUCCESS)
.build()));
mModel.get(ITEMS).add(buildCredentialItem(ANA));
});
pollUiThread(Criteria.equals(2, mPasswordCheckView.getListView()::getChildCount));
TextView entry = (TextView) mPasswordCheckView.getListView().getChildAt(1);
assertThat(entry.getText(), is(stripScheme(ANA.getOriginUrl())));
}
private static MVCListAdapter.ListItem buildCredentialItem(CompromisedCredential credential) {
return new MVCListAdapter.ListItem(PasswordCheckProperties.ItemType.COMPROMISED_CREDENTIAL,
new PropertyModel
.Builder(PasswordCheckProperties.CompromisedCredentialProperties.ALL_KEYS)
.with(COMPROMISED_CREDENTIAL, credential)
.build());
}
}
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