Fix Supervised User extension install metrics on "Skip Approval"-mode

The metrics for the Extension Installation dialog (the same dialog
that is used by non-supervised users) should be recorded for supervised
user when the "Skip parent approval"-mode is enabled.

This change re-enables observing the states of the
Extension Installation dialog dialog for supervised users,
when parental controls apply on Extensions.
The deprecated state "Request parent approval" is cleaned up
for where is was previously tracked. The state was only
meaningful for an old use-case of the dialog, which no longer exists.
A new state ("Child Accepted") is introduced instead and tracked
when the child proceeds with an extension installation in the
"Skip parent approval"-mode.

Bug: b/321240396, b/349322029
Change-Id: Iadbd2ef4062475be83053536f728db7e85bdc260
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5645669
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Anthi Orfanou <anthie@google.com>
Reviewed-by: Courtney Wong <courtneywong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1320974}
diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
index cfa152e..89229ce 100644
--- a/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
+++ b/chrome/browser/extensions/api/webstore_private/webstore_private_api.cc
@@ -894,8 +894,14 @@
     // to configure the install prompt to indicate that this is a child
     // asking a parent for installation permission.
     prompt->set_requires_parent_permission(requires_parent_permission);
-    if (requires_parent_permission) {
+    // Record metrics for supervised users that are in "Skip parent approval"-mode
+    // and use the Extension install dialog (that is used by non-supervised
+    // users).
+    if (supervised_user::AreExtensionsPermissionsEnabled(
+            *profile_->GetPrefs())) {
       prompt->AddObserver(&supervised_user_extensions_metrics_recorder_);
+    }
+    if (requires_parent_permission) {
       // Bypass the install prompt dialog if V2 is enabled. The
       // ParentAccessDialog handles both the blocked and install use case.
 #if BUILDFLAG(IS_CHROMEOS)
diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
index e5b6c5a..8be8233d 100644
--- a/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
+++ b/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
@@ -491,6 +491,7 @@
 // Tests install for a child when parent permission is granted.
 IN_PROC_BROWSER_TEST_P(SupervisedUserExtensionWebstorePrivateApiTest,
                        ParentPermissionGranted) {
+  base::UserActionTester user_action_tester;
   WebstoreInstallListener listener;
   auto delegate_reset = WebstorePrivateApi::SetDelegateForTesting(&listener);
   set_next_dialog_action(NextDialogAction::kAccept);
@@ -505,13 +506,26 @@
           .SetID(kTestAppId)
           .SetVersion(kTestAppVersion)
           .Build();
-  ASSERT_TRUE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+  EXPECT_TRUE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+  EXPECT_EQ(1, user_action_tester.GetActionCount(
+                   SupervisedUserExtensionsMetricsRecorder::
+                       kParentPermissionDialogOpenedActionName));
+  EXPECT_EQ(1, user_action_tester.GetActionCount(
+                   SupervisedUserExtensionsMetricsRecorder::
+                       kParentPermissionDialogParentApprovedActionName));
+  EXPECT_EQ(
+      1,
+      user_action_tester.GetActionCount(
+          SupervisedUserExtensionsMetricsRecorder::kApprovalGrantedActionName));
+#endif  // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
 }
 
 // Tests no install occurs for a child when the parent permission
 // dialog is canceled.
 IN_PROC_BROWSER_TEST_P(SupervisedUserExtensionWebstorePrivateApiTest,
                        ParentPermissionCanceled) {
+  base::UserActionTester user_action_tester;
   WebstoreInstallListener listener;
   set_next_dialog_action(NextDialogAction::kCancel);
   auto delegate_reset = WebstorePrivateApi::SetDelegateForTesting(&listener);
@@ -527,7 +541,20 @@
           .SetID(kTestAppId)
           .SetVersion(kTestAppVersion)
           .Build();
-  ASSERT_FALSE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+  EXPECT_FALSE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+// On the default configuration only the Parent approval dialog is used for
+// extension installations.
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+  EXPECT_EQ(1, user_action_tester.GetActionCount(
+                   SupervisedUserExtensionsMetricsRecorder::
+                       kParentPermissionDialogOpenedActionName));
+  EXPECT_EQ(1, user_action_tester.GetActionCount(
+                   SupervisedUserExtensionsMetricsRecorder::
+                       kParentPermissionDialogParentCanceledActionName));
+  EXPECT_EQ(0, user_action_tester.GetActionCount(
+                   SupervisedUserExtensionsMetricsRecorder::
+                       kExtensionInstallDialogChildCanceledActionName));
+#endif  // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
 }
 
 // Tests that no parent permission is required for a child to install a theme.
@@ -604,6 +631,7 @@
 // via the "Permissions" toggle, the parent approval is required.
 IN_PROC_BROWSER_TEST_P(SupervisedUserExtensionWebstorePrivateApiTest,
                        InstallSuccessfulWhenExtensionsToggleOn) {
+  base::UserActionTester user_action_tester;
   WebstoreInstallListener listener;
   auto delegate_reset = WebstorePrivateApi::SetDelegateForTesting(&listener);
 
@@ -624,7 +652,7 @@
   ASSERT_TRUE(listener.received_success());
   ASSERT_EQ(kTestAppId, listener.id());
 
-  ASSERT_EQ(GetParam() == SupervisedUserExtensionManagedBySwitch::kPermissions,
+  EXPECT_EQ(GetParam() == SupervisedUserExtensionManagedBySwitch::kPermissions,
             parent_permission_dialog_appeared_);
 
   scoped_refptr<const Extension> extension =
@@ -632,7 +660,33 @@
           .SetID(kTestAppId)
           .SetVersion(kTestAppVersion)
           .Build();
-  ASSERT_TRUE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+  EXPECT_TRUE(extensions_delegate_->IsExtensionAllowedByParent(*extension));
+
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+  int parent_approval_dialog_count =
+      GetParam() == SupervisedUserExtensionManagedBySwitch::kExtensions ? 0 : 1;
+  // Parent Approval dialog metrics (when managed by Permissions toggle):
+  EXPECT_EQ(parent_approval_dialog_count,
+            user_action_tester.GetActionCount(
+                SupervisedUserExtensionsMetricsRecorder::
+                    kParentPermissionDialogOpenedActionName));
+  EXPECT_EQ(
+      parent_approval_dialog_count,
+      user_action_tester.GetActionCount(
+          SupervisedUserExtensionsMetricsRecorder::kApprovalGrantedActionName));
+#endif  // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
+
+  // Extension Installation dialog metrics (when managed by Extensions toggle):
+  int extension_install_dialog_count =
+      GetParam() == SupervisedUserExtensionManagedBySwitch::kExtensions ? 1 : 0;
+  EXPECT_EQ(extension_install_dialog_count,
+            user_action_tester.GetActionCount(
+                SupervisedUserExtensionsMetricsRecorder::
+                    kExtensionInstallDialogOpenedActionName));
+  EXPECT_EQ(extension_install_dialog_count,
+            user_action_tester.GetActionCount(
+                SupervisedUserExtensionsMetricsRecorder::
+                    kApprovalGrantedByDefaultName));
 }
 
 INSTANTIATE_TEST_SUITE_P(
diff --git a/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.cc b/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.cc
index d75bcee5..46cbda9 100644
--- a/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.cc
+++ b/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.cc
@@ -6,7 +6,10 @@
 
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/user_metrics.h"
+#include "base/notreached.h"
 #include "chrome/browser/ui/supervised_user/parent_permission_dialog.h"
+#include "components/supervised_user/core/browser/supervised_user_preferences.h"
+#include "components/supervised_user/core/common/features.h"
 
 // static
 const char SupervisedUserExtensionsMetricsRecorder::kExtensionsHistogramName[] =
@@ -34,11 +37,13 @@
     kExtensionInstallDialogOpenedActionName[] =
         "SupervisedUsers_Extensions_ExtensionInstallDialog_Opened";
 const char SupervisedUserExtensionsMetricsRecorder::
-    kExtensionInstallDialogAskedParentActionName[] =
-        "SupervisedUsers_Extensions_ExtensionInstallDialog_AskedParent";
-const char SupervisedUserExtensionsMetricsRecorder::
     kExtensionInstallDialogChildCanceledActionName[] =
         "SupervisedUsers_Extensions_ExtensionInstallDialog_ChildCanceled";
+const char SupervisedUserExtensionsMetricsRecorder::
+    kExtensionInstallDialogChildAcceptedActionName[] =
+        "SupervisedUsers_Extensions_ExtensionInstallDialog_"
+        "ChildAccepted";
+
 // Parent Permission Dialog.
 const char SupervisedUserExtensionsMetricsRecorder::
     kParentPermissionDialogHistogramName[] =
@@ -82,7 +87,7 @@
 
 void SupervisedUserExtensionsMetricsRecorder::OnDialogAccepted() {
   RecordExtensionInstallDialogUmaMetrics(
-      ExtensionInstallDialogState::kAskedParent);
+      ExtensionInstallDialogState::kChildAccepted);
 }
 
 void SupervisedUserExtensionsMetricsRecorder::OnDialogCanceled() {
@@ -129,14 +134,16 @@
       base::RecordAction(
           base::UserMetricsAction(kExtensionInstallDialogOpenedActionName));
       break;
-    case ExtensionInstallDialogState::kAskedParent:
-      base::RecordAction(base::UserMetricsAction(
-          kExtensionInstallDialogAskedParentActionName));
-      break;
+    case ExtensionInstallDialogState::kAskedParentDeprecated:
+      NOTREACHED_NORETURN();
     case ExtensionInstallDialogState::kChildCanceled:
       base::RecordAction(base::UserMetricsAction(
           kExtensionInstallDialogChildCanceledActionName));
       break;
+    case ExtensionInstallDialogState::kChildAccepted:
+      base::RecordAction(base::UserMetricsAction(
+          kExtensionInstallDialogChildAcceptedActionName));
+      break;
   }
 }
 
diff --git a/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h b/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h
index 8f1067ba..955ae35d 100644
--- a/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h
+++ b/chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h
@@ -43,22 +43,28 @@
   // These enum values represent the state of the Extension Install Dialog for
   // installing and enabling extensions for supervised users.
   // These values are logged to UMA. Entries should not be renumbered and
-  // numeric values should never be reused. Please keep in sync with
-  // "SupervisedUserExtensionInstallDialog" in
-  // src/tools/metrics/histograms/enums.xml.
+  // numeric values should never be reused.
+  //
+  // LINT.IfChange(ExtensionInstallDialogState)
   enum class ExtensionInstallDialogState {
     // Recorded when the extension install dialog opens.
     kOpened = 0,
     // Recorded when the child clicks "Ask a parent".
-    kAskedParent = 1,
+    // Deprecated, the Extension install dialog does not display this button
+    // anymore.
+    // It was used in ChromeOS v1 extension installation flow.
+    kAskedParentDeprecated = 1,
     // Recorded when the child cancels the extension installation.
     kChildCanceled = 2,
-    // Add future entries above this comment, in sync with
-    // "SupervisedUserExtensionInstallDialog" in
-    // src/tools/metrics/histograms/enums.xml.
-    // Update kMaxValue to the last value.
-    kMaxValue = kChildCanceled
+    // Recorded when the child proceeds with the extension installation dialog.
+    // Under the Skip parent approval move, the `Accept` action is the
+    // installation of the extension without parent intervention.
+    kChildAccepted = 3,
+    // Add future entries above this comment, updating kMaxValue to the last
+    // value.
+    kMaxValue = kChildAccepted
   };
+  // LINT.ThenChange(//tools/metrics/histograms/metadata/families/enums.xml:SupervisedUserExtensionInstallDialog)
 
   // These enum values represent the state of the Parent Permission Dialog for
   // installing and enabling extensions for supervised users.
@@ -136,6 +142,8 @@
 
   // UMA metrics for adding to or removing from the set of approved extension
   // ids in the kSupervisedUserApprovedExtensions synced pref.
+  // They should be kept in sync with entries on
+  // tools/metrics/actions/actions.xml.
   static const char kExtensionsHistogramName[];
   static const char kApprovalGrantedActionName[];
   static const char kPermissionsIncreaseGrantedActionName[];
@@ -146,9 +154,8 @@
   // UMA metrics for the Extension Install Dialog.
   static const char kExtensionInstallDialogHistogramName[];
   static const char kExtensionInstallDialogOpenedActionName[];
-  static const char kExtensionInstallDialogAskedParentActionName[];
   static const char kExtensionInstallDialogChildCanceledActionName[];
-
+  static const char kExtensionInstallDialogChildAcceptedActionName[];
   // UMA metrics for the Parent Permission Dialog.
   static const char kParentPermissionDialogHistogramName[];
   static const char kParentPermissionDialogOpenedActionName[];
diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view_supervised_browsertest.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view_supervised_browsertest.cc
index a1f2189b..3ca0af56 100644
--- a/chrome/browser/ui/views/extensions/extension_install_dialog_view_supervised_browsertest.cc
+++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view_supervised_browsertest.cc
@@ -109,7 +109,7 @@
   return delegate_view;
 }
 
-IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTestSupervised, AskAParent) {
+IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTestSupervised, ChildAccepts) {
   base::HistogramTester histogram_tester;
   base::UserActionTester user_action_tester;
 
@@ -138,7 +138,7 @@
                    SupervisedUserExtensionsMetricsRecorder::
                        kExtensionInstallDialogOpenedActionName));
 
