Add more known misconfiguration failures occurred in case of force

installed extensions

Some failures occur due to NOT_PERORMING_NEW_INSTALL error. The reason
for this is a deprecated extension which should not be used, thus it is
a misconfiguration from admin.

Some failures occur due to REPLACED_BY_ARC_APP error. If a device
doesn't support ARC++, the app will be installed, otherwise the ARC++
app will be used and this one will fail as expected. This error is
considered a misconfiguration only if ARC++ is enabled for the
profile.

Bug: 1067605, 1067606
Change-Id: I8001c70c5b13a3aa1c06c4c3ab4549dfe1e04eda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144027
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760047}

chrome/browser/extensions/forced_extensions/installation_metrics.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/extensions/extension_management_constants.h"
 #include "chrome/browser/extensions/forced_extensions/installation_reporter.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/prefs/pref_service.h"
 #include "extensions/browser/disable_reason.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/install/crx_install_error.h"
@@ -20,6 +21,7 @@
 #include "extensions/browser/updater/extension_downloader_delegate.h"
 
 #if defined(OS_CHROMEOS)
+#include "components/arc/arc_prefs.h"
 #include "components/user_manager/user_manager.h"
 #endif  // defined(OS_CHROMEOS)
 
@@ -88,17 +90,33 @@ InstallationMetrics::~InstallationMetrics() = default;
 bool InstallationMetrics::IsMisconfiguration(
     const InstallationReporter::InstallationData& installation_data,
     const ExtensionId& id) {
-  ExtensionManagement* management =
-      ExtensionManagementFactory::GetForBrowserContext(profile_);
-  CrxInstallErrorDetail detail = installation_data.install_error_detail.value();
-  if (detail == CrxInstallErrorDetail::KIOSK_MODE_ONLY)
+  if (installation_data.install_error_detail) {
+    ExtensionManagement* management =
+        ExtensionManagementFactory::GetForBrowserContext(profile_);
+    CrxInstallErrorDetail detail =
+        installation_data.install_error_detail.value();
+    if (detail == CrxInstallErrorDetail::KIOSK_MODE_ONLY)
+      return true;
+
+    if (detail == CrxInstallErrorDetail::DISALLOWED_BY_POLICY &&
+        !management->IsAllowedManifestType(
+            installation_data.extension_type.value(), id)) {
+      return true;
+    }
+  }
+#if defined(OS_CHROMEOS)
+  // REPLACED_BY_ARC_APP error is a misconfiguration if it ARC++ is enabled for
+  // the device.
+  if (profile_->GetPrefs()->IsManagedPreference(arc::prefs::kArcEnabled) &&
+      profile_->GetPrefs()->GetBoolean(arc::prefs::kArcEnabled) &&
+      installation_data.failure_reason ==
+          InstallationReporter::FailureReason::REPLACED_BY_ARC_APP)
     return true;
+#endif  // defined(OS_CHROMEOS)
 
-  if (detail == CrxInstallErrorDetail::DISALLOWED_BY_POLICY &&
-      !management->IsAllowedManifestType(
-          installation_data.extension_type.value(), id)) {
+  if (installation_data.failure_reason ==
+      InstallationReporter::FailureReason::NOT_PERFORMING_NEW_INSTALL)
     return true;
-  }
 
   return false;
 }
@@ -184,6 +202,8 @@ void InstallationMetrics::ReportMetrics() {
             "Extensions.ForceInstalledDownloadingStage", downloading_stage);
       }
     }
+    if (IsMisconfiguration(installation, extension_id))
+      misconfigured_extensions++;
     InstallationReporter::FailureReason failure_reason =
         installation.failure_reason.value_or(
             InstallationReporter::FailureReason::UNKNOWN);
