Skip to content

Commit

Permalink
shortcuts: Add metric to track side panel navigation
Browse files Browse the repository at this point in the history
Bug: b/216049298
Test: ash_unittests, browser_tests
Change-Id: Ifacadbaf8b579cfebfe0eb7e0836a2840695a8c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4937235
Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: David Padlipsky <dpad@google.com>
Cr-Commit-Position: refs/heads/main@{#1212851}
  • Loading branch information
Jimmy authored and Chromium LUCI CQ committed Oct 20, 2023
1 parent c680850 commit f413d4f
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 2 deletions.
1 change: 1 addition & 0 deletions ash/public/mojom/accelerator_info.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ enum AcceleratorLayoutStyle {
};

// Enum of top-level accelerator categories. Used in the UI for categorization.
// Must be kept in sync with enums.xml: `ShortcutCustomizationMainCategory`.
enum AcceleratorCategory {
kGeneral,
kDevice,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,12 @@ void AcceleratorConfigurationProvider::RecordUserAction(
}
}

void AcceleratorConfigurationProvider::RecordMainCategoryNavigation(
mojom::AcceleratorCategory category) {
base::UmaHistogramEnumeration(
"Ash.ShortcutCustomization.MainCategoryNavigation", category);
}

void AcceleratorConfigurationProvider::BindInterface(
mojo::PendingReceiver<
shortcut_customization::mojom::AcceleratorConfigurationProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ class AcceleratorConfigurationProvider
void RestoreAllDefaults(RestoreAllDefaultsCallback callback) override;
void RecordUserAction(
shortcut_customization::mojom::UserAction user_action) override;
void RecordMainCategoryNavigation(
mojom::AcceleratorCategory category) override;

// ui::InputDeviceEventObserver:
void OnInputDeviceConfigurationChanged(uint8_t input_device_types) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3171,6 +3171,26 @@ TEST_F(AcceleratorConfigurationProviderTest, UserActions) {
"ShortcutCustomization_SuccessfullyModified"));
}

TEST_F(AcceleratorConfigurationProviderTest, MainNavigationCategoryMetrics) {
histogram_tester_->ExpectBucketCount(
"Ash.ShortcutCustomization.MainCategoryNavigation",
mojom::AcceleratorCategory::kGeneral, 0);
histogram_tester_->ExpectBucketCount(
"Ash.ShortcutCustomization.MainCategoryNavigation",
mojom::AcceleratorCategory::kAccessibility, 0);

provider_->RecordMainCategoryNavigation(mojom::AcceleratorCategory::kGeneral);
provider_->RecordMainCategoryNavigation(
mojom::AcceleratorCategory::kAccessibility);

histogram_tester_->ExpectBucketCount(
"Ash.ShortcutCustomization.MainCategoryNavigation",
mojom::AcceleratorCategory::kGeneral, 1);
histogram_tester_->ExpectBucketCount(
"Ash.ShortcutCustomization.MainCategoryNavigation",
mojom::AcceleratorCategory::kAccessibility, 1);
}

