Log user actions on Safe Browsing settings page.

The "SafeBrowsing.Settings.UserAction*" metrics was added on Android
settings page in https://crrev.com/c/2357677. We'd like to log the
same metrics on desktop settings page as well.

Bug: 1124491
Change-Id: Ib87672da86d696fd28c46955ac78159719cfba82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2389260Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805962}

chrome/browser/resources/settings/metrics_browser_proxy.js

@@ -59,6 +59,28 @@ export const SafetyCheckInteractions = {
   COUNT: 7,
 };
 
+/**
+ * Contains all safe browsing interactions.
+ *
+ * These values are persisted to logs. Entries should not be renumbered and
+ * numeric values should never be reused.
+ *
+ * Must be kept in sync with the UserAction in safe_browsing_settings_metrics.h.
+ * @enum {number}
+ */
+export const SafeBrowsingInteractions = {
+  SAFE_BROWSING_SHOWED: 0,
+  SAFE_BROWSING_ENHANCED_PROTECTION_CLICKED: 1,
+  SAFE_BROWSING_STANDARD_PROTECTION_CLICKED: 2,
+  SAFE_BROWSING_DISABLE_SAFE_BROWSING_CLICKED: 3,
+  SAFE_BROWSING_ENHANCED_PROTECTION_EXPAND_ARROW_CLICKED: 4,
+  SAFE_BROWSING_STANDARD_PROTECTION_EXPAND_ARROW_CLICKED: 5,
+  SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_CONFIRMED: 6,
+  SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_DENIED: 7,
+  // Leave this at the end.
+  COUNT: 8,
+};
+
 /** @interface */
 export class MetricsBrowserProxy {
   /**
@@ -81,6 +103,13 @@ export class MetricsBrowserProxy {
    * @param {!PrivacyElementInteractions} interaction
    */
   recordSettingsPageHistogram(interaction) {}
+
+  /**
+   * Helper function that calls recordHistogram for the
+   * SafeBrowsing.Settings.UserAction histogram
+   * @param {!SafeBrowsingInteractions} interaction
+   */
+  recordSafeBrowsingInteractionHistogram(interaction) {}
 }
 
 /**
@@ -107,6 +136,16 @@ export class MetricsBrowserProxyImpl {
       PrivacyElementInteractions.COUNT
     ]);
   }
+
+  /** @override*/
+  recordSafeBrowsingInteractionHistogram(interaction) {
+    // TODO(crbug.com/1124491): Set the correct suffix for
+    // SafeBrowsing.Settings.UserAction. Use the .Default suffix for now.
+    chrome.send('metricsHandler:recordInHistogram', [
+      'SafeBrowsing.Settings.UserAction.Default', interaction,
+      SafeBrowsingInteractions.COUNT
+    ]);
+  }
 }
 
 addSingletonGetter(MetricsBrowserProxyImpl);

chrome/browser/resources/settings/privacy_page/collapse_radio_button.html

@@ -89,7 +89,8 @@
           </cr-policy-pref-indicator>
         </template>
         <div hidden$="[[noCollapse]]" class="separator"></div>
-        <cr-expand-button hidden$="[[noCollapse]]" expanded="{{expanded}}">
+        <cr-expand-button hidden$="[[noCollapse]]" expanded="{{expanded}}"
+              on-click="onExpandClicked_">
         </cr-expand-button>
       </div>
     </div>

chrome/browser/resources/settings/privacy_page/collapse_radio_button.js

@@ -101,4 +101,9 @@ Polymer({
         !(!!this.pref.userSelectableValues &&
           this.pref.userSelectableValues.includes(this.name));
   },
+
+  /** @private */
+  onExpandClicked_() {
+    this.fire('expand-clicked');
+  },
 });

chrome/browser/resources/settings/privacy_page/security_page.html

@@ -71,6 +71,7 @@
             label="$i18n{safeBrowsingEnhanced}"
             sub-label="$i18n{safeBrowsingEnhancedDesc}"
             hidden="[[!safeBrowsingEnhancedEnabled_]]"
+            on-expand-clicked="onEnhancedProtectionExpandButtonClicked_"
             no-automatic-collapse>
           <div slot="collapse">
             <div class="bullet-line">