-  // Supervised user presses "Ask a parent".
+  // Supervised user presses the "Accept" button.
   ExtensionInstallDialogView::SetInstallButtonDelayForTesting(0);
   ExtensionInstallDialogView* delegate_view =
       CreateAndShowPrompt(&helper, install_prompt.GetPromptForTesting());
@@ -150,14 +150,14 @@
       SupervisedUserExtensionsMetricsRecorder::
           kExtensionInstallDialogHistogramName,
       SupervisedUserExtensionsMetricsRecorder::ExtensionInstallDialogState::
-          kAskedParent,
+          kChildAccepted,
       1);
   histogram_tester.ExpectTotalCount(SupervisedUserExtensionsMetricsRecorder::
                                         kExtensionInstallDialogHistogramName,
                                     2);
   EXPECT_EQ(1, user_action_tester.GetActionCount(
                    SupervisedUserExtensionsMetricsRecorder::
-                       kExtensionInstallDialogAskedParentActionName));
+                       kExtensionInstallDialogChildAcceptedActionName));
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTestSupervised,
diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml
index 251ed46f..5994cdda 100644
--- a/tools/metrics/actions/actions.xml
+++ b/tools/metrics/actions/actions.xml
@@ -39511,13 +39511,11 @@
   </description>
 </action>
 
