Revert "[Extensions] Handle reloading an extension that failed to load"

This reverts commit e8275717.

Reason for revert: Causes  ToolbarActionsBarUnitTest.ReuploadExtensionFailed to crash, e.g. https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests%20%28dbg%29%281%29/b8876023303247456768

Original change's description:
> [Extensions] Handle reloading an extension that failed to load
> 
> This CL enables ExtensionService::ReloadExtension() to handle reload
> failures, where subsequent calls can retry the reload, rather than
> immediately giving up.
> 
> Bug: 792277
> Change-Id: I482da093133a1baee7f2f5cad8abbc9c85e9e074
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2242642
> Commit-Queue: Ghazale Hosseinabadi <ghazale@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#784037}

TBR=rdevlin.cronin@chromium.org,ghazale@chromium.org

Change-Id: Id229fde9a694ac3e0a0b8376492604b11f461292
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 792277, 1101189
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277366Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784277}

chrome/browser/extensions/extension_service.cc

@@ -723,18 +723,10 @@ void ExtensionService::LoadExtensionForReload(
         UnpackedInstaller::Create(this);
     unpacked_installer->set_be_noisy_on_failure(load_error_behavior ==
                                                 LoadErrorBehavior::kNoisy);
-    unpacked_installer->set_completion_callback(base::BindOnce(
-        &ExtensionService::OnUnpackedReloadFailure, base::Unretained(this)));
     unpacked_installer->Load(path);
   }
 }
 
-void ExtensionService::OnUnpackedReloadFailure(const Extension* extension,
-                                               const base::FilePath& file_path,
-                                               const std::string& error) {
-  extension_registrar_.OnUnpackedExtensionReloadFailed(file_path);
-}
-
 void ExtensionService::ReloadExtension(const std::string& extension_id) {
   extension_registrar_.ReloadExtension(extension_id, LoadErrorBehavior::kNoisy);
 }

chrome/browser/extensions/extension_service.h

@@ -229,11 +229,6 @@ class ExtensionService : public ExtensionServiceInterface,
   // Called when the associated Profile is going to be destroyed.
   void Shutdown();
 
-  // Called when reloading an unpacked extension fails.
-  void OnUnpackedReloadFailure(const Extension* extension,
-                               const base::FilePath& file_path,
-                               const std::string& error);
-
   // Reloads the specified extension, sending the onLaunched() event to it if it
   // currently has any window showing.
   // Allows noisy failures.

chrome/browser/extensions/extension_service_unittest.cc

@@ -2496,74 +2496,6 @@ TEST_F(ExtensionServiceTest, UnpackedExtensionMayNotHaveUnderscore) {
   EXPECT_EQ(0u, registry()->enabled_extensions().size());
 }
 
