Commit 4146e1c0 authored by Tim Judkins's avatar Tim Judkins Committed by Commit Bot

[Extensions] Add user actions to settings page site access settings

Adds several user actions for interacting with the site access settings
on an extension's settings page. The user actions are specific to the
interface for extensions with "all_url" like host permissions requested
and don't cover the interface for extensions with a specific list of
hosts requested in their permissions.

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

Bug: 1102658
Change-Id: Iaaa547f3eec40c7fa4db84ff0724076cc700f18e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2298177
Commit-Queue: Tim Judkins <tjudkins@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791337}
parent 5271317f
......@@ -54,7 +54,8 @@
<div id="section-heading">
<span>$i18n{hostPermissionsHeading}</span>
<a class="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>
......
......@@ -101,6 +101,17 @@ Polymer({
value: null,
},
/**
* Indicator to track if an onHostAccessChange_ event is coming from the
* setting being automatically reverted to the previous value, after a
* change to a new value was canceled.
* @private
*/
revertingHostAccess_: {
type: Boolean,
value: false,
},
/**
* Proxying the enum to be used easily by the html template.
* @private
......@@ -119,6 +130,25 @@ Polymer({
const group = /** @type {!HTMLElement} */ (this.$['host-access']);
const access = group.selected;
// Log a user action when the host access selection is changed by the user,
// but not when reverting from a canceled change to another setting.
if (!this.revertingHostAccess_) {
switch (access) {
case chrome.developerPrivate.HostAccess.ON_CLICK:
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.OnClickSelected');
break;
case chrome.developerPrivate.HostAccess.ON_SPECIFIC_SITES:
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.OnSpecificSitesSelected');
break;
case chrome.developerPrivate.HostAccess.ON_ALL_SITES:
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.OnAllSitesSelected');
break;
}
}
if (access === chrome.developerPrivate.HostAccess.ON_SPECIFIC_SITES &&
this.permissions.hostAccess !==
chrome.developerPrivate.HostAccess.ON_SPECIFIC_SITES) {
......@@ -169,6 +199,8 @@ Polymer({
* @private
*/
onAddHostClick_(e) {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.AddHostActivated');
const target = /** @type {!HTMLElement} */ (e.target);
this.doShowHostDialog_(target, null);
},
......@@ -199,9 +231,13 @@ Polymer({
onHostDialogCancel_() {
// The user canceled the dialog. Set host-access back to the old value,
// if the dialog was shown when just transitioning to a new state.
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.AddHostDialogCanceled');
if (this.oldHostAccess_) {
assert(this.permissions.hostAccess === this.oldHostAccess_);
this.revertingHostAccess_ = true;
this.$['host-access'].selected = this.oldHostAccess_;
this.revertingHostAccess_ = false;
this.oldHostAccess_ = null;
}
},
......@@ -222,6 +258,8 @@ Polymer({
* @private
*/
onEditHostClick_(e) {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.ActionMenuOpened');
this.actionMenuModel_ = e.model.item;
this.actionMenuAnchorElement_ = e.target;
const actionMenu =
......@@ -231,6 +269,8 @@ Polymer({
/** @private */
onActionMenuEditClick_() {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.ActionMenuEditActivated');
// Cache the site before closing the action menu, since it's cleared.
const site = this.actionMenuModel_;
......@@ -246,6 +286,8 @@ Polymer({
/** @private */
onActionMenuRemoveClick_() {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.ActionMenuRemoveActivated');
this.delegate.removeRuntimeHostPermission(
this.itemId, assert(this.actionMenuModel_, 'Action Menu Model'));
this.closeActionMenu_();
......@@ -263,4 +305,10 @@ Polymer({
this.actionMenuModel_ = null;
this.actionMenuAnchorElement_ = null;
},
/** @private */
onLearnMoreClick_() {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.LearnMoreActivated');
}
});
......@@ -163,6 +163,8 @@ Polymer({
* @private
*/
onSubmitTap_() {
chrome.metricsPrivate.recordUserAction(
'Extensions.Settings.Hosts.AddHostDialogSubmitted');
if (this.currentSite !== null) {
this.handleEdit_();
} else {
......
......@@ -9,10 +9,12 @@ import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min
import {eventToPromise, isChildVisible} from '../test_util.m.js';
import {TestService} from './test_service.js';
import {MetricsPrivateMock} from './test_util.js';
suite('RuntimeHostPermissions', function() {
/** @type {RuntimeHostPermissionsElement} */ let element;
/** @type {TestService} */ let delegate;
/** @type {Function} */ let getUserActionCount;
const HostAccess = chrome.developerPrivate.HostAccess;
const ITEM_ID = 'a'.repeat(32);
......@@ -25,6 +27,12 @@ suite('RuntimeHostPermissions', function() {
element.itemId = ITEM_ID;
document.body.appendChild(element);
chrome.metricsPrivate = new MetricsPrivateMock();
// For convenience, we define a shorter name to call getUserActionCount
// with.
getUserActionCount = (...args) =>
chrome.metricsPrivate.getUserActionCount(...args);
});
teardown(function() {
......@@ -70,7 +78,7 @@ suite('RuntimeHostPermissions', function() {
expectTrue(testIsVisible('#add-host'));
});
test('permissions selection', function() {
test('permissions selection', async () => {
const permissions = {
hostAccess: HostAccess.ON_CLICK,
hasAllHosts: true,
......@@ -96,13 +104,20 @@ suite('RuntimeHostPermissions', function() {
}
// Check that selecting different values correctly notifies the delegate.
return expectDelegateCallOnAccessChange(HostAccess.ON_ALL_SITES)
.then(() => {
return expectDelegateCallOnAccessChange(HostAccess.ON_CLICK);
});
await expectDelegateCallOnAccessChange(HostAccess.ON_ALL_SITES);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnAllSitesSelected'), 1);
await expectDelegateCallOnAccessChange(HostAccess.ON_CLICK);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnClickSelected'), 1);
// Finally select back to all sites and ensure the user action for it has
// incremented.
await expectDelegateCallOnAccessChange(HostAccess.ON_ALL_SITES);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnAllSitesSelected'), 2);
});
test('on select sites cancel', function() {
test('on select sites cancel', async () => {
const permissions = {
hostAccess: HostAccess.ON_CLICK,
hasAllHosts: true,
......@@ -120,6 +135,11 @@ suite('RuntimeHostPermissions', function() {
flush();
const dialog = element.$$('extensions-runtime-hosts-dialog');
assertTrue(!!dialog);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnClickSelected'), 0);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnSpecificSitesSelected'),
1);
expectTrue(dialog.updateHostAccess);
......@@ -128,10 +148,23 @@ suite('RuntimeHostPermissions', function() {
assertTrue(dialog.isOpen());
let whenClosed = eventToPromise('close', dialog);
dialog.$$('.cancel-button').click();
return whenClosed.then(() => {
flush();
expectEquals(HostAccess.ON_CLICK, selectHostAccess.selected);
});
await whenClosed;
flush();
expectEquals(HostAccess.ON_CLICK, selectHostAccess.selected);
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.AddHostDialogCanceled'),
1);
// Reverting to a previous option when canceling the dialog doesn't count
// as a user action, so the on-click action count should still be 0.
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnClickSelected'), 0);
// Changing to a different option after this should still log a user action
// as expected.
selectHostAccess.selected = HostAccess.ON_ALL_SITES;
flush();
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnAllSitesSelected'), 1);
});
test('on select sites accept', function() {
......@@ -148,6 +181,9 @@ suite('RuntimeHostPermissions', function() {
assertTrue(!!selectHostAccess);
selectHostAccess.selected = HostAccess.ON_SPECIFIC_SITES;
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.OnSpecificSitesSelected'),
1);
flush();
const dialog = element.$$('extensions-runtime-hosts-dialog');
......@@ -168,6 +204,10 @@ suite('RuntimeHostPermissions', function() {
return whenClosed.then(() => {
flush();
expectEquals(HostAccess.ON_SPECIFIC_SITES, selectHostAccess.selected);
expectEquals(
getUserActionCount(
'Extensions.Settings.Hosts.AddHostDialogSubmitted'),
1);
// Simulate the new host being added.
const updatedPermissions = {
......@@ -184,11 +224,17 @@ suite('RuntimeHostPermissions', function() {
// Open the dialog by clicking to edit the host permission.
const editHost = element.$$('.edit-host');
editHost.click();
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.ActionMenuOpened'), 1);
const actionMenu = element.$$('cr-action-menu');
const actionMenuEdit = actionMenu.querySelector('#action-menu-edit');
assertTrue(!!actionMenuEdit);
actionMenuEdit.click();
flush();
expectEquals(
getUserActionCount(
'Extensions.Settings.Hosts.ActionMenuEditActivated'),
1);
// Verify that the dialog does not want to update the old host access.
// Regression test for https://crbug.com/903082.
......@@ -220,6 +266,8 @@ suite('RuntimeHostPermissions', function() {
addHostButton.click();
flush();
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.AddHostActivated'), 1);
const dialog = element.$$('extensions-runtime-hosts-dialog');
assertTrue(!!dialog);
expectTrue(dialog.$.dialog.open);
......@@ -243,6 +291,8 @@ suite('RuntimeHostPermissions', function() {
const editHost = element.$$('.edit-host');
assertTrue(!!editHost);
editHost.click();
expectEquals(
getUserActionCount('Extensions.Settings.Hosts.ActionMenuOpened'), 1);
const actionMenu = element.$$('cr-action-menu');
assertTrue(!!actionMenu);
expectTrue(actionMenu.open);
......@@ -251,6 +301,10 @@ suite('RuntimeHostPermissions', function() {
assertTrue(!!remove);
remove.click();
expectEquals(
getUserActionCount(
'Extensions.Settings.Hosts.ActionMenuRemoveActivated'),
1);
return delegate.whenCalled('removeRuntimeHostPermission').then((args) => {
expectEquals(ITEM_ID, args[0] /* id */);
expectEquals('https://chromium.org', args[1] /* site */);
......
......@@ -4,7 +4,10 @@
import {getPatternFromSite} from 'chrome://extensions/extensions.js';
import {eventToPromise} from '../test_util.m.js';
import {TestService} from './test_service.js';
import {MetricsPrivateMock} from './test_util.js';
suite('RuntimeHostsDialog', function() {
/** @type {RuntimeHostsDialogElement} */ let dialog;
......@@ -20,6 +23,8 @@ suite('RuntimeHostsDialog', function() {
dialog.itemId = ITEM_ID;
document.body.appendChild(dialog);
chrome.metricsPrivate = new MetricsPrivateMock();
});
teardown(function() {
......@@ -104,6 +109,14 @@ suite('RuntimeHostsDialog', function() {
.then((args) => {
expectEquals(ITEM_ID, args[0] /* id */);
expectEquals(newPattern, args[1] /* pattern */);
return eventToPromise('close', dialog);
})
.then(() => {
expectFalse(dialog.isOpen());
expectEquals(
chrome.metricsPrivate.getUserActionCount(
'Extensions.Settings.Hosts.AddHostDialogSubmitted'),
1);
});
});
......
......@@ -131,6 +131,24 @@ export class MockItemDelegate extends ClickMock {
}
}
/**
* A mock to intercept User Action logging calls and verify how many times they
* were called.
*/
export class MetricsPrivateMock {
constructor() {
this.userActionMap = new Map();
}
getUserActionCount(metricName) {
return this.userActionMap.get(metricName) || 0;
}
recordUserAction(metricName) {
this.userActionMap.set(metricName, this.getUserActionCount(metricName) + 1);
}
}
/**
* @param {!HTMLElement} element
* @return {boolean} whether or not the element passed in is visible
......
......@@ -6366,6 +6366,114 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="Extensions.Settings.Hosts.ActionMenuEditActivated">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user activated the &quot;Edit&quot; option in the context menu for a
specific host entry in the site access section of an extension's settings
page.
</description>
</action>
<action name="Extensions.Settings.Hosts.ActionMenuOpened">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
While having the site access on an extension's setting page set to &quot;On
specific sites&quot;, the user activated the three dot button on a specific
host entry.
</description>
</action>
<action name="Extensions.Settings.Hosts.ActionMenuRemoveActivated">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user activated the &quot;Remove&quot; option in the context menu for a
specific host entry in the site access section of an extension's settings
page.
</description>
</action>
<action name="Extensions.Settings.Hosts.AddHostActivated">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user activated the link to add a new host to the list of sites an
extension is allowed to access, on an extesion's settings page. Note that
this is only visible and can only be activated when the extension is already
set to run only on a specific list of sites.
</description>
</action>
<action name="Extensions.Settings.Hosts.AddHostDialogCanceled">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user closed the dialog for adding a specific host to the list of sites
an extension is allowed to access, on an extension's settings page. This can
be triggered either by activating the cancel button on the dialog, or by
pressing escape to dismiss it.
</description>
</action>
<action name="Extensions.Settings.Hosts.AddHostDialogSubmitted">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user activated the confirmation button on the dialog for adding or
editing a specific host in the list of sites an extension is allowed to
access, on an extension's settings page. Note that this button is only
enabled when a host pattern entered into the textbox of the dialog is valid.
</description>
</action>
<action name="Extensions.Settings.Hosts.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 an
&quot;all_urls&quot; type permission.
</description>
</action>
<action name="Extensions.Settings.Hosts.OnAllSitesSelected">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user changed the site access setting for an extension to the &quot;On
all sites&quot; radio option on an extension's settings page. This is only
recorded when the user choses to change to this option and not if it is
automatically changed back when canceling a change to the &quot;specific
sites&quot; option.
</description>
</action>
<action name="Extensions.Settings.Hosts.OnClickSelected">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user changed the site access setting for an extension to the &quot;On
click&quot; radio option on an extension's settings page. This is only
recorded when the user choses to change to this option and not if it is
automatically changed back when canceling a change to the &quot;specific
sites&quot; option.
</description>
</action>
<action name="Extensions.Settings.Hosts.OnSpecificSitesSelected">
<owner>tjudkins@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<description>
The user changed the site access setting for an extension to the &quot;On
specific sites&quot; radio option on an extension's settings page. Note that
on changing to this selection, a dialog pops up for entering a specfic host
pattern.
</description>
</action>
<action name="Extensions.Toolbar.ExtensionActivatedFromMenu">
<owner>rdevlin.cronin@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
......
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