-<action name="SupervisedUsers_Extensions_ExtensionInstallDialog_AskedParent">
-  <owner>tobyhuang@chromium.org</owner>
-  <owner>agawronska@chromium.org</owner>
-  <owner>danan@chromium.org</owner>
-  <owner>cros-families@google.com</owner>
+<action name="SupervisedUsers_Extensions_ExtensionInstallDialog_ChildAccepted">
+  <owner>anthie@google.com</owner>
+  <owner>chrome-kids-eng@google.com</owner>
   <description>
-    Emitted when a supervised user clicks &quot;Ask a parent&quot; on the
+    Emitted when a supervised user clicks &quot;Add Extension&quot; on the
     Extension Install Dialog.
   </description>
 </action>
diff --git a/tools/metrics/histograms/metadata/families/enums.xml b/tools/metrics/histograms/metadata/families/enums.xml
index ac67e1d..a3802d3 100644
--- a/tools/metrics/histograms/metadata/families/enums.xml
+++ b/tools/metrics/histograms/metadata/families/enums.xml
@@ -353,15 +353,20 @@
   <int value="2" label="Enable attempt failed"/>
 </enum>
 
+<!-- LINT.IfChange(SupervisedUserExtensionInstallDialog) -->
+
 <enum name="SupervisedUserExtensionInstallDialog">
   <summary>
     Tracks the actions of a supervised user on the Extension Install Dialog.
   </summary>
   <int value="0" label="Opened"/>
-  <int value="1" label="Asked Parent"/>
+  <int value="1" label="Asked Parent (Deprecated)"/>
   <int value="2" label="Child Canceled"/>
+  <int value="3" label="Child Accepted"/>
 </enum>
 
+<!-- LINT.ThenChange(//chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h:ExtensionInstallDialogState) -->
+
 <!-- LINT.IfChange(SupervisedUserExtensionParentApprovalEntryPoint) -->
 
 <enum name="SupervisedUserExtensionParentApprovalEntryPoint">