@@ -245,8 +265,6 @@ void InstallationMetrics::ReportMetrics() {
             << InstallationReporter::GetFormattedInstallationData(installation);
     if (installation.install_error_detail) {
       CrxInstallErrorDetail detail = installation.install_error_detail.value();
-      if (IsMisconfiguration(installation, extension_id))
-        misconfigured_extensions++;
       base::UmaHistogramEnumeration(
           "Extensions.ForceInstalledFailureCrxInstallError", detail);
     }

chrome/browser/extensions/forced_extensions/installation_tracker_unittest.cc

@@ -28,6 +28,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_CHROMEOS)
+#include "components/arc/arc_prefs.h"
 #include "components/user_manager/fake_user_manager.h"
 #include "components/user_manager/scoped_user_manager.h"
 #include "components/user_manager/user_names.h"
@@ -618,6 +619,70 @@ TEST_F(ForcedExtensionsInstallationTrackerTest,
                                       1);
 }
 
+#if defined(OS_CHROMEOS)
+// Session in which either all the extensions installed successfully, or all
+// failures are admin-side misconfigurations. This test verifies that failure
+// REPLACED_BY_ARC_APP is not considered as misconfiguration when ARC++ is
+// enabled for the profile.
+TEST_F(ForcedExtensionsInstallationTrackerTest,
+       NonMisconfigurationFailureNotPresentReplacedByArcAppErrorArcEnabled) {
+  // Enable ARC++ for this profile.
+  prefs_->SetManagedPref(arc::prefs::kArcEnabled,
+                         std::make_unique<base::Value>(true));
+  SetupForceList();
+  auto extension =
+      ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(&profile_, extension.get());
+  installation_reporter_->ReportFailure(
+      kExtensionId2, InstallationReporter::FailureReason::REPLACED_BY_ARC_APP);
+  // InstallationTracker shuts down timer because all extension are either
+  // loaded or failed.
+  EXPECT_FALSE(fake_timer_->IsRunning());
+  histogram_tester_.ExpectBucketCount(kPossibleNonMisconfigurationFailures, 0,
+                                      1);
+}
+
+// Session in which at least one non misconfiguration failure occurred. This
+// test verifies that failure REPLACED_BY_ARC_APP is not considered as
+// misconfiguration when ARC++ is disabled for the profile.
+TEST_F(ForcedExtensionsInstallationTrackerTest,
+       NonMisconfigurationFailureNotPresentReplacedByArcAppErrorArcDisabled) {
+  // Enable ARC++ for this profile.
+  prefs_->SetManagedPref(arc::prefs::kArcEnabled,
+                         std::make_unique<base::Value>(false));
+  SetupForceList();
+  auto extension =
+      ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(&profile_, extension.get());
+  installation_reporter_->ReportFailure(
+      kExtensionId2, InstallationReporter::FailureReason::REPLACED_BY_ARC_APP);
+  // InstallationTracker shuts down timer because all extension are either
+  // loaded or failed.
+  EXPECT_FALSE(fake_timer_->IsRunning());
+  histogram_tester_.ExpectBucketCount(kPossibleNonMisconfigurationFailures, 1,
+                                      1);
+}
+#endif  // defined(OS_CHROMEOS)
+
+// Session in which either all the extensions installed successfully, or all
+// failures are admin-side misconfigurations. This test verifies that failure
+// NOT_PERFORMING_NEW_INSTALL is considered as misconfiguration.
+TEST_F(ForcedExtensionsInstallationTrackerTest,
+       NonMisconfigurationFailureNotPresentNotPerformingNewInstallError) {
+  SetupForceList();
+  auto extension =
+      ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(&profile_, extension.get());
+  installation_reporter_->ReportFailure(
+      kExtensionId2,
+      InstallationReporter::FailureReason::NOT_PERFORMING_NEW_INSTALL);
+  // InstallationTracker shuts down timer because all extension are either
+  // loaded or failed.
+  EXPECT_FALSE(fake_timer_->IsRunning());
+  histogram_tester_.ExpectBucketCount(kPossibleNonMisconfigurationFailures, 0,
+                                      1);
+}
+
 TEST_F(ForcedExtensionsInstallationTrackerTest, NoExtensionsConfigured) {
   EXPECT_TRUE(fake_timer_->IsRunning());
   fake_timer_->Fire();