Report failure when ExtensionSettings override the forced list

We have many extensions stuck in CREATED stage which is reported
in ExtensionManagement::UpdateForcedExtensions when extension list is
fetched from kInstallForceList pref. We add the extensions to
|settings_by_id_| dictionary. This dictionary may be overridden by
the entries in kExtensionManagement pref and it is possible that
the extensions which are listed in forced list may be listed
in kExtensionManagement pref as "allowed" or "blocked". In these
cases, we might not install the extension as ExtensionSettings policy
overrides the policy for force installed extensions.

This CL adds a new failure reason which is reported when extension
listed in forced list policy has its installation mode
overridden by ExtensionSettings policy to something else.

Bug: 989526
Change-Id: I20995df211d5e09492c35c09e7c5152deba2fdd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390181
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Reviewed-by: Saurabh Nijhara <snijhara@google.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805702}

chrome/browser/chromeos/policy/extension_install_event_log_collector.cc

@@ -124,6 +124,8 @@ em::ExtensionInstallReportLogEvent_FailureReason ConvertFailureReasonToProto(
       return em::ExtensionInstallReportLogEvent::CRX_FETCH_URL_EMPTY;
     case extensions::InstallStageTracker::FailureReason::CRX_FETCH_URL_INVALID:
       return em::ExtensionInstallReportLogEvent::CRX_FETCH_URL_INVALID;
+    case extensions::InstallStageTracker::FailureReason::OVERRIDDEN_BY_SETTINGS:
+      return em::ExtensionInstallReportLogEvent::OVERRIDDEN_BY_SETTINGS;
     default:
       NOTREACHED();
   }

chrome/browser/extensions/extension_management.cc

@@ -502,6 +502,8 @@ void ExtensionManagement::Refresh() {
             continue;
           }
           internal::IndividualSettings* by_id = AccessById(extension_id);
+          const bool included_in_forcelist =
+              by_id->installation_mode == InstallationMode::INSTALLATION_FORCED;
           if (!by_id->Parse(subdict,
                             internal::IndividualSettings::SCOPE_INDIVIDUAL)) {
             settings_by_id_.erase(extension_id);
@@ -511,6 +513,16 @@ void ExtensionManagement::Refresh() {
             SYSLOG(WARNING) << "Malformed Extension Management settings for "
                             << extension_id << ".";
           }
+          // If applying the ExtensionSettings policy changes installation mode
+          // from force-installed to anything else, the extension might not get
+          // installed and will get stuck in CREATED stage.
+          if (included_in_forcelist &&
+              by_id->installation_mode !=
+                  InstallationMode::INSTALLATION_FORCED) {
+            install_stage_tracker->ReportFailure(
+                extension_id,
+                InstallStageTracker::FailureReason::OVERRIDDEN_BY_SETTINGS);
+          }
         }
       }
     }

chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc

@@ -193,6 +193,18 @@ class ForceInstalledMetricsTest : public testing::Test,
     base::RunLoop().RunUntilIdle();
   }
 
+  void SetupExtensionManagementPref() {
+    std::unique_ptr<base::DictionaryValue> extension_entry =
+        DictionaryBuilder()
+            .Set("installation_mode", "allowed")
+            .Set(ExternalProviderImpl::kExternalUpdateUrl, kExtensionUpdateUrl)
+            .Build();
+    prefs_->SetManagedPref(pref_names::kExtensionManagement,
+                           DictionaryBuilder()
+                               .Set(kExtensionId1, std::move(extension_entry))
+                               .Build());
+  }
+
   void SetupEmptyForceList() {
     std::unique_ptr<base::Value> dict = DictionaryBuilder().Build();
     prefs_->SetManagedPref(pref_names::kInstallForceList, std::move(dict));
@@ -299,6 +311,23 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsInstalled) {
   EXPECT_EQ(1u, ready_call_count_);
 }
 
+// Verifies that failure is reported for the extensions which are listed in
+// forced list, and their installation mode are overridden by ExtensionSettings
+// policy to something else.
+TEST_F(ForceInstalledMetricsTest, ExtensionSettingsOverrideForcedList) {
+  SetupForceList();
+  SetupExtensionManagementPref();
+  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
+  tracker_->OnExtensionLoaded(profile_, ext2.get());
+  // ForceInstalledMetrics shuts down timer because all extension are either
+  // loaded or failed.
+  EXPECT_FALSE(fake_timer_->IsRunning());
+  EXPECT_EQ(1u, loaded_call_count_);
+  histogram_tester_.ExpectBucketCount(
+      kFailureReasonsCWS,
+      InstallStageTracker::FailureReason::OVERRIDDEN_BY_SETTINGS, 1);
+}
+
 TEST_F(ForceInstalledMetricsTest, ObserversOnlyCalledOnce) {
   // Start with a non-empty force-list, and install them, which triggers
   // observer.

chrome/browser/extensions/forced_extensions/install_stage_tracker.h

@@ -184,9 +184,13 @@ class InstallStageTracker : public KeyedService {
     // The download of the crx failed.
     CRX_FETCH_URL_INVALID = 26,
 
+    // Applying the ExtensionSettings policy changed installation mode from
+    // force-installed to anything else.
+    OVERRIDDEN_BY_SETTINGS = 27,
+
     // Magic constant used by the histogram macros.
     // Always update it to the max value.
-    kMaxValue = CRX_FETCH_URL_INVALID,
+    kMaxValue = OVERRIDDEN_BY_SETTINGS,
   };
 
   // Status for the app returned by server while fetching manifest when status

components/policy/proto/device_management_backend.proto

@@ -2926,6 +2926,8 @@ message ExtensionInstallReportLogEvent {
     CRX_FETCH_URL_EMPTY = 25;
 
     CRX_FETCH_URL_INVALID = 26;
+
+    OVERRIDDEN_BY_SETTINGS = 27;
   }
 
   //  Stage of extension installing process. See InstallStageTracker::Stage for

tools/metrics/histograms/enums.xml

@@ -24623,6 +24623,7 @@ Called by update_extension_histograms.py.-->
   <int value="24" label="IN_PROGRESS"/>
   <int value="25" label="CRX_FETCH_URL_EMPTY"/>
   <int value="26" label="CRX_FETCH_URL_INVALID"/>
+  <int value="27" label="OVERRIDEN_BY_SETTINGS"/>
 </enum>
 
 <enum name="ExtensionInstallationStage">