Correctly reinstall corrupt policy-installed extensions

When we manually install an extension and then add it to force
installed list, it should get updated. If the extension gets corrupted
before the Manifest::Location of the extension is updated, the
extension does not get reinstalled. This was because after updating
the location, the function OnExternalExtensionUpdateUrlFound
return false in each case. It should add the corrupted extension to
pending extensions list instead.

Bug: 1045371
Change-Id: I9af45488b64d6a7ecde89921023bd35941e1ed0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023525Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Cr-Commit-Position: refs/heads/master@{#739356}

chrome/browser/extensions/content_verifier_browsertest.cc

@@ -319,6 +319,72 @@ IN_PROC_BROWSER_TEST_F(ContentVerifierTest, PolicyCorrupted) {
   EXPECT_FALSE(reasons & disable_reason::DISABLE_CORRUPTED);
 }
 
+// Tests the case when an extension is first manually installed, then it gets
+// corrupted and then it is added to force installed list. The extension should
+// get reinstalled and should be enabled.
+IN_PROC_BROWSER_TEST_F(ContentVerifierTest,
+                       ManualInstalledExtensionGotCorruptedThenForceInstalled) {
+  ExtensionSystem* system = ExtensionSystem::Get(profile());
+  ExtensionService* service = system->extension_service();
+
+  ExtensionId kTestExtensionId("dkjgfphccejbobpbljnpjcmhmagkdoia");
+  base::FilePath crx_path =
+      test_data_dir_.AppendASCII("content_verifier/v1.crx");
+
+  const Extension* extension = InstallExtension(crx_path, 1);
+  ASSERT_TRUE(extension);
+
+  TestExtensionRegistryObserver registry_observer(
+      ExtensionRegistry::Get(profile()), kTestExtensionId);
+  // Explicitly corrupt the extension.
+  ContentVerifier* verifier = system->content_verifier();
+  verifier->VerifyFailedForTest(kTestExtensionId,
+                                ContentVerifyJob::HASH_MISMATCH);
+
+  // Make sure the extension first got disabled due to corruption.
+  EXPECT_TRUE(registry_observer.WaitForExtensionUnloaded());
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+  int reasons = prefs->GetDisableReasons(kTestExtensionId);
+  EXPECT_TRUE(reasons & disable_reason::DISABLE_CORRUPTED);
+
+  VerifierObserver verifier_observer;
+
+  // Setup fake policy and update check objects.
+  content_verifier_test::ForceInstallProvider policy(kTestExtensionId);
+  system->management_policy()->RegisterProvider(&policy);
+  auto external_provider = std::make_unique<MockExternalProvider>(
+      service, Manifest::EXTERNAL_POLICY_DOWNLOAD);
+
+  external_provider->UpdateOrAddExtension(
+      std::make_unique<ExternalInstallInfoUpdateUrl>(
+          kTestExtensionId, std::string() /* install_parameter */,
+          extension_urls::GetWebstoreUpdateUrl(),
+          Manifest::EXTERNAL_POLICY_DOWNLOAD, 0 /* creation_flags */,
+          true /* mark_acknowldged */));
+  service->AddProviderForTesting(std::move(external_provider));
+
+  service->CheckForExternalUpdates();
+  // Set our mock update client to check that the corrupt bit is set on the
+  // data structure it receives.
+  ON_CALL(update_service_, StartUpdateCheck)
+      .WillByDefault(Invoke(
+          this, &ContentVerifierTest::AssertIsCorruptBitSetOnUpdateCheck));
+
+  // Make sure the extension then got re-installed, and that after reinstall it
+  // is no longer disabled due to corruption.
+  EXPECT_TRUE(registry_observer.WaitForExtensionInstalled());
+
+  // Wait for the content verification code to finish processing the hashes.
+  if (!base::Contains(verifier_observer.completed_fetches(), kTestExtensionId))
+    verifier_observer.WaitForFetchComplete(kTestExtensionId);
+
+  reasons = prefs->GetDisableReasons(kTestExtensionId);
+  EXPECT_FALSE(reasons & disable_reason::DISABLE_CORRUPTED);
+  EXPECT_TRUE(extensions::ExtensionRegistry::Get(profile())
+                  ->enabled_extensions()
+                  .GetByID(kTestExtensionId));
+}
+
 // Tests that verification failure during navigating to an extension resource
 // correctly disables the extension.
 IN_PROC_BROWSER_TEST_F(ContentVerifierTest, VerificationFailureOnNavigate) {

chrome/browser/extensions/extension_service.cc

@@ -254,7 +254,7 @@ bool ExtensionService::OnExternalExtensionUpdateUrlFound(
                                           ExtensionRegistry::EVERYTHING),
               nullptr)) {
         int disable_reasons =
-            extension_prefs_->GetDisableReasons(extension->id());
+            extension_prefs_->GetDisableReasons(info.extension_id);
         disable_reasons &= (~(disable_reason::DISABLE_USER_ACTION |
                               disable_reason::DISABLE_PERMISSIONS_INCREASE));
         extension_prefs_->ReplaceDisableReasons(info.extension_id,
@@ -266,7 +266,15 @@ bool ExtensionService::OnExternalExtensionUpdateUrlFound(
           EnableExtension(info.extension_id);
         }
       }
-      return false;
+      // If the extension is not corrupted, it is already installed with the
+      // correct install location, so there is no need to add it to the pending
+      // set of extensions. If the extension is corrupted, it should be
+      // reinstalled, thus it should be added to the pending extensions for
+      // installation.
+      if (!pending_extension_manager_.IsPolicyReinstallForCorruptionExpected(
+              info.extension_id)) {
+        return false;
+      }
     }
     // Otherwise, overwrite the current installation.
   }