Resubmit: Refactor external_install_ui

chrome/browser/extensions/extension_service_unittest.cc

Index: chrome/browser/extensions/extension_service_unittest.cc
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index da2e19300eeb77c55e62edc942b98d9d6260029e..b1a5df0c02968001cd3aa3bcbae244313ee575ed 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -47,7 +47,8 @@
 #include "chrome/browser/extensions/extension_sync_data.h"
 #include "chrome/browser/extensions/extension_sync_service.h"
 #include "chrome/browser/extensions/extension_util.h"
-#include "chrome/browser/extensions/external_install_ui.h"
+#include "chrome/browser/extensions/external_install_error.h"
+#include "chrome/browser/extensions/external_install_manager.h"
 #include "chrome/browser/extensions/external_policy_loader.h"
 #include "chrome/browser/extensions/external_pref_loader.h"
 #include "chrome/browser/extensions/external_provider_impl.h"
@@ -6743,7 +6744,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
   service_->UpdateExternalExtensionAlert();
   // Should return false, meaning there aren't any extensions that the user
   // needs to know about.
-  EXPECT_FALSE(extensions::HasExternalInstallError(service_));
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
 
   // This is a normal extension, installed normally.
   // This should NOT trigger an alert.
@@ -6753,7 +6754,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
 
   service_->CheckForExternalUpdates();
   base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(extensions::HasExternalInstallError(service_));
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
 
   // A hosted app, installed externally.
   // This should NOT trigger an alert.
@@ -6765,7 +6766,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
       content::NotificationService::AllSources());
   service_->CheckForExternalUpdates();
   observer.Wait();
-  EXPECT_FALSE(extensions::HasExternalInstallError(service_));
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
 
   // Another normal extension, but installed externally.
   // This SHOULD trigger an alert.
@@ -6777,7 +6778,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
       content::NotificationService::AllSources());
   service_->CheckForExternalUpdates();
   observer2.Wait();
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
 }
 
 // Test that external extensions are initially disabled, and that enabling
@@ -6799,7 +6800,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallInitiallyDisabled) {
       content::NotificationService::AllSources());
   service_->CheckForExternalUpdates();
   observer.Wait();
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
   EXPECT_FALSE(service_->IsExtensionEnabled(page_action));
 
   const Extension* extension =
@@ -6808,7 +6809,7 @@ TEST_F(ExtensionServiceTest, ExternalInstallInitiallyDisabled) {
   EXPECT_EQ(page_action, extension->id());
 
   service_->EnableExtension(page_action);
-  EXPECT_FALSE(extensions::HasExternalInstallError(service_));
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
   EXPECT_TRUE(service_->IsExtensionEnabled(page_action));
 }
 
