[Extensions] Add user actions to setting page site access host toggling

Adds several user actions for interacting with the site access section
of an extensions setting page, for extensions which requested a list of
specific hosts in their permissions.

For descriptions of each user action, check the changes to actions.xml

Bug: 1102658
Change-Id: I627d91f3166647f4bfc581e8c9ecab96414c04fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317819
Commit-Queue: Tim Judkins <tjudkins@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802809}

chrome/browser/resources/extensions/extensions.js

@@ -7,6 +7,7 @@ import './manager.js';
 export {getToastManager} from 'chrome://resources/cr_elements/cr_toast/cr_toast_manager.m.js';
 export {ActivityLogPageState} from './activity_log/activity_log_history.js';
 export {ARG_URL_PLACEHOLDER} from './activity_log/activity_log_stream_item.js';
+export {UserAction} from './item_util.js';
 // <if expr="chromeos">
 export {KioskBrowserProxyImpl} from './kiosk_browser_proxy.js';
 // </if>

chrome/browser/resources/extensions/host_permissions_toggle_list.html

@@ -27,7 +27,8 @@
 <div id="section-heading">
   <span>$i18n{hostPermissionsDescription}</span>
   <a id="link-icon-button" aria-label="$i18n{learnMore}"
-      href="$i18n{hostPermissionsLearnMoreLink}" target="_blank">
+      href="$i18n{hostPermissionsLearnMoreLink}" target="_blank"
+      on-click="onLearnMoreClick_">
     <iron-icon icon="cr:help-outline"></iron-icon>
   </a>
 </div>

chrome/browser/resources/extensions/host_permissions_toggle_list.js

