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
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 @@
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 @@
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 @@
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 @@
// 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 a552bccc..1ed6ae0 100644
--- a/chrome/browser/background_application_list_model_unittest.cc
+++ b/chrome/browser/background_application_list_model_unittest.cc
@@ -141,23 +141,23 @@
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 @@
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 @@
// 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..277e7d46 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -279,7 +279,7 @@
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::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..70722bc4 100644
--- a/chrome/browser/extensions/extension_management_api.cc
+++ b/chrome/browser/extensions/extension_management_api.cc
@@ -242,7 +242,8 @@
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 @@
}
// 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 @@
}
// 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 @@
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::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 @@
// 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 @@
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..344d3234 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -60,8 +60,9 @@
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 @@
// 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 cae8b1bd..d28eef55 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -1128,7 +1128,7 @@
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 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 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 @@
// 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 @@
// 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 @@
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 @@
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..2834bb7e 100644
--- a/chrome/browser/sync/glue/extension_sync.cc
+++ b/chrome/browser/sync/glue/extension_sync.cc
@@ -457,7 +457,7 @@
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..72a2dba6 100644
--- a/chrome/browser/themes/theme_service.cc
+++ b/chrome/browser/themes/theme_service.cc
@@ -328,7 +328,7 @@
}
}
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 3bf4cf46..989c472a 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 @@
// 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..b71e113a 100644
--- a/chrome/browser/ui/webui/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/app_launcher_handler.cc
@@ -559,7 +559,7 @@
return;
extensions_service_->UninstallExtension(extension_id_prompting_,
- false /* external_uninstall */);
+ false /* external_uninstall */, NULL);
extension_id_prompting_ = "";
}
@@ -617,6 +617,6 @@
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 0b612a6a..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 @@
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