-// Tests that an unpacked extension with a malformed manifest can't reload.
-// Reload succeeds after fixing the manifest.
-TEST_F(ExtensionServiceTest,
-       ReloadExtensionWithMalformedManifestAndCorrectManifest) {
-  InitializeEmptyExtensionService();
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::FilePath extension_dir = base::MakeAbsoluteFilePath(temp_dir.GetPath());
-  base::FilePath manifest_dir = extension_dir.Append(kManifestFilename);
-  ASSERT_FALSE(base::PathExists(manifest_dir));
-
-  // First create a correct manifest and Load the extension successfully.
-  base::DictionaryValue manifest;
-  manifest.SetString("version", "1.0");
-  manifest.SetString("name", "malformed manifest reload test");
-  manifest.SetInteger("manifest_version", 2);
-
-  JSONFileValueSerializer serializer(manifest_dir);
-  ASSERT_TRUE(serializer.Serialize(manifest));
-
-  // Load the extension successfully.
-  UnpackedInstaller::Create(service())->Load(extension_dir);
-  content::RunAllTasksUntilIdle();
-  // Verify that Load was successful
-  EXPECT_EQ(0u, GetErrors().size());
-  ASSERT_EQ(1u, loaded_.size());
-  EXPECT_EQ(Manifest::UNPACKED, loaded_[0]->location());
-  EXPECT_EQ(1u, registry()->enabled_extensions().size());
-  EXPECT_EQ("1.0", loaded_[0]->VersionString());
-
-  // Change the version to a malformed version.
-  manifest.SetString("version", "2.0b");
-  ASSERT_TRUE(serializer.Serialize(manifest));
-
-  std::string extension_id = loaded_[0]->id();
-
-  // Reload the extension.
-  service()->ReloadExtension(extension_id);
-  content::RunAllTasksUntilIdle();
-
-  // An error is generated.
-  ASSERT_EQ(1u, GetErrors().size());
-  EXPECT_THAT(
-      base::UTF16ToUTF8(GetErrors()[0]),
-      ::testing::HasSubstr("Required value 'version' is missing or invalid."));
-
-  // Verify that ReloadExtension() was not successful.
-  ASSERT_EQ(0u, loaded_.size());
-  EXPECT_TRUE(registry()->disabled_extensions().Contains(extension_id));
-
-  // Fix the version.
-  manifest.SetString("version", "2.0");
-  ASSERT_TRUE(serializer.Serialize(manifest));
-
-  // Reload the extension.
-  service()->ReloadExtension(extension_id);
-  content::RunAllTasksUntilIdle();
-
-  // No new error is generated. Since the error generated above is still there,
-  // the error size is 1.
-  EXPECT_EQ(1u, GetErrors().size());
-  // Verify that ReloadExtension() was successful.
-  ASSERT_EQ(1u, loaded_.size());
-  EXPECT_EQ(Manifest::UNPACKED, loaded_[0]->location());
-  EXPECT_EQ(1u, registry()->enabled_extensions().size());
-  EXPECT_EQ("2.0", loaded_[0]->VersionString());
-}
-
 TEST_F(ExtensionServiceTest, InstallLocalizedTheme) {
   InitializeEmptyExtensionService();
   service()->Init();

extensions/browser/extension_registrar.cc

@@ -96,7 +96,6 @@ void ExtensionRegistrar::AddExtension(
   delegate_->PreAddExtension(extension.get(), old);
 
   if (was_reloading) {
-    failed_to_reload_unpacked_extensions_.erase(extension->path());
     ReplaceReloadedExtension(extension);
   } else {
     if (is_extension_loaded) {
@@ -292,26 +291,12 @@ void ExtensionRegistrar::ReloadExtension(
     LoadErrorBehavior load_error_behavior) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  base::FilePath path;
-
-  const Extension* disabled_extension =
-      registry_->disabled_extensions().GetByID(extension_id);
-
-  if (disabled_extension) {
-    path = disabled_extension->path();
-  }
-
   // If the extension is already reloading, don't reload again.
   if (extension_prefs_->HasDisableReason(extension_id,
                                          disable_reason::DISABLE_RELOAD)) {
-    DCHECK(disabled_extension);
-    // If an unpacked extension previously failed to reload, it will still be
-    // marked as disabled, but we can try to reload it again - the developer
-    // may have fixed the issue.
-    if (failed_to_reload_unpacked_extensions_.count(path) == 0)
-      return;
-    failed_to_reload_unpacked_extensions_.erase(path);
+    return;
   }
+
   // Ignore attempts to reload a blocklisted or blocked extension. Sometimes
   // this can happen in a convoluted reload sequence triggered by the
   // termination of a blocklisted or blocked extension and a naive attempt to
@@ -321,6 +306,8 @@ void ExtensionRegistrar::ReloadExtension(
     return;
   }
 
+  base::FilePath path;
+
   const Extension* enabled_extension =
       registry_->enabled_extensions().GetByID(extension_id);
 
@@ -347,7 +334,7 @@ void ExtensionRegistrar::ReloadExtension(
     DisableExtension(extension_id, disable_reason::DISABLE_RELOAD);
     DCHECK(registry_->disabled_extensions().Contains(extension_id));
     reloading_extensions_.insert(extension_id);
-  } else if (!disabled_extension) {
+  } else {
     std::map<ExtensionId, base::FilePath>::const_iterator iter =
         unloaded_extension_paths_.find(extension_id);
     if (iter == unloaded_extension_paths_.end()) {
@@ -359,11 +346,6 @@ void ExtensionRegistrar::ReloadExtension(
   delegate_->LoadExtensionForReload(extension_id, path, load_error_behavior);
 }
 
-void ExtensionRegistrar::OnUnpackedExtensionReloadFailed(
-    const base::FilePath& path) {
-  failed_to_reload_unpacked_extensions_.insert(path);
-}
-
 void ExtensionRegistrar::TerminateExtension(const ExtensionId& extension_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 

extensions/browser/extension_registrar.h

@@ -139,8 +139,6 @@ class ExtensionRegistrar {
   // host is created.
   void DidCreateRenderViewForBackgroundPage(ExtensionHost* host);
 
-  void OnUnpackedExtensionReloadFailed(const base::FilePath& path);
-
  private:
   // Adds the extension to the appropriate registry set, based on ExtensionPrefs
   // and our |delegate_|. Activates the extension if it's added to the enabled
@@ -196,10 +194,6 @@ class ExtensionRegistrar {
   // which were disabled for a reload.
   ExtensionIdSet reloading_extensions_;
 
-  // Store the paths of extensions that failed to reload. We use this to retry
-  // reload.
-  std::set<base::FilePath> failed_to_reload_unpacked_extensions_;
-
   base::WeakPtrFactory<ExtensionRegistrar> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionRegistrar);

extensions/browser/extension_registrar_unittest.cc

@@ -204,8 +204,6 @@ class ExtensionRegistrarTest : public ExtensionsTest {
                                 UnloadedExtensionReason::UNINSTALL);
     ExpectInSet(ExtensionRegistry::NONE);
 
-    ExtensionPrefs::Get(browser_context())
-        ->DeleteExtensionPrefs(extension_->id());
     // Removing a disabled extension should trigger a notification.
     EXPECT_TRUE(notification_tracker_.Check1AndReset(
         extensions::NOTIFICATION_EXTENSION_REMOVED));