TEST_F(AcceleratorConfigurationProviderTest,
VerifyAllAcceleratorsHaveKeyString) {
// The following is a set of VKEYs that are ignored in this test. If there is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,7 @@ interface AcceleratorConfigurationProvider {

// Records the specific user action histogram.
RecordUserAction(UserAction user_action);

// Records the side panel category the user navigates to.
RecordMainCategoryNavigation(ash.mojom.AcceleratorCategory category);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {assert} from 'chrome://resources/js/assert.js';

import {AcceleratorResultData, AcceleratorsUpdatedObserverRemote, PolicyUpdatedObserverRemote, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';

import {Accelerator, AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';
import {Accelerator, AcceleratorCategory, AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';


/**
Expand All @@ -34,6 +34,7 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
private addAcceleratorCallCount: number = 0;
private removeAcceleratorCallCount: number = 0;
private lastRecordedUserAction: UserAction;
private lastRecordedMainCategory: AcceleratorCategory;

constructor() {
this.methods = new FakeMethodResolver();
Expand All @@ -55,6 +56,7 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
this.methods.register('getConflictAccelerator');
this.methods.register('getDefaultAcceleratorsForId');
this.methods.register('recordUserAction');
this.methods.register('recordMainCategoryNavigation');
this.registerObservables();
}

Expand Down Expand Up @@ -179,6 +181,14 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
return this.lastRecordedUserAction;
}

recordMainCategoryNavigation(category: AcceleratorCategory): void {
this.lastRecordedMainCategory = category;
}

getLatestMainCategoryNavigated(): AcceleratorCategory {
return this.lastRecordedMainCategory;
}

preventProcessingAccelerators(_preventProcessingAccelerators: boolean):
Promise<void> {
++this.preventProcessingAcceleratorsCallCount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {AcceleratorConfigurationProvider, AcceleratorConfigurationProviderRemote

import {fakeAcceleratorConfig, fakeLayoutInfo} from './fake_data.js';
import {FakeShortcutProvider} from './fake_shortcut_provider.js';
import {Accelerator, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';
import {Accelerator, AcceleratorCategory, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';

/**
* @fileoverview
Expand Down Expand Up @@ -145,6 +145,10 @@ export class ShortcutProviderWrapper implements ShortcutProviderInterface {
recordUserAction(userAction: UserAction): void {
this.remote.recordUserAction(userAction);
}

recordMainCategoryNavigation(category: AcceleratorCategory): void {
this.remote.recordMainCategoryNavigation(category);
}
}

export function getShortcutProvider(): ShortcutProviderInterface {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {afterNextRender, microTask, PolymerElement} from 'chrome://resources/pol
import {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
import {AcceleratorRowElement} from './accelerator_row.js';
import {AcceleratorSubsectionElement} from './accelerator_subsection.js';
import {getShortcutProvider} from './mojo_interface_provider.js';
import {RouteObserver, Router} from './router.js';
import {AcceleratorCategory, AcceleratorSubcategory} from './shortcut_types';
import {getTemplate} from './shortcuts_page.html.js';
Expand Down Expand Up @@ -108,6 +109,10 @@ export class ShortcutsPageElement extends PolymerElement implements
onNavigationPageChanged({isActive}: {isActive: boolean}): void {
if (isActive) {
afterNextRender(this, () => {
if (this.initialData) {
getShortcutProvider().recordMainCategoryNavigation(
this.initialData.category);
}
// Dispatch a custom event to inform the parent to scroll to the top
// after active page changes.
this.dispatchEvent(new CustomEvent('scroll-to-top', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ suite('shortcutCustomizationAppTest', function() {
page = initShortcutCustomizationAppElement();
await flushTasks();

assertEquals(undefined, provider.getLatestMainCategoryNavigated());

const navPanel =
getPage().shadowRoot!.querySelector('navigation-view-panel');
const navSelector =
Expand All @@ -281,6 +283,10 @@ suite('shortcutCustomizationAppTest', function() {

await flushTasks();

assertEquals(
AcceleratorCategory.kBrowser,
provider.getLatestMainCategoryNavigated());

const actualSubsections = getSubsections(AcceleratorCategory.kBrowser);
const expectedLayouts =
getManager().getSubcategories(AcceleratorCategory.kBrowser);
Expand Down
14 changes: 14 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -100057,6 +100057,20 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
<int value="4" label="ResetAll"/>
</enum>

<enum name="ShortcutCustomizationMainCategory">
<summary>
The categories that are present within the Key Shortcuts app.
</summary>
<int value="0" label="General"/>
<int value="1" label="Device"/>
<int value="2" label="Browser"/>
<int value="3" label="Text"/>
<int value="4" label="WindowsAndDesks"/>
<int value="5" label="Accessibility"/>
<int value="6" label="Debug"/>
<int value="7" label="Developer"/>
</enum>

<enum name="ShortcutsCreationResult">
<summary>Result of creating shortcuts for PWA.</summary>
<int value="0" label="Success"/>
Expand Down
10 changes: 10 additions & 0 deletions tools/metrics/histograms/metadata/ash/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6294,6 +6294,16 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Ash.ShortcutCustomization.MainCategoryNavigation"
enum="ShortcutCustomizationMainCategory" expires_after="2024-08-17">
<owner>jimmyxgong@chromium.org</owner>
<owner>cros-peripherals@google.com</owner>
<summary>
Tracks the number of times a user navigates to a main category in the side
panel of the Key Shortcuts app.
</summary>
</histogram>

<histogram
name="Ash.ShortcutCustomization.RemoveDefaultAccelerator.{ActionName}"
units="Shortcuts" expires_after="2024-08-17">
Expand Down

0 comments on commit f413d4f

Please sign in to comment.