New notification sent when extension uninstall is not allowed.

In the event that an extension uninstall is skipped (not allowed)
because that extension is not user-manageable, a new notification is
sent.  The automation hook UninstallExtensionById is revised to handle
this case, and a new PyAuto test is written to exercise this new
functionality (the test attempts to uninstall the WebStore and verifies that
this extension cannot be uninstalled).

BUG=76598
TEST=None.

Review URL: http://codereview.chromium.org/6794040

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80814 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 102 insertions, 55 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 06842ac..59e3134 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -820,7 +820,7 @@ void AutomationProvider::UninstallExtension(int extension_handle,
   ExtensionService* service = profile_->GetExtensionService();
   if (extension && service) {
     ExtensionUnloadNotificationObserver observer;
-    service->UninstallExtension(extension->id(), false);
+    service->UninstallExtension(extension->id(), false, NULL);
     // The extension unload notification should have been sent synchronously
     // with the uninstall. Just to be safe, check that it was received.
     *success = observer.did_receive_unload_notification();
diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 3eff584..3e8d840 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -509,6 +509,8 @@ ExtensionUninstallObserver::ExtensionUninstallObserver(
       id_(id) {
   registrar_.Add(this, NotificationType::EXTENSION_UNINSTALLED,
                  NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSION_UNINSTALL_NOT_ALLOWED,
+                 NotificationService::AllSources());
 }
 
 ExtensionUninstallObserver::~ExtensionUninstallObserver() {
@@ -523,13 +525,36 @@ void ExtensionUninstallObserver::Observe(
     return;
   }
 
-  DCHECK(type == NotificationType::EXTENSION_UNINSTALLED);
-  UninstalledExtensionInfo* info =
-      Details<UninstalledExtensionInfo>(details).ptr();
-  if (id_ == info->extension_id) {
-    AutomationJSONReply(automation_, reply_message_.release())
-        .SendSuccess(NULL);
-    delete this;
+  switch (type.value) {
+    case NotificationType::EXTENSION_UNINSTALLED: {
+      UninstalledExtensionInfo* info =
+          Details<UninstalledExtensionInfo>(details).ptr();
+      if (id_ == info->extension_id) {
+        scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
+        return_value->SetBoolean("success", true);
+        AutomationJSONReply(automation_, reply_message_.release())
+            .SendSuccess(return_value.get());
+        delete this;
+        return;
+      }
+      break;
+    }
+
+    case NotificationType::EXTENSION_UNINSTALL_NOT_ALLOWED: {
+      const Extension* extension = Details<Extension>(details).ptr();
+      if (id_ == extension->id()) {
+        scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
+        return_value->SetBoolean("success", false);
+        AutomationJSONReply(automation_, reply_message_.release())
+            .SendSuccess(return_value.get());
+        delete this;
+        return;
+      }
+      break;
+    }
+
+    default:
+      NOTREACHED();
   }
 }
 
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 8d22f6e..67ffa9e 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -3910,7 +3910,7 @@ void TestingAutomationProvider::UninstallExtensionById(
   // Wait for a notification indicating that the extension with the given ID
   // has been uninstalled.  This observer will delete itself.
   new ExtensionUninstallObserver(this, reply_message, id);
-  service->UninstallExtension(id, false);
+  service->UninstallExtension(id, false, NULL);
 }
 
 // Sample json input:
diff --git a/chrome/browser/background_application_list_model_unittest.cc b/chrome/browser/background_application_list_model_unittest.cc
index a552bcc..1ed6ae0 100644
--- a/chrome/browser/background_application_list_model_unittest.cc
+++ b/chrome/browser/background_application_list_model_unittest.cc
@@ -141,23 +141,23 @@ TEST_F(BackgroundApplicationListModelTest, LoadExplicitExtensions) {
   ASSERT_EQ(2U, model->size());
   // Remove in FIFO order.
   ASSERT_FALSE(BackgroundApplicationListModel::IsBackgroundApp(*ext1));
-  service->UninstallExtension(ext1->id(), false);
+  service->UninstallExtension(ext1->id(), false, NULL);
   ASSERT_EQ(4U, service->extensions()->size());
   ASSERT_EQ(2U, model->size());
   ASSERT_TRUE(BackgroundApplicationListModel::IsBackgroundApp(*bgapp1));
-  service->UninstallExtension(bgapp1->id(), false);
+  service->UninstallExtension(bgapp1->id(), false, NULL);
   ASSERT_EQ(3U, service->extensions()->size());
   ASSERT_EQ(1U, model->size());
   ASSERT_FALSE(BackgroundApplicationListModel::IsBackgroundApp(*ext2));
-  service->UninstallExtension(ext2->id(), false);
+  service->UninstallExtension(ext2->id(), false, NULL);
   ASSERT_EQ(2U, service->extensions()->size());
   ASSERT_EQ(1U, model->size());
   ASSERT_TRUE(BackgroundApplicationListModel::IsBackgroundApp(*bgapp2));
-  service->UninstallExtension(bgapp2->id(), false);
+  service->UninstallExtension(bgapp2->id(), false, NULL);
   ASSERT_EQ(1U, service->extensions()->size());
   ASSERT_EQ(0U, model->size());
   ASSERT_FALSE(BackgroundApplicationListModel::IsBackgroundApp(*ext3));
-  service->UninstallExtension(ext3->id(), false);
+  service->UninstallExtension(ext3->id(), false, NULL);
   ASSERT_EQ(0U, service->extensions()->size());
   ASSERT_EQ(0U, model->size());
 }
@@ -232,7 +232,7 @@ TEST_F(BackgroundApplicationListModelTest, LoadRandomExtension) {
         extensions.erase(cursor);
         --count;
         ASSERT_EQ(count, extensions.size());
-        service->UninstallExtension(extension->id(), false);
+        service->UninstallExtension(extension->id(), false, NULL);
         ASSERT_EQ(count, service->extensions()->size());
         ASSERT_EQ(expected, model->size());
       }
diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 4b1c38c..b4c5cbb 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -502,7 +502,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, TabClosingWhenRemovingExtension) {
 
   // Uninstall the extension and make sure TabClosing is sent.
   ExtensionService* service = browser()->profile()->GetExtensionService();
-  service->UninstallExtension(GetExtension()->id(), false);
+  service->UninstallExtension(GetExtension()->id(), false, NULL);
   EXPECT_EQ(1, observer.closing_count());
 
   model->RemoveObserver(&observer);
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 670789a..277e7d4 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -279,7 +279,7 @@ void ExtensionBrowserTest::UnloadExtension(const std::string& extension_id) {
 
 void ExtensionBrowserTest::UninstallExtension(const std::string& extension_id) {
   ExtensionService* service = browser()->profile()->GetExtensionService();
-  service->UninstallExtension(extension_id, false);
+  service->UninstallExtension(extension_id, false, NULL);
 }
 
 void ExtensionBrowserTest::DisableExtension(const std::string& extension_id) {
diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc
index d057e58..6f2a0d0 100644
--- a/chrome/browser/extensions/extension_context_menu_model.cc
+++ b/chrome/browser/extensions/extension_context_menu_model.cc
@@ -152,7 +152,8 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id) {
 
 void ExtensionContextMenuModel::ExtensionDialogAccepted() {
   if (GetExtension())
-    profile_->GetExtensionService()->UninstallExtension(extension_id_, false);
+    profile_->GetExtensionService()->UninstallExtension(extension_id_, false,
+                                                        NULL);
 
   Release();
 }
diff --git a/chrome/browser/extensions/extension_management_api.cc b/chrome/browser/extensions/extension_management_api.cc
index ceaa864..70722bc 100644
--- a/chrome/browser/extensions/extension_management_api.cc
+++ b/chrome/browser/extensions/extension_management_api.cc
@@ -242,7 +242,8 @@ bool UninstallFunction::RunImpl() {
     return false;
   }
 
-  service()->UninstallExtension(extension_id, false /* external_uninstall */);
+  service()->UninstallExtension(extension_id, false /* external_uninstall */,
+                                NULL);
   return true;
 }
 
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 361e165..9887b4c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -258,7 +258,7 @@ void ExtensionService::CheckExternalUninstall(const std::string& id) {
   }
 
   // This is an external extension that we don't have registered.  Uninstall.
-  UninstallExtension(id, true);
+  UninstallExtension(id, true, NULL);
 }
 
 void ExtensionService::ClearProvidersForTesting() {
@@ -362,6 +362,9 @@ bool ExtensionService::IsInstalledApp(const GURL& url) {
 }
 
 // static
+// This function is used to implement the command-line switch
+// --uninstall-extension.  The LOG statements within this function are used to
+// inform the user if the uninstall cannot be done.
 bool ExtensionService::UninstallExtensionHelper(
     ExtensionService* extensions_service,
     const std::string& extension_id) {
@@ -371,19 +374,21 @@ bool ExtensionService::UninstallExtensionHelper(
   if (!extension)
     extension = extensions_service->GetTerminatedExtension(extension_id);
 
-  // We can't call UninstallExtension with an invalid extension ID. We should
-  // not allow an uninstall of a policy-controlled extension.
+  // We can't call UninstallExtension with an invalid extension ID.
   if (!extension) {
     LOG(WARNING) << "Attempted uninstallation of non-existent extension with "
                  << "id: " << extension_id;
     return false;
-  } else if (!Extension::UserMayDisable(extension->location())) {
-    LOG(WARNING) << "Attempted uninstallation of an extension that is non-"
-                 << "usermanagable with id: " << extension_id;
-    return false;
   }
 
-  extensions_service->UninstallExtension(extension_id, false);
+  // The following call to UninstallExtension will not allow an uninstall of a
+  // policy-controlled extension.
+  std::string error;
+  if (!extensions_service->UninstallExtension(extension_id, false, &error)) {
+    LOG(WARNING) << "Cannot uninstall extension with id " << extension_id
+                 << ": " << error;
+    return false;
+  }
 
   return true;
 }
@@ -607,8 +612,9 @@ void ExtensionService::ReloadExtension(const std::string& extension_id) {
   }
 }
 
-void ExtensionService::UninstallExtension(const std::string& extension_id,
-                                          bool external_uninstall) {
+bool ExtensionService::UninstallExtension(const std::string& extension_id,
+                                          bool external_uninstall,
+                                          std::string* error) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const Extension* extension =
@@ -627,8 +633,16 @@ void ExtensionService::UninstallExtension(const std::string& extension_id,
   // Policy change which triggers an uninstall will always set
   // |external_uninstall| to true so this is the only way to uninstall
   // managed extensions.
-  if (!Extension::UserMayDisable(location) && !external_uninstall)
-    return;
+  if (!Extension::UserMayDisable(location) && !external_uninstall) {
+    NotificationService::current()->Notify(
+        NotificationType::EXTENSION_UNINSTALL_NOT_ALLOWED,
+        Source<Profile>(profile_),
+        Details<const Extension>(extension));
+    if (error != NULL) {
+      *error = errors::kCannotUninstallManagedExtension;
+    }
+    return false;
+  }
 
   UninstalledExtensionInfo uninstalled_extension_info(*extension);
 
@@ -667,6 +681,8 @@ void ExtensionService::UninstallExtension(const std::string& extension_id,
       NotificationType::EXTENSION_UNINSTALLED,
       Source<Profile>(profile_),
       Details<UninstalledExtensionInfo>(&uninstalled_extension_info));
+
+  return true;
 }
 
 void ExtensionService::ClearExtensionData(const GURL& extension_url) {
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 54f70d6..344d323 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -60,8 +60,9 @@ class ExtensionServiceInterface {
   virtual const Extension* GetExtensionById(const std::string& id,
                                             bool include_disabled) const = 0;
 
-  virtual void UninstallExtension(const std::string& extension_id,
-                                  bool external_uninstall) = 0;
+  virtual bool UninstallExtension(const std::string& extension_id,
+                                  bool external_uninstall,
+                                  std::string* error) = 0;
 
   virtual bool IsExtensionEnabled(const std::string& extension_id) const = 0;
   virtual bool IsExternalExtensionUninstalled(
@@ -228,8 +229,9 @@ class ExtensionService
   // callers should never set to true.
   // TODO(aa): Remove |external_uninstall| -- this information should be passed
   // to ExtensionPrefs some other way.
-  virtual void UninstallExtension(const std::string& extension_id,
-                                  bool external_uninstall);
+  virtual bool UninstallExtension(const std::string& extension_id,
+                                  bool external_uninstall,
+                                  std::string* error);
 
   virtual bool IsExtensionEnabled(const std::string& extension_id) const;
   virtual bool IsExternalExtensionUninstalled(
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index cae8b1b..d28eef5 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -1128,7 +1128,7 @@ TEST_F(ExtensionServiceTest, UninstallingExternalExtensions) {
   ASSERT_TRUE(service_->GetExtensionById(good_crx, false));
 
   // Uninstall it and check that its killbit gets set.
-  service_->UninstallExtension(good_crx, false);
+  service_->UninstallExtension(good_crx, false, NULL);
   loop_.RunAllPending();
   ValidateIntegerPref(good_crx, "location",
                       Extension::EXTERNAL_EXTENSION_UNINSTALLED);
@@ -1789,7 +1789,7 @@ TEST_F(ExtensionServiceTest, InstallAppsWithUnlimtedStorage) {
 
   // Uninstall one of them, unlimited storage should still be granted
   // to the origin.
-  service_->UninstallExtension(id1, false);
+  service_->UninstallExtension(id1, false, NULL);
   loop_.RunAllPending();
   EXPECT_EQ(1u, service_->extensions()->size());
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
@@ -1797,7 +1797,7 @@ TEST_F(ExtensionServiceTest, InstallAppsWithUnlimtedStorage) {
 
 
   // Uninstall the other, unlimited storage should be revoked.
-  service_->UninstallExtension(id2, false);
+  service_->UninstallExtension(id2, false, NULL);
   loop_.RunAllPending();
   EXPECT_EQ(0u, service_->extensions()->size());
   EXPECT_FALSE(profile_->GetExtensionSpecialStoragePolicy()->
@@ -1834,11 +1834,11 @@ TEST_F(ExtensionServiceTest, InstallAppsAndCheckStorageProtection) {
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageProtected(origin2));
 
-  service_->UninstallExtension(id1, false);
+  service_->UninstallExtension(id1, false, NULL);
   loop_.RunAllPending();
   EXPECT_EQ(1u, service_->extensions()->size());
 
-  service_->UninstallExtension(id2, false);
+  service_->UninstallExtension(id2, false, NULL);
   loop_.RunAllPending();
 
   EXPECT_TRUE(service_->extensions()->empty());
@@ -2702,7 +2702,7 @@ TEST_F(ExtensionServiceTest, UninstallExtension) {
   ValidateIntegerPref(good_crx, "location", Extension::INTERNAL);
 
   // Uninstall it.
-  service_->UninstallExtension(extension_id, false);
+  service_->UninstallExtension(extension_id, false, NULL);
   total_successes_ = 0;
 
   // We should get an unload notification.
@@ -2819,7 +2819,7 @@ TEST_F(ExtensionServiceTest, ClearExtensionData) {
   EXPECT_TRUE(file_util::PathExists(idb_path));
 
   // Uninstall the extension.
-  service_->UninstallExtension(good_crx, false);
+  service_->UninstallExtension(good_crx, false, NULL);
   loop_.RunAllPending();
 
   // Check that the cookie is gone.
@@ -2873,7 +2873,7 @@ TEST_F(ExtensionServiceTest, LoadExtension) {
   // Test uninstall.
   std::string id = loaded_[0]->id();
   EXPECT_FALSE(unloaded_id_.length());
-  service_->UninstallExtension(id, false);
+  service_->UninstallExtension(id, false, NULL);
   loop_.RunAllPending();
   EXPECT_EQ(id, unloaded_id_);
   ASSERT_EQ(0u, loaded_.size());
@@ -2966,7 +2966,7 @@ void ExtensionServiceTest::TestExternalProvider(
   // Uninstall the extension and reload. Nothing should happen because the
   // preference should prevent us from reinstalling.
   std::string id = loaded_[0]->id();
-  service_->UninstallExtension(id, false);
+  service_->UninstallExtension(id, false, NULL);
   loop_.RunAllPending();
 
   FilePath install_path = extensions_install_dir_.AppendASCII(id);
@@ -3025,7 +3025,7 @@ void ExtensionServiceTest::TestExternalProvider(
 
     // User uninstalls.
     loaded_.clear();
-    service_->UninstallExtension(id, false);
+    service_->UninstallExtension(id, false, NULL);
     loop_.RunAllPending();
     ASSERT_EQ(0u, loaded_.size());
 
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 16de5b4..e2f9b9f 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -76,9 +76,11 @@ class MockService : public ExtensionServiceInterface {
     return NULL;
   }
 
-  virtual void UninstallExtension(const std::string& extension_id,
-                                  bool external_uninstall) {
-    FAIL();
+  virtual bool UninstallExtension(const std::string& extension_id,
+                                  bool external_uninstall,
+                                  std::string* error) {
+    ADD_FAILURE();
+    return false;
   }
 
   virtual bool IsExtensionEnabled(const std::string& extension_id) const {
diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc
index 166c80c..ed29460 100644
--- a/chrome/browser/extensions/extensions_ui.cc
+++ b/chrome/browser/extensions/extensions_ui.cc
@@ -476,7 +476,7 @@ void ExtensionsDOMHandler::ExtensionDialogAccepted() {
     return;
 
   extensions_service_->UninstallExtension(extension_id_prompting_,
-                                          false /* external_uninstall */);
+                                          false /* external_uninstall */, NULL);
   extension_id_prompting_ = "";
 
   // There will be no EXTENSION_UNLOADED notification for terminated
diff --git a/chrome/browser/sync/glue/extension_sync.cc b/chrome/browser/sync/glue/extension_sync.cc
index bef56cb..2834bb7 100644
--- a/chrome/browser/sync/glue/extension_sync.cc
+++ b/chrome/browser/sync/glue/extension_sync.cc
@@ -457,7 +457,7 @@ void RemoveFromClient(const ExtensionSyncTraits& traits,
   const Extension* extension = extensions_service->GetExtensionById(id, true);
   if (extension) {
     if (traits.is_valid_and_syncable(*extension)) {
-      extensions_service->UninstallExtension(id, false);
+      extensions_service->UninstallExtension(id, false, NULL);
     } else {
       LOG(WARNING) << "Ignoring server data for invalid or "
                    << "non-syncable extension " << extension->id();
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
index 3c4c3ff..72a2dba 100644
--- a/chrome/browser/themes/theme_service.cc
+++ b/chrome/browser/themes/theme_service.cc
@@ -328,7 +328,7 @@ void ThemeService::RemoveUnusedThemes() {
     }
   }
   for (size_t i = 0; i < remove_list.size(); ++i)
-    service->UninstallExtension(remove_list[i], false);
+    service->UninstallExtension(remove_list[i], false, NULL);
 }
 
 void ThemeService::UseDefaultTheme() {
diff --git a/chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm b/chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm
index 3bf4cf4..989c472 100644
--- a/chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm
+++ b/chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm
@@ -49,7 +49,7 @@ class AsyncUninstaller : public ExtensionUninstallDialog::Delegate {
   // ExtensionUninstallDialog::Delegate:
   virtual void ExtensionDialogAccepted() {
     profile_->GetExtensionService()->
-        UninstallExtension(extension_->id(), false);
+        UninstallExtension(extension_->id(), false, NULL);
   }
   virtual void ExtensionDialogCanceled() {}
 
diff --git a/chrome/browser/ui/webui/app_launcher_handler.cc b/chrome/browser/ui/webui/app_launcher_handler.cc
index 5cde49a..b71e113 100644
--- a/chrome/browser/ui/webui/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/app_launcher_handler.cc
@@ -559,7 +559,7 @@ void AppLauncherHandler::ExtensionDialogAccepted() {
     return;
 
   extensions_service_->UninstallExtension(extension_id_prompting_,
-                                          false /* external_uninstall */);
+                                          false /* external_uninstall */, NULL);
 
   extension_id_prompting_ = "";
 }
@@ -617,6 +617,6 @@ void AppLauncherHandler::UninstallDefaultApps() {
   for (ExtensionIdSet::const_iterator iter = app_ids.begin();
        iter != app_ids.end(); ++iter) {
     if (extensions_service_->GetExtensionById(*iter, true))
-      extensions_service_->UninstallExtension(*iter, false);
+      extensions_service_->UninstallExtension(*iter, false, NULL);
   }
 }
diff --git a/chrome/browser/ui/webui/options/extension_settings_handler.cc b/chrome/browser/ui/webui/options/extension_settings_handler.cc
index 0b612a6..1392671 100644
--- a/chrome/browser/ui/webui/options/extension_settings_handler.cc
+++ b/chrome/browser/ui/webui/options/extension_settings_handler.cc
@@ -569,7 +569,7 @@ void ExtensionsDOMHandler::ExtensionDialogAccepted() {
     return;
 
   extensions_service_->UninstallExtension(extension_id_prompting_,
-                                          false /* external_uninstall */);
+                                          false /* external_uninstall */, NULL);
   extension_id_prompting_ = "";
 
   // There will be no EXTENSION_UNLOADED notification for terminated