@@ -13,6 +13,7 @@ import './strings.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {ItemDelegate} from './item.js';
+import {UserAction} from './item_util.js';
 
 Polymer({
   is: 'extensions-host-permissions-toggle-list',
@@ -61,8 +62,11 @@ Polymer({
     });
   },
 
-  /** @private */
-  onAllHostsToggleChanged_() {
+  /**
+   * @param {!CustomEvent<boolean>} e
+   * @private
+   */
+  onAllHostsToggleChanged_(e) {
     // TODO(devlin): In the case of going from all sites to specific sites,
     // we'll withhold all sites (i.e., all specific site toggles will move to
     // unchecked, and the user can check them individually). This is slightly
@@ -70,22 +74,38 @@ Polymer({
     // switch leaves everything synced, and user can uncheck them
     // individually. It could be nice to align on behavior, but probably not
     // super high priority.
-    this.delegate.setItemHostAccess(
-        this.itemId,
-        this.$.allHostsToggle.checked ?
-            chrome.developerPrivate.HostAccess.ON_ALL_SITES :
-            chrome.developerPrivate.HostAccess.ON_SPECIFIC_SITES);
+    const checked = e.detail;
+
+    if (checked) {
+      this.delegate.setItemHostAccess(
+          this.itemId, chrome.developerPrivate.HostAccess.ON_ALL_SITES);
+      this.delegate.recordUserAction(UserAction.ALL_TOGGLED_ON);
+    } else {
+      this.delegate.setItemHostAccess(
+          this.itemId, chrome.developerPrivate.HostAccess.ON_SPECIFIC_SITES);
+      this.delegate.recordUserAction(UserAction.ALL_TOGGLED_OFF);
+    }
   },
 
-  /** @private */
+  /**
+   * @param {!CustomEvent<boolean>} e
+   * @private
+   */
   onHostAccessChanged_(e) {
     const host = e.target.host;
     const checked = e.target.checked;
 
     if (checked) {
       this.delegate.addRuntimeHostPermission(this.itemId, host);
+      this.delegate.recordUserAction(UserAction.SPECIFIC_TOGGLED_ON);
     } else {
       this.delegate.removeRuntimeHostPermission(this.itemId, host);
+      this.delegate.recordUserAction(UserAction.SPECIFIC_TOGGLED_OFF);
     }
   },
+
+  /** @private */
+  onLearnMoreClick_() {
+    this.delegate.recordUserAction(UserAction.LEARN_MORE);
+  }
 });

chrome/browser/resources/extensions/item.js

@@ -110,6 +110,11 @@ export class ItemDelegate {
    * @return {!Promise<void>}
    */
   removeRuntimeHostPermission(id, host) {}
+
+  // TODO(tjudkins): This function is not specific to items, so should be pulled
+  // out to a more generic place when we need to access it from elsewhere.
+  /** @param {string} metricName */
+  recordUserAction(metricName) {}
 }
 
 Polymer({

chrome/browser/resources/extensions/item_util.js

@@ -25,6 +25,16 @@ export const EnableControl = {
   ENABLE_TOGGLE: 'ENABLE_TOGGLE',
 };
 
+// TODO(tjudkins): This should be extracted to a shared metrics module.
+/** @enum {string} */
+export const UserAction = {
+  ALL_TOGGLED_ON: 'Extensions.Settings.HostList.AllHostsToggledOn',
+  ALL_TOGGLED_OFF: 'Extensions.Settings.HostList.AllHostsToggledOff',
+  SPECIFIC_TOGGLED_ON: 'Extensions.Settings.HostList.SpecificHostToggledOn',
+  SPECIFIC_TOGGLED_OFF: 'Extensions.Settings.HostList.SpecificHostToggledOff',
+  LEARN_MORE: 'Extensions.Settings.HostList.LearnMoreActivated',
+};
+
 /**
  * Returns true if the extension is enabled, including terminated
  * extensions.

chrome/browser/resources/extensions/service.js

@@ -106,6 +106,11 @@ export class Service {
     });
   }
 
+  /** @override */
+  recordUserAction(metricName) {
+    chrome.metricsPrivate.recordUserAction(metricName);
+  }
+
   /**
    * Opens a file browser dialog for the user to select a file (or directory).
    * @param {chrome.developerPrivate.SelectType} selectType

chrome/test/data/webui/extensions/host_permissions_toggle_list_test.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'chrome://extensions/extensions.js';
+import {UserAction} from 'chrome://extensions/extensions.js';
 
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {TestService} from './test_service.js';
@@ -145,9 +145,33 @@ suite('HostPermissionsToggleList', function() {
     expectFalse(hostToggles[2].checked);
   });
 
-  // Tests that clicking the "allow on all sites" toggle changes the item
-  // host access properly.
-  test('clicking all hosts toggle', function() {
+  // Tests that clicking the "learn more" button is logged as a user action
+  // correctly.
+  test('clicking learn more link', async function() {
+    const permissions = {
+      hostAccess: HostAccess.ON_CLICK,
+      hasAllHosts: false,
+      hosts: [
+        {host: EXAMPLE_COM, granted: false},
+      ],
+    };
+
+    element.permissions = permissions;
+    flush();
+
+    const learnMoreButton = element.$$('#link-icon-button');
+    // Prevent triggering the navigation, which could interfere with tests.
+    learnMoreButton.href = '#';
+    learnMoreButton.target = '_self';
+    learnMoreButton.click();
+
+    const metricName = await delegate.whenCalled('recordUserAction');
+    expectEquals(UserAction.LEARN_MORE, metricName);
+  });
+
+  // Tests that clicking the "allow on the following sites" toggle when it is in
+  // the "off" state calls the delegate as expected.
+  test('clicking all hosts toggle from off to on', function() {
     const permissions = {
       hostAccess: HostAccess.ON_CLICK,
       hasAllHosts: false,
@@ -163,10 +187,44 @@ suite('HostPermissionsToggleList', function() {
     assertTrue(!!element.$);
     const allSites = element.$.allHostsToggle;
     allSites.getLabel().click();
-    return delegate.whenCalled('setItemHostAccess').then((args) => {
-      expectEquals(ITEM_ID, args[0] /* id */);
-      expectEquals(HostAccess.ON_ALL_SITES, args[1] /* access */);
-    });
+    return delegate.whenCalled('setItemHostAccess')
+        .then(([id, access]) => {
+          expectEquals(ITEM_ID, id);
+          expectEquals(HostAccess.ON_ALL_SITES, access);
+          return delegate.whenCalled('recordUserAction');
+        })
+        .then(metricName => {
+          expectEquals(UserAction.ALL_TOGGLED_ON, metricName);
+        });
+  });
+
+  // Tests that clicking the "allow on the following sites" toggle when it is in
+  // the "on" state calls the delegate as expected.
+  test('clicking all hosts toggle from on to off', function() {
+    const permissions = {
+      hostAccess: HostAccess.ON_ALL_SITES,
+      hasAllHosts: false,
+      hosts: [
+        {host: EXAMPLE_COM, granted: true},
+        {host: GOOGLE_COM, granted: true},
+        {host: CHROMIUM_ORG, granted: true},
+      ],
+    };
+    element.permissions = permissions;
+    flush();
+
+    assertTrue(!!element.$);
+    const allSites = element.$.allHostsToggle;
+    allSites.getLabel().click();
+    return delegate.whenCalled('setItemHostAccess')
+        .then(([id, access]) => {
+          expectEquals(ITEM_ID, id);
+          expectEquals(HostAccess.ON_SPECIFIC_SITES, access);
+          return delegate.whenCalled('recordUserAction');
+        })
+        .then((metricName) => {
+          expectEquals(UserAction.ALL_TOGGLED_OFF, metricName);
+        });
   });
 
   // Tests that toggling a site's enabled state toggles the extension's access
