shortcuts: Record subactions for adding and editing an accelerator

Provides insights on how users add or edit their accelerators.

Bug: b/216049298
Test: browser_test, ash_webui_unittests
Change-Id: Ie6f19422911eccd11f8ebedbf5b23e41a386aa7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4973617
Reviewed-by: David Padlipsky <dpad@google.com>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216506}

ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.cc

@@ -1210,6 +1210,15 @@ void AcceleratorConfigurationProvider::RecordEditDialogCompletedActions(
       completed_actions);
 }
 
+void AcceleratorConfigurationProvider::RecordAddOrEditSubactions(
+    bool is_add,
+    shortcut_customization::mojom::Subactions subactions) {
+  const std::string histogram_name =
+      is_add ? "Ash.ShortcutCustomization.AddAcceleratorSubactions"
+             : "Ash.ShortcutCustomization.EditAcceleratorSubactions";
+  base::UmaHistogramEnumeration(histogram_name, subactions);
+}
+
 void AcceleratorConfigurationProvider::BindInterface(
     mojo::PendingReceiver<
         shortcut_customization::mojom::AcceleratorConfigurationProvider>

ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.h

@@ -140,6 +140,9 @@ class AcceleratorConfigurationProvider
       shortcut_customization::mojom::UserAction user_action) override;
   void RecordMainCategoryNavigation(
       mojom::AcceleratorCategory category) override;
+  void RecordAddOrEditSubactions(
+      bool is_add,
+      shortcut_customization::mojom::Subactions subactions) override;
 
   void RecordEditDialogCompletedActions(
       shortcut_customization::mojom::EditDialogCompletedActions

ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider_unittest.cc

@@ -65,6 +65,7 @@ using shortcut_customization::mojom::AcceleratorResultDataPtr;
 using shortcut_customization::mojom::EditDialogCompletedActions;
 using shortcut_customization::mojom::SimpleAccelerator;
 using shortcut_customization::mojom::SimpleAcceleratorPtr;
+using shortcut_customization::mojom::Subactions;
 using shortcut_customization::mojom::UserAction;
 
 using mojom::AcceleratorConfigResult;
@@ -3271,6 +3272,43 @@ TEST_F(AcceleratorConfigurationProviderTest, MainNavigationCategoryMetrics) {
       mojom::AcceleratorCategory::kAccessibility, 1);
 }
 
