Commit d88f33a5 authored by Andrey Zaytsev's avatar Andrey Zaytsev Committed by Commit Bot

Safety check on Android: added actions on Safety check element clicks

Tested manually, including updates by hardcoding the Chrome app id values.

Bug: 1070620, 1107387
Change-Id: I0631df830ff20c000c57235eab1dbd300645db24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307330
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791767}
parent cac4ce44
......@@ -281,7 +281,7 @@ public class SettingsActivity extends ChromeBaseAppCompatActivity
framentSettingsLauncher.setSettingsLauncher(mSettingsLauncher);
} else if (fragment instanceof SafetyCheckSettingsFragment) {
SafetyCheckCoordinator.create((SafetyCheckSettingsFragment) fragment,
new SafetyCheckUpdatesDelegateImpl(this));
new SafetyCheckUpdatesDelegateImpl(this), new SettingsLauncherImpl());
} else if (fragment instanceof PasswordCheckFragmentView) {
PasswordCheckComponentUiFactory.create((PasswordCheckFragmentView) fragment);
}
......
......@@ -38,7 +38,9 @@ android_library("java") {
"//base:base_java",
"//base:jni_java",
"//chrome/browser/preferences:java",
"//chrome/browser/settings:java",
"//components/browser_ui/settings/android:java",
"//content/public/android:content_java",
"//third_party/android_deps:android_support_v7_appcompat_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:androidx_fragment_fragment_java",
......@@ -92,6 +94,7 @@ android_library("junit") {
"//base:base_junit_test_support",
"//base/test:test_support_java",
"//chrome/browser/preferences:java",
"//chrome/browser/settings:java",
"//third_party/junit:junit",
"//third_party/mockito:mockito_java",
"//ui/android:ui_full_java",
......
include_rules = [
"+components/safety_check",
"+components/prefs",
"+content/public/android",
]
......@@ -9,6 +9,7 @@ import androidx.lifecycle.DefaultLifecycleObserver;
import androidx.lifecycle.LifecycleOwner;
import androidx.lifecycle.Observer;
import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
......@@ -21,19 +22,20 @@ public class SafetyCheckCoordinator implements DefaultLifecycleObserver {
private SafetyCheckMediator mMediator;
/**
* Creates a new Coordinator instance given a settings fragment and an updates client.
* Creates a new instance given a settings fragment, an updates client, and a settings launcher.
* There is no need to hold on to a reference since the settings fragment's lifecycle is
* observed and a reference is retained there.
* @param settingsFragment An instance of {SafetyCheckSettingsFragment} to observe.
* @param updatesClient An instance implementing the {@SafetyCheckUpdatesDelegate} interface.
* @param settingsLauncher An instance implementing the {@SettingsLauncher} interface.
*/
public static void create(SafetyCheckSettingsFragment settingsFragment,
SafetyCheckUpdatesDelegate updatesClient) {
new SafetyCheckCoordinator(settingsFragment, updatesClient);
SafetyCheckUpdatesDelegate updatesClient, SettingsLauncher settingsLauncher) {
new SafetyCheckCoordinator(settingsFragment, updatesClient, settingsLauncher);
}
private SafetyCheckCoordinator(SafetyCheckSettingsFragment settingsFragment,
SafetyCheckUpdatesDelegate updatesClient) {
SafetyCheckUpdatesDelegate updatesClient, SettingsLauncher settingsLauncher) {
mSettingsFragment = settingsFragment;
mUpdatesClient = updatesClient;
// Create the model and the mediator once the view is created.
......@@ -56,7 +58,8 @@ public class SafetyCheckCoordinator implements DefaultLifecycleObserver {
// The View is available, so now we can create the Model, MCP, and
// Mediator.
PropertyModel model = createModelAndMcp(mSettingsFragment);
mMediator = new SafetyCheckMediator(model, mUpdatesClient);
mMediator = new SafetyCheckMediator(
model, mUpdatesClient, settingsLauncher);
}
}
});
......
......@@ -4,13 +4,18 @@
package org.chromium.chrome.browser.safety_check;
import android.content.Intent;
import android.net.Uri;
import android.os.Handler;
import android.os.SystemClock;
import android.view.View;
import androidx.annotation.VisibleForTesting;
import androidx.preference.Preference;
import org.chromium.base.BuildConfig;
import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.password_check.BulkLeakCheckServiceState;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
......@@ -18,6 +23,8 @@ import org.chromium.chrome.browser.safety_check.SafetyCheckBridge.SafetyCheckCom
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.PasswordsState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.SafeBrowsingState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.UpdatesState;
import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.content_public.common.ContentUrlConstants;
import org.chromium.ui.modelutil.PropertyModel;
import java.lang.ref.WeakReference;
......@@ -58,25 +65,62 @@ class SafetyCheckMediator implements SafetyCheckCommonObserver {
};
/**
* Creates a new instance of the Safety check mediator given a model and an updates client.
* Creates a new instance given a model, an updates client, and a settings launcher.
*
* @param model A model instance.
* @param client An updates client.
* @param settingsLauncher An instance of the {@link SettingsLauncher} implementation.
*/
public SafetyCheckMediator(PropertyModel model, SafetyCheckUpdatesDelegate client) {
this(model, client, null, new Handler());
public SafetyCheckMediator(PropertyModel model, SafetyCheckUpdatesDelegate client,
SettingsLauncher settingsLauncher) {
this(model, client, settingsLauncher, null, new Handler());
// Have to initialize this after the constructor call, since a "this" instance is needed.
mSafetyCheckBridge = new SafetyCheckBridge(SafetyCheckMediator.this);
}
@VisibleForTesting
SafetyCheckMediator(PropertyModel model, SafetyCheckUpdatesDelegate client,
SafetyCheckBridge bridge, Handler handler) {
SettingsLauncher settingsLauncher, SafetyCheckBridge bridge, Handler handler) {
mModel = model;
mUpdatesClient = client;
mSafetyCheckBridge = bridge;
mHandler = handler;
mPreferenceManager = SharedPreferencesManager.getInstance();
// Set the listener for clicking the updates element.
mModel.set(SafetyCheckProperties.UPDATES_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
if (!BuildConfig.IS_CHROME_BRANDED) {
return true;
}
String chromeAppId = ContextUtils.getApplicationContext().getPackageName();
// Open the Play Store page for the installed Chrome channel.
p.getContext().startActivity(new Intent(Intent.ACTION_VIEW,
Uri.parse(ContentUrlConstants.PLAY_STORE_URL_PREFIX + chromeAppId)));
return true;
});
// Set the listener for clicking the Safe Browsing element.
mModel.set(SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
// Open the Sync and Services settings.
// TODO(crbug.com/1070620): replace the hardcoded class name with an import and
// ".class.getName()" once SyncAndServicesSettings is moved out of
// //chrome/android.
p.getContext().startActivity(settingsLauncher.createSettingsActivityIntent(
p.getContext(),
"org.chromium.chrome.browser.sync.settings.SyncAndServicesSettings"));
return true;
});
// Set the listener for clicking the passwords element.
mModel.set(SafetyCheckProperties.PASSWORDS_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
// Open the Passwords settings.
// TODO(crbug.com/1070620): replace the hardcoded class name with an import and
// ".class.getName()" once PasswordSettings is moved out of //chrome/android.
p.getContext().startActivity(settingsLauncher.createSettingsActivityIntent(
p.getContext(),
"org.chromium.chrome.browser.password_manager.settings.PasswordSettings"));
return true;
});
// Set the listener for clicking the Check button.
mModel.set(SafetyCheckProperties.SAFETY_CHECK_BUTTON_CLICK_LISTENER,
(View.OnClickListener) (v) -> performSafetyCheck());
......
......@@ -23,6 +23,14 @@ class SafetyCheckProperties {
static final WritableIntPropertyKey SAFE_BROWSING_STATE = new WritableIntPropertyKey();
/** State of the updates check, one of the {@link UpdatesState} values. */
static final WritableIntPropertyKey UPDATES_STATE = new WritableIntPropertyKey();
/** Listener for the passwords element click events. */
static final WritableObjectPropertyKey PASSWORDS_CLICK_LISTENER =
new WritableObjectPropertyKey();
/** Listener for the Safe Browsing element click events. */
static final WritableObjectPropertyKey SAFE_BROWSING_CLICK_LISTENER =
new WritableObjectPropertyKey();
/** Listener for the updates element click events. */
static final WritableObjectPropertyKey UPDATES_CLICK_LISTENER = new WritableObjectPropertyKey();
/** Listener for Safety check button click events. */
static final WritableObjectPropertyKey SAFETY_CHECK_BUTTON_CLICK_LISTENER =
new WritableObjectPropertyKey();
......@@ -115,7 +123,8 @@ class SafetyCheckProperties {
}
static final PropertyKey[] ALL_KEYS = new PropertyKey[] {PASSWORDS_STATE, SAFE_BROWSING_STATE,
UPDATES_STATE, SAFETY_CHECK_BUTTON_CLICK_LISTENER, LAST_RUN_TIMESTAMP};
UPDATES_STATE, PASSWORDS_CLICK_LISTENER, SAFE_BROWSING_CLICK_LISTENER,
UPDATES_CLICK_LISTENER, SAFETY_CHECK_BUTTON_CLICK_LISTENER, LAST_RUN_TIMESTAMP};
static PropertyModel createSafetyCheckModel() {
return new PropertyModel.Builder(ALL_KEYS)
......
......@@ -8,6 +8,7 @@ import android.content.Context;
import android.view.View;
import androidx.annotation.VisibleForTesting;
import androidx.preference.Preference;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.PasswordsState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.SafeBrowsingState;
......@@ -237,6 +238,18 @@ class SafetyCheckViewBinder {
preference.showStatusIcon(getStatusIconForPasswords(state));
preference.setEnabled(true);
}
} else if (SafetyCheckProperties.PASSWORDS_CLICK_LISTENER == propertyKey) {
fragment.findPreference(PASSWORDS_KEY)
.setOnPreferenceClickListener((Preference.OnPreferenceClickListener) model.get(
SafetyCheckProperties.PASSWORDS_CLICK_LISTENER));
} else if (SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER == propertyKey) {
fragment.findPreference(SAFE_BROWSING_KEY)
.setOnPreferenceClickListener((Preference.OnPreferenceClickListener) model.get(
SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER));
} else if (SafetyCheckProperties.UPDATES_CLICK_LISTENER == propertyKey) {
fragment.findPreference(UPDATES_KEY)
.setOnPreferenceClickListener((Preference.OnPreferenceClickListener) model.get(
SafetyCheckProperties.UPDATES_CLICK_LISTENER));
} else if (SafetyCheckProperties.SAFETY_CHECK_BUTTON_CLICK_LISTENER == propertyKey) {
fragment.getCheckButton().setOnClickListener((View.OnClickListener) model.get(
SafetyCheckProperties.SAFETY_CHECK_BUTTON_CLICK_LISTENER));
......
......@@ -24,6 +24,7 @@ import org.chromium.chrome.browser.password_check.BulkLeakCheckServiceState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.PasswordsState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.SafeBrowsingState;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.UpdatesState;
import org.chromium.chrome.browser.settings.SettingsLauncher;
import org.chromium.ui.modelutil.PropertyModel;
import java.lang.ref.WeakReference;
......@@ -35,6 +36,8 @@ public class SafetyCheckMediatorTest {
@Mock
private SafetyCheckUpdatesDelegate mUpdatesDelegate;
@Mock
private SettingsLauncher mSettingsLauncher;
@Mock
private SafetyCheckBridge mBridge;
@Mock
private Handler mHandler;
......@@ -45,7 +48,8 @@ public class SafetyCheckMediatorTest {
public void setUp() {
MockitoAnnotations.initMocks(this);
mModel = SafetyCheckProperties.createSafetyCheckModel();
mMediator = new SafetyCheckMediator(mModel, mUpdatesDelegate, mBridge, mHandler);
mMediator = new SafetyCheckMediator(
mModel, mUpdatesDelegate, mSettingsLauncher, mBridge, mHandler);
// Execute any delayed tasks immediately.
doAnswer(invocation -> {
Runnable runnable = (Runnable) (invocation.getArguments()[0]);
......
......@@ -22,6 +22,7 @@ import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.safety_check.SafetyCheckProperties.SafeBrowsingState;
......@@ -152,4 +153,51 @@ public class SafetyCheckSettingsFragmentTest {
R.string.safety_check_updates_outdated),
updates.getSummary());
}
@Test
@MediumTest
public void testSafetyCheckElementsOnClick() {
createFragmentAndModel();
CallbackHelper passwordsClicked = new CallbackHelper();
CallbackHelper safeBrowsingClicked = new CallbackHelper();
CallbackHelper updatesClicked = new CallbackHelper();
// Set the listeners
TestThreadUtils.runOnUiThreadBlocking(() -> {
mModel.set(SafetyCheckProperties.PASSWORDS_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
passwordsClicked.notifyCalled();
return true;
});
mModel.set(SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
safeBrowsingClicked.notifyCalled();
return true;
});
mModel.set(SafetyCheckProperties.UPDATES_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> {
updatesClicked.notifyCalled();
return true;
});
});
Preference passwords = mFragment.findPreference(PASSWORDS);
Preference safeBrowsing = mFragment.findPreference(SAFE_BROWSING);
Preference updates = mFragment.findPreference(UPDATES);
TestThreadUtils.runOnUiThreadBlocking(() -> {
// Passwords state remains unchanged, should be clickable.
passwords.performClick();
// Safe browsing is in "checking", the element deactivates, not clickable.
mModel.set(SafetyCheckProperties.SAFE_BROWSING_STATE, SafeBrowsingState.CHECKING);
safeBrowsing.performClick();
// Updates goes through "checking" and ends up in "outdated".
// Checking: the element deactivates, clicks are not handled.
mModel.set(SafetyCheckProperties.UPDATES_STATE, UpdatesState.CHECKING);
// Final state: the element is reactivated and should handle clicks.
mModel.set(SafetyCheckProperties.UPDATES_STATE, UpdatesState.OUTDATED);
updates.performClick();
});
// Passwords and updates should get clicked, SB element is inactive.
assertEquals(1, passwordsClicked.getCallCount());
assertEquals(0, safeBrowsingClicked.getCallCount());
assertEquals(1, updatesClicked.getCallCount());
}
}
......@@ -16,6 +16,7 @@ public final class ContentUrlConstants {
public static final String ABOUT_BLANK_URL = "about://blank";
public static final String FILE_URL_PREFIX = "file://";
public static final String PLAY_STORE_URL_PREFIX = "market://details?id=";
public static final String DATA_SCHEME = "data";
public static final String HTTP_SCHEME = "http";
......
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