Commit 39e7c8da authored by Andrey Zaytsev's avatar Andrey Zaytsev Committed by Commit Bot

Safety check on Android: added UMA metrics recording

Bug: 1070620
Change-Id: I27b0b8097b73dc92d5ee9cbcaba26233e7ff50db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362653
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Reviewed-by: default avatarMartin Šrámek <msramek@chromium.org>
Auto-Submit: Andrey Zaytsev <andzaytsev@google.com>
Cr-Commit-Position: refs/heads/master@{#800118}
parent 9b4e7fce
...@@ -18,6 +18,8 @@ import androidx.preference.Preference; ...@@ -18,6 +18,8 @@ import androidx.preference.Preference;
import org.chromium.base.BuildConfig; import org.chromium.base.BuildConfig;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.password_check.CompromisedCredential; import org.chromium.chrome.browser.password_check.CompromisedCredential;
import org.chromium.chrome.browser.password_check.PasswordCheck; import org.chromium.chrome.browser.password_check.PasswordCheck;
...@@ -53,6 +55,7 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -53,6 +55,7 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
private static final int CHECKING_MIN_DURATION_MS = 1000; private static final int CHECKING_MIN_DURATION_MS = 1000;
/** Time after which the null-states will be shown: 10 minutes. */ /** Time after which the null-states will be shown: 10 minutes. */
private static final long RESET_TO_NULL_AFTER_MS = 10 * DateUtils.MINUTE_IN_MILLIS; private static final long RESET_TO_NULL_AFTER_MS = 10 * DateUtils.MINUTE_IN_MILLIS;
private static final String SAFETY_CHECK_INTERACTIONS = "Settings.SafetyCheck.Interactions";
/** Bridge to the C++ side for the Safe Browsing and passwords checks. */ /** Bridge to the C++ side for the Safe Browsing and passwords checks. */
private SafetyCheckBridge mSafetyCheckBridge; private SafetyCheckBridge mSafetyCheckBridge;
...@@ -91,6 +94,28 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -91,6 +94,28 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
private Runnable mRunnableUpdates; private Runnable mRunnableUpdates;
private long mCheckStartTime = -1; private long mCheckStartTime = -1;
/**
* UMA histogram values for Safety check interactions. Some value don't apply to Android.
* Note: this should stay in sync with SettingsSafetyCheckInteractions in enums.xml.
*/
@IntDef({SafetyCheckInteractions.STARTED, SafetyCheckInteractions.UPDATES_RELAUNCH,
SafetyCheckInteractions.PASSWORDS_MANAGE, SafetyCheckInteractions.SAFE_BROWSING_MANAGE,
SafetyCheckInteractions.EXTENSIONS_REVIEW,
SafetyCheckInteractions.CHROME_CLEANER_REBOOT,
SafetyCheckInteractions.CHROME_CLEANER_REVIEW, SafetyCheckInteractions.MAX_VALUE})
@Retention(RetentionPolicy.SOURCE)
public @interface SafetyCheckInteractions {
int STARTED = 0;
int UPDATES_RELAUNCH = 1;
int PASSWORDS_MANAGE = 2;
int SAFE_BROWSING_MANAGE = 3;
int EXTENSIONS_REVIEW = 4;
int CHROME_CLEANER_REBOOT = 5;
int CHROME_CLEANER_REVIEW = 6;
// New elements go above.
int MAX_VALUE = CHROME_CLEANER_REVIEW;
}
private final SharedPreferencesManager mPreferenceManager; private final SharedPreferencesManager mPreferenceManager;
/** /**
...@@ -100,6 +125,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -100,6 +125,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
private final Callback<Integer> mUpdatesCheckCallback = (status) -> { private final Callback<Integer> mUpdatesCheckCallback = (status) -> {
mRunnableUpdates = () -> { mRunnableUpdates = () -> {
if (mModel != null) { if (mModel != null) {
RecordHistogram.recordEnumeratedHistogram("Settings.SafetyCheck.UpdatesResult",
SafetyCheckProperties.updatesStateToNative(status), UpdateStatus.MAX_VALUE);
mModel.set(SafetyCheckProperties.UPDATES_STATE, status); mModel.set(SafetyCheckProperties.UPDATES_STATE, status);
} }
}; };
...@@ -150,6 +177,11 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -150,6 +177,11 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
// Set the listener for clicking the Safe Browsing element. // Set the listener for clicking the Safe Browsing element.
mModel.set(SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER, mModel.set(SafetyCheckProperties.SAFE_BROWSING_CLICK_LISTENER,
(Preference.OnPreferenceClickListener) (p) -> { (Preference.OnPreferenceClickListener) (p) -> {
// Record UMA metrics.
RecordUserAction.record("Settings.SafetyCheck.ManageSafeBrowsing");
RecordHistogram.recordEnumeratedHistogram(SAFETY_CHECK_INTERACTIONS,
SafetyCheckInteractions.SAFE_BROWSING_MANAGE,
SafetyCheckInteractions.MAX_VALUE);
String safeBrowsingSettingsClassName; String safeBrowsingSettingsClassName;
if (ChromeFeatureList.isEnabled( if (ChromeFeatureList.isEnabled(
ChromeFeatureList.SAFE_BROWSING_SECURITY_SECTION_UI)) { ChromeFeatureList.SAFE_BROWSING_SECURITY_SECTION_UI)) {
...@@ -225,6 +257,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -225,6 +257,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
// Cancel pending delayed show callbacks if a new check is starting while any existing // Cancel pending delayed show callbacks if a new check is starting while any existing
// elements are pending. // elements are pending.
cancelCallbacks(); cancelCallbacks();
// Record the start action in UMA.
RecordUserAction.record("Settings.SafetyCheck.Start");
// Record the start time for tracking 1 second checking delay in the UI. // Record the start time for tracking 1 second checking delay in the UI.
mCheckStartTime = SystemClock.elapsedRealtime(); mCheckStartTime = SystemClock.elapsedRealtime();
// Record the absolute start time for showing when the last Safety check was performed. // Record the absolute start time for showing when the last Safety check was performed.
...@@ -258,6 +292,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -258,6 +292,8 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
@Override @Override
public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) { public void onSafeBrowsingCheckResult(@SafeBrowsingStatus int status) {
mRunnableSafeBrowsing = () -> { mRunnableSafeBrowsing = () -> {
RecordHistogram.recordEnumeratedHistogram("Settings.SafetyCheck.SafeBrowsingResult",
status, SafeBrowsingStatus.MAX_VALUE);
mModel.set(SafetyCheckProperties.SAFE_BROWSING_STATE, mModel.set(SafetyCheckProperties.SAFE_BROWSING_STATE,
SafetyCheckProperties.safeBrowsingStateFromNative(status)); SafetyCheckProperties.safeBrowsingStateFromNative(status));
}; };
...@@ -301,8 +337,12 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -301,8 +337,12 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
// Handle error state. // Handle error state.
if (status != PasswordCheckUIStatus.IDLE) { if (status != PasswordCheckUIStatus.IDLE) {
mRunnablePasswords = () -> { mRunnablePasswords = () -> {
mModel.set(SafetyCheckProperties.PASSWORDS_STATE, @SafetyCheckProperties.PasswordsState
SafetyCheckProperties.passwordsStatefromErrorState(status)); int state = SafetyCheckProperties.passwordsStatefromErrorState(status);
RecordHistogram.recordEnumeratedHistogram("Settings.SafetyCheck.PasswordsResult",
SafetyCheckProperties.passwordsStateToNative(state),
PasswordsStatus.MAX_VALUE);
mModel.set(SafetyCheckProperties.PASSWORDS_STATE, state);
updatePasswordElementClickDestination(); updatePasswordElementClickDestination();
}; };
// Show the checking state for at least 1 second for a smoother UX. // Show the checking state for at least 1 second for a smoother UX.
...@@ -413,6 +453,11 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb ...@@ -413,6 +453,11 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
}; };
} else if (state == PasswordsState.COMPROMISED_EXIST || state == PasswordsState.SAFE) { } else if (state == PasswordsState.COMPROMISED_EXIST || state == PasswordsState.SAFE) {
listener = (p) -> { listener = (p) -> {
// Record UMA metrics.
RecordUserAction.record("Settings.SafetyCheck.ManagePasswords");
RecordHistogram.recordEnumeratedHistogram(SAFETY_CHECK_INTERACTIONS,
SafetyCheckInteractions.PASSWORDS_MANAGE,
SafetyCheckInteractions.MAX_VALUE);
// Open the Password Check UI. // Open the Password Check UI.
PasswordCheckFactory.getOrCreate().showUi( PasswordCheckFactory.getOrCreate().showUi(
p.getContext(), PasswordCheckReferrer.SAFETY_CHECK); p.getContext(), PasswordCheckReferrer.SAFETY_CHECK);
......
...@@ -76,6 +76,34 @@ class SafetyCheckProperties { ...@@ -76,6 +76,34 @@ class SafetyCheckProperties {
return 0; return 0;
} }
static @PasswordsStatus int passwordsStateToNative(@PasswordsState int state) {
switch (state) {
case PasswordsState.UNCHECKED:
// This is not used.
assert false : "PasswordsState.UNCHECKED has no native equivalent.";
return PasswordsStatus.ERROR;
case PasswordsState.CHECKING:
return PasswordsStatus.CHECKING;
case PasswordsState.SAFE:
return PasswordsStatus.SAFE;
case PasswordsState.COMPROMISED_EXIST:
return PasswordsStatus.COMPROMISED_EXIST;
case PasswordsState.OFFLINE:
return PasswordsStatus.OFFLINE;
case PasswordsState.NO_PASSWORDS:
return PasswordsStatus.NO_PASSWORDS;
case PasswordsState.SIGNED_OUT:
return PasswordsStatus.SIGNED_OUT;
case PasswordsState.QUOTA_LIMIT:
return PasswordsStatus.QUOTA_LIMIT;
case PasswordsState.ERROR:
return PasswordsStatus.ERROR;
default:
assert false : "Unknown PasswordsState value.";
return PasswordsStatus.ERROR;
}
}
@IntDef({SafeBrowsingState.UNCHECKED, SafeBrowsingState.CHECKING, @IntDef({SafeBrowsingState.UNCHECKED, SafeBrowsingState.CHECKING,
SafeBrowsingState.ENABLED_STANDARD, SafeBrowsingState.ENABLED_ENHANCED, SafeBrowsingState.ENABLED_STANDARD, SafeBrowsingState.ENABLED_ENHANCED,
SafeBrowsingState.DISABLED, SafeBrowsingState.DISABLED_BY_ADMIN, SafeBrowsingState.DISABLED, SafeBrowsingState.DISABLED_BY_ADMIN,
...@@ -127,6 +155,28 @@ class SafetyCheckProperties { ...@@ -127,6 +155,28 @@ class SafetyCheckProperties {
int ERROR = 5; int ERROR = 5;
} }
static @UpdateStatus int updatesStateToNative(@UpdatesState int state) {
switch (state) {
case UpdatesState.UNCHECKED:
// This is not used.
assert false : "UpdatesState.UNCHECKED has no native equivalent.";
return UpdateStatus.FAILED;
case UpdatesState.CHECKING:
return UpdateStatus.CHECKING;
case UpdatesState.UPDATED:
return UpdateStatus.UPDATING;
case UpdatesState.OUTDATED:
return UpdateStatus.OUTDATED;
case UpdatesState.OFFLINE:
return UpdateStatus.FAILED_OFFLINE;
case UpdatesState.ERROR:
return UpdateStatus.FAILED;
default:
assert false : "Unknown UpdatesState value.";
return UpdateStatus.FAILED;
}
}
static final PropertyKey[] ALL_KEYS = static final PropertyKey[] ALL_KEYS =
new PropertyKey[] {PASSWORDS_STATE, COMPROMISED_PASSWORDS, SAFE_BROWSING_STATE, new PropertyKey[] {PASSWORDS_STATE, COMPROMISED_PASSWORDS, SAFE_BROWSING_STATE,
UPDATES_STATE, PASSWORDS_CLICK_LISTENER, SAFE_BROWSING_CLICK_LISTENER, UPDATES_STATE, PASSWORDS_CLICK_LISTENER, SAFE_BROWSING_CLICK_LISTENER,
......
...@@ -497,6 +497,10 @@ base::string16 SafetyCheckHandler::GetStringForUpdates(UpdateStatus status) { ...@@ -497,6 +497,10 @@ base::string16 SafetyCheckHandler::GetStringForUpdates(UpdateStatus status) {
: IDS_VERSION_UI_UNOFFICIAL), : IDS_VERSION_UI_UNOFFICIAL),
base::UTF8ToUTF16(chrome::GetChannelName()), base::UTF8ToUTF16(chrome::GetChannelName()),
l10n_util::GetStringUTF16(VersionUI::VersionProcessorVariation())); l10n_util::GetStringUTF16(VersionUI::VersionProcessorVariation()));
// This state is only used on Android for recording metrics. This codepath
// is unreachable.
case UpdateStatus::kOutdated:
return base::UTF8ToUTF16("");
} }
} }
......
...@@ -56,34 +56,9 @@ class SafetyCheckHandler ...@@ -56,34 +56,9 @@ class SafetyCheckHandler
// check and should be kept in sync with the JS frontend // check and should be kept in sync with the JS frontend
// (safety_check_browser_proxy.js) and |SafetyCheck*| metrics enums in // (safety_check_browser_proxy.js) and |SafetyCheck*| metrics enums in
// enums.xml. // enums.xml.
using PasswordsStatus = safety_check::SafetyCheck::PasswordsStatus;
using SafeBrowsingStatus = safety_check::SafetyCheck::SafeBrowsingStatus; using SafeBrowsingStatus = safety_check::SafetyCheck::SafeBrowsingStatus;
enum class UpdateStatus { using UpdateStatus = safety_check::SafetyCheck::UpdateStatus;
kChecking = 0,
kUpdated = 1,
kUpdating = 2,
kRelaunch = 3,
kDisabledByAdmin = 4,
kFailedOffline = 5,
kFailed = 6,
// Non-Google branded browsers cannot check for updates using
// VersionUpdater.
kUnknown = 7,
// New enum values must go above here.
kMaxValue = kUnknown,
};
enum class PasswordsStatus {
kChecking = 0,
kSafe = 1,
kCompromisedExist = 2,
kOffline = 3,
kNoPasswords = 4,
kSignedOut = 5,
kQuotaLimit = 6,
kError = 7,
kFeatureUnavailable = 8,
// New enum values must go above here.
kMaxValue = kFeatureUnavailable,
};
enum class ExtensionsStatus { enum class ExtensionsStatus {
kChecking = 0, kChecking = 0,
kError = 1, kError = 1,
......
...@@ -20,6 +20,21 @@ class SafetyCheck { ...@@ -20,6 +20,21 @@ class SafetyCheck {
// desktop, Android, and iOS) of the safety check and should be kept in sync // desktop, Android, and iOS) of the safety check and should be kept in sync
// with the JS frontend (safety_check_browser_proxy.js) and |SafetyCheck*| // with the JS frontend (safety_check_browser_proxy.js) and |SafetyCheck*|
// metrics enums in enums.xml. // metrics enums in enums.xml.
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.safety_check
enum class PasswordsStatus {
kChecking = 0,
kSafe = 1,
kCompromisedExist = 2,
kOffline = 3,
kNoPasswords = 4,
kSignedOut = 5,
kQuotaLimit = 6,
kError = 7,
kFeatureUnavailable = 8,
// New enum values must go above here.
kMaxValue = kFeatureUnavailable,
};
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.safety_check // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.safety_check
enum class SafeBrowsingStatus { enum class SafeBrowsingStatus {
kChecking = 0, kChecking = 0,
...@@ -34,6 +49,24 @@ class SafetyCheck { ...@@ -34,6 +49,24 @@ class SafetyCheck {
kMaxValue = kEnabledStandardAvailableEnhanced, kMaxValue = kEnabledStandardAvailableEnhanced,
}; };
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.safety_check
enum class UpdateStatus {
kChecking = 0,
kUpdated = 1,
kUpdating = 2,
kRelaunch = 3,
kDisabledByAdmin = 4,
kFailedOffline = 5,
kFailed = 6,
// Non-Google branded browsers cannot check for updates using
// VersionUpdater.
kUnknown = 7,
// Only used in Android where the user is directed to the Play Store.
kOutdated = 8,
// New enum values must go above here.
kMaxValue = kOutdated,
};
class SafetyCheckHandlerInterface { class SafetyCheckHandlerInterface {
public: public:
virtual void OnSafeBrowsingCheckResult(SafeBrowsingStatus status) = 0; virtual void OnSafeBrowsingCheckResult(SafeBrowsingStatus status) = 0;
......
...@@ -61928,6 +61928,7 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf ...@@ -61928,6 +61928,7 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
<int value="5" label="FAILED_OFFLINE"/> <int value="5" label="FAILED_OFFLINE"/>
<int value="6" label="FAILED"/> <int value="6" label="FAILED"/>
<int value="7" label="UNKNOWN"/> <int value="7" label="UNKNOWN"/>
<int value="8" label="OUTDATED"/>
</enum> </enum>
<enum name="SafetyTipInteraction"> <enum name="SafetyTipInteraction">
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