@@ -6841,20 +6842,25 @@ TEST_F(ExtensionServiceTest, MAYBE_ExternalInstallMultiple) {
       base::Bind(&WaitForCountNotificationsCallback, &count));
   service_->CheckForExternalUpdates();
   observer.Wait();
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
   EXPECT_FALSE(service_->IsExtensionEnabled(page_action));
   EXPECT_FALSE(service_->IsExtensionEnabled(good_crx));
   EXPECT_FALSE(service_->IsExtensionEnabled(theme_crx));
 
   service_->EnableExtension(page_action);
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
-  EXPECT_FALSE(extensions::HasExternalInstallBubble(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+  EXPECT_FALSE(
+      service_->external_install_manager()->HasExternalInstallBubble());
+
   service_->EnableExtension(theme_crx);
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
-  EXPECT_FALSE(extensions::HasExternalInstallBubble(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+  EXPECT_FALSE(
+      service_->external_install_manager()->HasExternalInstallBubble());
+
   service_->EnableExtension(good_crx);
-  EXPECT_FALSE(extensions::HasExternalInstallError(service_));
-  EXPECT_FALSE(extensions::HasExternalInstallBubble(service_));
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
+  EXPECT_FALSE(
+      service_->external_install_manager()->HasExternalInstallBubble());
 }
 
 // Test that there is a bubble for external extensions that update
@@ -6884,8 +6890,8 @@ TEST_F(ExtensionServiceTest, ExternalInstallUpdatesFromWebstoreOldProfile) {
       content::NotificationService::AllSources());
   service_->CheckForExternalUpdates();
   observer.Wait();
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
-  EXPECT_TRUE(extensions::HasExternalInstallBubble(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallBubble());
   EXPECT_FALSE(service_->IsExtensionEnabled(updates_from_webstore));
 }
 
@@ -6911,11 +6917,97 @@ TEST_F(ExtensionServiceTest, ExternalInstallUpdatesFromWebstoreNewProfile) {
       content::NotificationService::AllSources());
   service_->CheckForExternalUpdates();
   observer.Wait();
-  EXPECT_TRUE(extensions::HasExternalInstallError(service_));
-  EXPECT_FALSE(extensions::HasExternalInstallBubble(service_));
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+  EXPECT_FALSE(
+      service_->external_install_manager()->HasExternalInstallBubble());
   EXPECT_FALSE(service_->IsExtensionEnabled(updates_from_webstore));
 }
 
+// Test that clicking to remove the extension on an external install warning
+// uninstalls the extension.
+TEST_F(ExtensionServiceTest, ExternalInstallClickToRemove) {
+  FeatureSwitch::ScopedOverride prompt(
+      FeatureSwitch::prompt_for_external_extensions(), true);
+
+  ExtensionServiceInitParams params = CreateDefaultInitParams();
+  params.is_first_run = false;
+  InitializeExtensionService(params);
+
+  base::FilePath crx_path = temp_dir_.path().AppendASCII("webstore.crx");
+  PackCRX(data_dir_.AppendASCII("update_from_webstore"),
+          data_dir_.AppendASCII("update_from_webstore.pem"),
+          crx_path);
+
+  MockExtensionProvider* provider =
+      new MockExtensionProvider(service_, Manifest::EXTERNAL_PREF);
+  AddMockExternalProvider(provider);
+  provider->UpdateOrAddExtension(updates_from_webstore, "1", crx_path);
+
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+      content::NotificationService::AllSources());
+  service_->CheckForExternalUpdates();
+  observer.Wait();
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+
+  // We check both enabled and disabled, since these are "eventually exclusive"
+  // sets.
+  EXPECT_TRUE(registry_->disabled_extensions().GetByID(updates_from_webstore));
+  EXPECT_FALSE(registry_->enabled_extensions().GetByID(updates_from_webstore));
+
+  // Click the negative response.
+  service_->external_install_manager()->error_for_testing()->InstallUIAbort(
+      true);
+  // The Extension should be uninstalled.
+  EXPECT_FALSE(registry_->GetExtensionById(updates_from_webstore,
+                                           ExtensionRegistry::EVERYTHING));
+  // The error should be removed.
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
+}
+
+// Test that clicking to keep the extension on an external install warning
+// re-enables the extension.
+TEST_F(ExtensionServiceTest, ExternalInstallClickToKeep) {
+  FeatureSwitch::ScopedOverride prompt(
+      FeatureSwitch::prompt_for_external_extensions(), true);
+
+  ExtensionServiceInitParams params = CreateDefaultInitParams();
+  params.is_first_run = false;
+  InitializeExtensionService(params);
+
+  base::FilePath crx_path = temp_dir_.path().AppendASCII("webstore.crx");
+  PackCRX(data_dir_.AppendASCII("update_from_webstore"),
+          data_dir_.AppendASCII("update_from_webstore.pem"),
+          crx_path);
+
+  MockExtensionProvider* provider =
+      new MockExtensionProvider(service_, Manifest::EXTERNAL_PREF);
+  AddMockExternalProvider(provider);
+  provider->UpdateOrAddExtension(updates_from_webstore, "1", crx_path);
+
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+      content::NotificationService::AllSources());
+  service_->CheckForExternalUpdates();
+  observer.Wait();
+  EXPECT_TRUE(service_->external_install_manager()->HasExternalInstallError());
+
+  // We check both enabled and disabled, since these are "eventually exclusive"
+  // sets.
+  EXPECT_TRUE(registry_->disabled_extensions().GetByID(updates_from_webstore));
+  EXPECT_FALSE(registry_->enabled_extensions().GetByID(updates_from_webstore));
+
+  // Accept the extension.
+  service_->external_install_manager()->error_for_testing()->InstallUIProceed();
+
+  // It should be enabled again.
+  EXPECT_TRUE(registry_->enabled_extensions().GetByID(updates_from_webstore));
+  EXPECT_FALSE(registry_->disabled_extensions().GetByID(updates_from_webstore));
+
+  // The error should be removed.
+  EXPECT_FALSE(service_->external_install_manager()->HasExternalInstallError());
+}
+
 TEST_F(ExtensionServiceTest, InstallBlacklistedExtension) {
   InitializeEmptyExtensionService();