@@ -111,6 +112,7 @@
             label="$i18n{safeBrowsingStandard}"
             sub-label="$i18n{safeBrowsingStandardDesc}"
             info-opened="{{infoOpened_}}"
+            on-expand-clicked="onStandardProtectionExpandButtonClicked_"
             no-automatic-collapse>
           <div slot="collapse">
             <div class="bullet-line">

chrome/browser/resources/settings/privacy_page/security_page.js

@@ -20,11 +20,11 @@ import {focusWithoutInk} from 'chrome://resources/js/cr/ui/focus_without_ink.m.j
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {loadTimeData} from '../i18n_setup.js';
-import {MetricsBrowserProxy, MetricsBrowserProxyImpl, PrivacyElementInteractions} from '../metrics_browser_proxy.js';
+import {MetricsBrowserProxy, MetricsBrowserProxyImpl, PrivacyElementInteractions, SafeBrowsingInteractions} from '../metrics_browser_proxy.js';
 import {SyncStatus} from '../people_page/sync_browser_proxy.m.js';
 import {PrefsBehavior} from '../prefs/prefs_behavior.m.js';
 import {routes} from '../route.js';
-import {Router} from '../router.m.js';
+import {Route, RouteObserverBehavior, Router} from '../router.m.js';
 
 import {PrivacyPageBrowserProxy, PrivacyPageBrowserProxyImpl} from './privacy_page_browser_proxy.m.js';
 