+TEST_F(AcceleratorConfigurationProviderTest, SubactionsMetrics) {
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.AddAcceleratorSubactions",
+      Subactions::kErrorCancel, 0);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.AddAcceleratorSubactions",
+      Subactions::kNoErrorSuccess, 0);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.EditAcceleratorSubactions",
+      Subactions::kErrorCancel, 0);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.EditAcceleratorSubactions",
+      Subactions::kNoErrorSuccess, 0);
+
+  provider_->RecordAddOrEditSubactions(/*is_add=*/true,
+                                       Subactions::kErrorCancel);
+  provider_->RecordAddOrEditSubactions(/*is_add=*/true,
+                                       Subactions::kNoErrorSuccess);
+  provider_->RecordAddOrEditSubactions(/*is_add=*/false,
+                                       Subactions::kErrorCancel);
+  provider_->RecordAddOrEditSubactions(/*is_add=*/false,
+                                       Subactions::kNoErrorSuccess);
+
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.AddAcceleratorSubactions",
+      Subactions::kErrorCancel, 1);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.AddAcceleratorSubactions",
+      Subactions::kNoErrorSuccess, 1);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.EditAcceleratorSubactions",
+      Subactions::kErrorCancel, 1);
+  histogram_tester_->ExpectBucketCount(
+      "Ash.ShortcutCustomization.EditAcceleratorSubactions",
+      Subactions::kNoErrorSuccess, 1);
+}
+
 TEST_F(AcceleratorConfigurationProviderTest,
        VerifyAllAcceleratorsHaveKeyString) {
   // The following is a set of VKEYs that are ignored in this test. If there is

ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom

@@ -73,6 +73,17 @@ enum EditDialogCompletedActions {
   kResetRemoveEditAdd,
 };
 
+// Enum of possible subactions done during either adding or editing an
+// accelerator.
+// Important: Do not change the ordering of this enum, these are used directly
+// in metrics.
+enum Subactions {
+  kNoErrorCancel,
+  kNoErrorSuccess,
+  kErrorCancel,
+  kErrorSuccess,
+};
+
 // Observer interface, to be implemented by the Shortcut Customization SWA to
 // receive updated accelerators.
 interface AcceleratorsUpdatedObserver {
@@ -201,4 +212,7 @@ interface AcceleratorConfigurationProvider {
   // closed.
   RecordEditDialogCompletedActions(
       EditDialogCompletedActions completed_actions);
+
+  // Records the subactions done when adding or editing an accelerator.
+  RecordAddOrEditSubactions(bool is_add, Subactions subactions);
 };

ash/webui/shortcut_customization_ui/resources/js/accelerator_edit_view.html

@@ -109,6 +109,7 @@
     <accelerator-view id="acceleratorItem"
         accelerator-info="[[acceleratorInfo]]" view-state="{{viewState}}"
         status-message="{{statusMessage}}" has-error="{{hasError}}"
+        recorded-error="{{recordedError}}"
         action="[[action]]" source="[[source]]">
     </accelerator-view>
   </div>

ash/webui/shortcut_customization_ui/resources/js/accelerator_edit_view.ts

@@ -16,7 +16,7 @@ import {String16} from 'chrome://resources/mojo/mojo/public/mojom/base/string16.
 import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/interfaces.js';
 import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
+import {Subactions, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
 
 import {getTemplate} from './accelerator_edit_view.html.js';
 import {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
@@ -100,6 +100,13 @@ export class AcceleratorEditViewElement extends AcceleratorEditViewElementBase {
         reflectToAttribute: true,
       },
 
+      // Keeps track if there was ever an error when interacting with this
+      // accelerator.
+      recordedError: {
+        type: Boolean,
+        value: false,
+      },
+
       action: {
         type: Number,
         value: 0,
@@ -116,6 +123,7 @@ export class AcceleratorEditViewElement extends AcceleratorEditViewElementBase {
   isEditView: boolean;
   viewState: number;
   hasError: boolean;
+  recordedError: boolean;
   action: number;
   source: AcceleratorSource;
   restoreDefaultHasError: boolean;
@@ -216,6 +224,10 @@ export class AcceleratorEditViewElement extends AcceleratorEditViewElementBase {
   }
 
   protected onCancelButtonClicked(): void {
+    this.shortcutProvider.recordAddOrEditSubactions(
+        this.viewState === ViewState.ADD,
+        this.recordedError ? Subactions.kErrorCancel :
+                             Subactions.kNoErrorCancel);
     this.cancelButtonClicked = true;
     this.endCapture();
   }

ash/webui/shortcut_customization_ui/resources/js/accelerator_view.ts

@@ -15,7 +15,7 @@ import {String16} from 'chrome://resources/mojo/mojo/public/mojom/base/string16.
 import {PolymerElementProperties} from 'chrome://resources/polymer/v3_0/polymer/interfaces.js';
 import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {AcceleratorResultData, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
+import {AcceleratorResultData, Subactions, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
 
 import {AcceleratorLookupManager} from './accelerator_lookup_manager.js';
 import {getTemplate} from './accelerator_view.html.js';
@@ -98,6 +98,15 @@ export class AcceleratorViewElement extends AcceleratorViewElementBase {
         type: Boolean,
         value: false,
         notify: true,
+        observer: AcceleratorViewElement.prototype.onErrorUpdated,
+      },
+
+      // Keeps track if there was ever an error when interacting with this
+      // accelerator.
+      recordedError: {
+        type: Boolean,
+        value: false,
+        notify: true,
       },
 
       action: {
@@ -141,6 +150,7 @@ export class AcceleratorViewElement extends AcceleratorViewElementBase {
   viewState: ViewState;
   statusMessage: string;
   hasError: boolean;
+  recordedError: boolean;
   action: number;
   source: AcceleratorSource;
   sourceIsLocked: boolean;
@@ -463,6 +473,10 @@ export class AcceleratorViewElement extends AcceleratorViewElementBase {
       }
       case AcceleratorConfigResult.kSuccess: {
         this.fireEditCompletedActionEvent(this.editAction);
+        getShortcutProvider().recordAddOrEditSubactions(
+            this.viewState === ViewState.ADD,
+            this.recordedError ? Subactions.kErrorSuccess :
+                                 Subactions.kNoErrorSuccess);
         getShortcutProvider().recordUserAction(
             UserAction.kSuccessfulModification);
         const message = (this.viewState == ViewState.ADD) ?
@@ -730,6 +744,14 @@ export class AcceleratorViewElement extends AcceleratorViewElementBase {
     return this.acceleratorInfo.state === AcceleratorState.kDisabledByUser ||
         this.acceleratorInfo.state === AcceleratorState.kDisabledByConflict;
   }
+
+  private onErrorUpdated(): void {
+    // `recordedError` will only update if it was previously false and
+    // an error has been detected.
+    if (!this.recordedError && this.hasError) {
+      this.recordedError = true;
+    }
+  }
 }
 
 declare global {

ash/webui/shortcut_customization_ui/resources/js/fake_shortcut_provider.ts

@@ -6,7 +6,7 @@ import {FakeMethodResolver} from 'chrome://resources/ash/common/fake_method_reso
 import {FakeObservables} from 'chrome://resources/ash/common/fake_observables.js';
 import {assert} from 'chrome://resources/js/assert.js';
 
-import {AcceleratorResultData, AcceleratorsUpdatedObserverRemote, EditDialogCompletedActions, PolicyUpdatedObserverRemote, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
+import {AcceleratorResultData, AcceleratorsUpdatedObserverRemote, EditDialogCompletedActions, PolicyUpdatedObserverRemote, Subactions, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
 
 import {Accelerator, AcceleratorCategory, AcceleratorConfigResult, AcceleratorSource, MojoAcceleratorConfig, MojoLayoutInfo, ShortcutProviderInterface} from './shortcut_types.js';
 
@@ -36,6 +36,8 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
   private lastRecordedUserAction: UserAction;
   private lastRecordedMainCategory: AcceleratorCategory;
   private lastRecoredEditDialogActions: EditDialogCompletedActions;
+  private lastRecordedIsAdd: boolean = false;
+  private lastRecorededSubactions: Subactions;
 
   constructor() {
     this.methods = new FakeMethodResolver();
@@ -59,6 +61,7 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
     this.methods.register('recordUserAction');
     this.methods.register('recordMainCategoryNavigation');
     this.methods.register('recordEditDialogCompetedActions');
+    this.methods.register('recordAddOrEditSubactions');
     this.registerObservables();
   }
 
@@ -200,6 +203,19 @@ export class FakeShortcutProvider implements ShortcutProviderInterface {
     return this.lastRecordedMainCategory;
   }
 
+  recordAddOrEditSubactions(isAdd: boolean, subactions: Subactions): void {
+    this.lastRecordedIsAdd = isAdd;
+    this.lastRecorededSubactions = subactions;
+  }
+
+  getLastRecordedIsAdd(): boolean {
+    return this.lastRecordedIsAdd;
+  }
+
+  getLastRecordedSubactions(): Subactions {
+    return this.lastRecorededSubactions;
+  }
+
   preventProcessingAccelerators(_preventProcessingAccelerators: boolean):
       Promise<void> {
     ++this.preventProcessingAcceleratorsCallCount;

ash/webui/shortcut_customization_ui/resources/js/mojo_interface_provider.ts

@@ -4,7 +4,7 @@
 
 import {assert} from 'chrome://resources/js/assert.js';
 
-import {AcceleratorConfigurationProvider, AcceleratorConfigurationProviderRemote, AcceleratorResultData, AcceleratorsUpdatedObserverRemote, EditDialogCompletedActions, PolicyUpdatedObserverRemote, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
+import {AcceleratorConfigurationProvider, AcceleratorConfigurationProviderRemote, AcceleratorResultData, AcceleratorsUpdatedObserverRemote, EditDialogCompletedActions, PolicyUpdatedObserverRemote, Subactions, UserAction} from '../mojom-webui/ash/webui/shortcut_customization_ui/mojom/shortcut_customization.mojom-webui.js';
 
 import {fakeAcceleratorConfig, fakeLayoutInfo} from './fake_data.js';
 import {FakeShortcutProvider} from './fake_shortcut_provider.js';
@@ -154,6 +154,10 @@ export class ShortcutProviderWrapper implements ShortcutProviderInterface {
                                        EditDialogCompletedActions): void {
     this.remote.recordEditDialogCompletedActions(completed_actions);
   }
+
+  recordAddOrEditSubactions(isAdd: boolean, subactions: Subactions): void {
+    this.remote.recordAddOrEditSubactions(isAdd, subactions);
+  }
 }
 
 export function getShortcutProvider(): ShortcutProviderInterface {