@@ -193,16 +251,25 @@ suite('HostPermissionsToggleList', function() {
 
     hostToggles[0].getLabel().click();
     return delegate.whenCalled('removeRuntimeHostPermission')
-        .then((args) => {
-          expectEquals(ITEM_ID, args[0] /* id */);
-          expectEquals(CHROMIUM_ORG, args[1] /* site */);
+        .then(([id, site]) => {
+          expectEquals(ITEM_ID, id);
+          expectEquals(CHROMIUM_ORG, site);
+          return delegate.whenCalled('recordUserAction');
+        })
+        .then((metricName) => {
+          expectEquals(UserAction.SPECIFIC_TOGGLED_OFF, metricName);
+          delegate.resetResolver('recordUserAction');
 
           hostToggles[2].getLabel().click();
           return delegate.whenCalled('addRuntimeHostPermission');
         })
-        .then((args) => {
-          expectEquals(ITEM_ID, args[0] /* id */);
-          expectEquals(GOOGLE_COM, args[1] /* site */);
+        .then(([id, site]) => {
+          expectEquals(ITEM_ID, id);
+          expectEquals(GOOGLE_COM, site);
+          return delegate.whenCalled('recordUserAction');
+        })
+        .then((metricName) => {
+          expectEquals(UserAction.SPECIFIC_TOGGLED_ON, metricName);
         });
   });
 });

chrome/test/data/webui/extensions/test_service.js

@@ -19,6 +19,7 @@ export class TestService extends TestBrowserProxy {
       'getFilteredExtensionActivityLog',
       'getProfileConfiguration',
       'loadUnpacked',
+      'recordUserAction',
       'retryLoadUnpacked',
       'reloadItem',
       'removeRuntimeHostPermission',
@@ -218,4 +219,9 @@ export class TestService extends TestBrowserProxy {
   downloadActivities(rawActivityData, fileName) {
     this.methodCalled('downloadActivities', [rawActivityData, fileName]);
   }
+
+  /** @override */
+  recordUserAction(metricName) {
+    this.methodCalled('recordUserAction', metricName);
+  }
 }

tools/metrics/actions/actions.xml

@@ -6457,6 +6457,64 @@ should be able to be added at any place in this file.
   </description>
 </action>
 
+<action name="Extensions.Settings.HostList.AllHostsToggledOff">
+  <owner>tjudkins@chromium.org</owner>
+  <owner>extensions-core@chromium.org</owner>
+  <description>
+    The user activated the toggle at the top of the list of hosts in the site
+    access section of an extension's settings page, switching it from the on
+    state to the off state. Note: the host list is only shown on the settings
+    page for extensions which request a list of specific hosts in their
+    permissions.
+  </description>
+</action>
+
+<action name="Extensions.Settings.HostList.AllHostsToggledOn">
+  <owner>tjudkins@chromium.org</owner>
+  <owner>extensions-core@chromium.org</owner>
+  <description>
+    The user activated the toggle at the top of the list of hosts in the site
+    access section of an extension's settings page, switching it from the off
+    state to the on state. Note: the host list is only shown on the settings
+    page for extensions which request a list of specific hosts in their
+    permissions.
+  </description>
+</action>
+
+<action name="Extensions.Settings.HostList.LearnMoreActivated">
+  <owner>tjudkins@chromium.org</owner>
+  <owner>extensions-core@chromium.org</owner>
+  <description>
+    The user activated the learn more icon on the site access section of an
+    extension's settings page, for an extension which requested a list of
+    specific hosts in their permissions.
+  </description>
+</action>
+
+<action name="Extensions.Settings.HostList.SpecificHostToggledOff">
+  <owner>tjudkins@chromium.org</owner>
+  <owner>extensions-core@chromium.org</owner>
+  <description>
+    The user activated a toggle next to a specific host entry in the list of
+    hosts on the site access section of an extension's settings page, switching
+    it from the on state to the off state. Note: the host list is only shown on
+    the settings page for extensions which request a list of specific hosts in
+    their permissions.
+  </description>
+</action>
+
+<action name="Extensions.Settings.HostList.SpecificHostToggledOn">
+  <owner>tjudkins@chromium.org</owner>
+  <owner>extensions-core@chromium.org</owner>
+  <description>
+    The user activated a toggle next to a specific host entry in the list of
+    hosts on the site access section of an extension's settings page, switching
+    it from the off state to the on state. Note: the host list is only shown on
+    the settings page for extensions which request a list of specific hosts in
+    their permissions.
+  </description>
+</action>
+
 <action name="Extensions.Settings.Hosts.ActionMenuEditActivated">
   <owner>tjudkins@chromium.org</owner>
   <owner>extensions-core@chromium.org</owner>