@@ -47,6 +47,7 @@ Polymer({
 
   behaviors: [
     PrefsBehavior,
+    RouteObserverBehavior,
   ],
 
   properties: {
@@ -153,6 +154,18 @@ Polymer({
     this.metricsBrowserProxy_ = MetricsBrowserProxyImpl.getInstance();
   },
 
+  /**
+   * RouteObserverBehavior
+   * @param {!Route} route
+   * @protected
+   */
+  currentRouteChanged(route) {
+    if (route === routes.SECURITY) {
+      this.metricsBrowserProxy_.recordSafeBrowsingInteractionHistogram(
+          SafeBrowsingInteractions.SAFE_BROWSING_SHOWED);
+    }
+  },
+
   /**
    * Updates the buttons' expanded status by propagating previous click
    * events
@@ -171,6 +184,11 @@ Polymer({
   onSafeBrowsingRadioChange_: function() {
     const selected =
         Number.parseInt(this.$.safeBrowsingRadioGroup.selected, 10);
+    const prefValue = this.getPref('generated.safe_browsing').value;
+    if (prefValue !== selected) {
+      this.recordInteractionHistogramOnRadioChange_(
+          /** @type {!SafeBrowsingSetting} */ (selected));
+    }
     if (selected === SafeBrowsingSetting.DISABLED) {
       this.showDisableSafebrowsingDialog_ = true;
     } else {
@@ -223,9 +241,13 @@ Polymer({
    * @private
    */
   onDisableSafebrowsingDialogClose_() {
+    const confirmed =
+        /** @type {!SettingsDisableSafebrowsingDialogElement} */ (
+            this.$$('settings-disable-safebrowsing-dialog'))
+            .wasConfirmed();
+    this.recordInteractionHistogramOnSafeBrowsingDialogClose_(confirmed);
     // Check if the dialog was confirmed before closing it.
-    if (/** @type {!SettingsDisableSafebrowsingDialogElement} */
-        (this.$$('settings-disable-safebrowsing-dialog')).wasConfirmed()) {
+    if (confirmed) {
       this.$.safeBrowsingRadioGroup.sendPrefChange();
       this.updateCollapsedButtons_();
     } else {
@@ -238,4 +260,60 @@ Polymer({
     // with the dialog, as it was the entry point to the dialog.
     focusWithoutInk(assert(this.$.safeBrowsingDisabled));
   },
+
+  /** @private */
+  onEnhancedProtectionExpandButtonClicked_() {
+    this.recordInteractionHistogramOnExpandButtonClicked_(
+        SafeBrowsingSetting.ENHANCED);
+  },
+
+  /** @private */
+  onStandardProtectionExpandButtonClicked_() {
+    this.recordInteractionHistogramOnExpandButtonClicked_(
+        SafeBrowsingSetting.STANDARD);
+  },
+
+  /**
+   * @param {!SafeBrowsingSetting} safeBrowsingSetting
+   * @private
+   */
+  recordInteractionHistogramOnRadioChange_(safeBrowsingSetting) {
+    let action;
+    if (safeBrowsingSetting === SafeBrowsingSetting.ENHANCED) {
+      action =
+          SafeBrowsingInteractions.SAFE_BROWSING_ENHANCED_PROTECTION_CLICKED;
+    } else if (safeBrowsingSetting === SafeBrowsingSetting.STANDARD) {
+      action =
+          SafeBrowsingInteractions.SAFE_BROWSING_STANDARD_PROTECTION_CLICKED;
+    } else {
+      action =
+          SafeBrowsingInteractions.SAFE_BROWSING_DISABLE_SAFE_BROWSING_CLICKED;
+    }
+    this.metricsBrowserProxy_.recordSafeBrowsingInteractionHistogram(action);
+  },
+
+  /**
+   * @param {!SafeBrowsingSetting} safeBrowsingSetting
+   * @private
+   */
+  recordInteractionHistogramOnExpandButtonClicked_(safeBrowsingSetting) {
+    this.metricsBrowserProxy_.recordSafeBrowsingInteractionHistogram(
+        safeBrowsingSetting === SafeBrowsingSetting.ENHANCED ?
+            SafeBrowsingInteractions
+                .SAFE_BROWSING_ENHANCED_PROTECTION_EXPAND_ARROW_CLICKED :
+            SafeBrowsingInteractions
+                .SAFE_BROWSING_STANDARD_PROTECTION_EXPAND_ARROW_CLICKED);
+  },
+
+  /**
+   * @param {!boolean} confirmed
+   * @private
+   */
+  recordInteractionHistogramOnSafeBrowsingDialogClose_(confirmed) {
+    this.metricsBrowserProxy_.recordSafeBrowsingInteractionHistogram(
+        confirmed ? SafeBrowsingInteractions
+                        .SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_CONFIRMED :
+                    SafeBrowsingInteractions
+                        .SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_DENIED);
+  },
 });

chrome/browser/resources/settings/settings.js

@@ -16,7 +16,7 @@ export {DefaultBrowserBrowserProxyImpl} from './default_browser_page/default_bro
 export {ExtensionControlBrowserProxyImpl} from './extension_control_browser_proxy.m.js';
 export {HatsBrowserProxy, HatsBrowserProxyImpl} from './hats_browser_proxy.js';
 export {LifetimeBrowserProxy, LifetimeBrowserProxyImpl} from './lifetime_browser_proxy.m.js';
-export {MetricsBrowserProxy, MetricsBrowserProxyImpl, PrivacyElementInteractions, SafetyCheckInteractions} from './metrics_browser_proxy.js';
+export {MetricsBrowserProxy, MetricsBrowserProxyImpl, PrivacyElementInteractions, SafeBrowsingInteractions, SafetyCheckInteractions} from './metrics_browser_proxy.js';
 export {OnStartupBrowserProxy, OnStartupBrowserProxyImpl} from './on_startup_page/on_startup_browser_proxy.js';
 export {EDIT_STARTUP_URL_EVENT} from './on_startup_page/startup_url_entry.js';
 export {StartupUrlsPageBrowserProxy, StartupUrlsPageBrowserProxyImpl} from './on_startup_page/startup_urls_page_browser_proxy.js';

chrome/test/data/webui/settings/security_page_test.js

@@ -7,8 +7,7 @@ import {isMac, isWindows} from 'chrome://resources/js/cr.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {SafeBrowsingSetting} from 'chrome://settings/lazy_load.js';
-import {MetricsBrowserProxyImpl, PrivacyElementInteractions,PrivacyPageBrowserProxyImpl, Router, routes, SecureDnsMode, SyncBrowserProxyImpl} from 'chrome://settings/settings.js';
-
+import {MetricsBrowserProxyImpl, PrivacyElementInteractions, PrivacyPageBrowserProxyImpl, Router, routes, SafeBrowsingInteractions, SecureDnsMode, SyncBrowserProxyImpl} from 'chrome://settings/settings.js';
 import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js';
 import {flushTasks, isChildVisible} from '../test_util.m.js';
 
@@ -76,6 +75,7 @@ suite('CrSettingsSecurityPageTest', function() {
 
   teardown(function() {
     page.remove();
+    Router.getInstance().navigateTo(routes.BASIC);
   });
 
   if (isMac || isWindows) {
@@ -387,6 +387,103 @@ suite('CrSettingsSecurityPageTest', function() {
     assertTrue(
         page.prefs.profile.password_manager_leak_detection.value === previous);
   });
+
+  test('safeBrowsingUserActionRecorded', async function() {
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('#safeBrowsingStandard').click();
+    assertEquals(
+        SafeBrowsingSetting.STANDARD, page.prefs.generated.safe_browsing.value);
+    // Not logged because it is already in standard mode.
+    assertEquals(
+        0,
+        testMetricsBrowserProxy.getCallCount(
+            'recordSafeBrowsingInteractionHistogram'));
+
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('#safeBrowsingEnhanced').click();
+    flush();
+    const enhancedClickedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions.SAFE_BROWSING_ENHANCED_PROTECTION_CLICKED,
+        enhancedClickedResult);
+
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('#safeBrowsingEnhanced').$$('cr-expand-button').click();
+    flush();
+    const enhancedExpandedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions
+            .SAFE_BROWSING_ENHANCED_PROTECTION_EXPAND_ARROW_CLICKED,
+        enhancedExpandedResult);
+
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('#safeBrowsingStandard').$$('cr-expand-button').click();
+    flush();
+    const standardExpandedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions
+            .SAFE_BROWSING_STANDARD_PROTECTION_EXPAND_ARROW_CLICKED,
+        standardExpandedResult);
+
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('#safeBrowsingDisabled').click();
+    flush();
+    const disableClickedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions.SAFE_BROWSING_DISABLE_SAFE_BROWSING_CLICKED,
+        disableClickedResult);
+
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('settings-disable-safebrowsing-dialog')
+        .$$('.cancel-button')
+        .click();
+    flush();
+    const disableDeniedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions
+            .SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_DENIED,
+        disableDeniedResult);
+
+    await flushTasks();
+
+    page.$$('#safeBrowsingDisabled').click();
+    flush();
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    page.$$('settings-disable-safebrowsing-dialog')
+        .$$('.action-button')
+        .click();
+    flush();
+    const disableConfirmedResult = await testMetricsBrowserProxy.whenCalled(
+        'recordSafeBrowsingInteractionHistogram');
+    assertEquals(
+        SafeBrowsingInteractions
+            .SAFE_BROWSING_DISABLE_SAFE_BROWSING_DIALOG_CONFIRMED,
+        disableConfirmedResult);
+  });
+
+  test('securityPageShowedRecorded', async function() {
+    testMetricsBrowserProxy.resetResolver(
+        'recordSafeBrowsingInteractionHistogram');
+    Router.getInstance().navigateTo(
+        routes.SECURITY, /* dynamicParams= */ null,
+        /* removeSearch= */ true);
+    assertEquals(
+        SafeBrowsingInteractions.SAFE_BROWSING_SHOWED,
+        await testMetricsBrowserProxy.whenCalled(
+            'recordSafeBrowsingInteractionHistogram'));
+  });
 });
 
 

chrome/test/data/webui/settings/test_metrics_browser_proxy.js

@@ -13,6 +13,7 @@ export class TestMetricsBrowserProxy extends TestBrowserProxy {
       'recordAction',
       'recordSafetyCheckInteractionHistogram',
       'recordSettingsPageHistogram',
+      'recordSafeBrowsingInteractionHistogram',
     ]);
   }
 
@@ -30,4 +31,9 @@ export class TestMetricsBrowserProxy extends TestBrowserProxy {
   recordSettingsPageHistogram(interaction) {
     this.methodCalled('recordSettingsPageHistogram', interaction);
   }
+
+  /** @override */
+  recordSafeBrowsingInteractionHistogram(interaction) {
+    this.methodCalled('recordSafeBrowsingInteractionHistogram', interaction);
+  }
 }