diff options
author | jennb@chromium.org <jennb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-13 00:21:20 +0000 |
---|---|---|
committer | jennb@chromium.org <jennb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-13 00:21:20 +0000 |
commit | bb7f4095f73cf4c0ce0fe8a3cff37def81311d57 (patch) | |
tree | 7622be5faaa5342ea48c2627eb45ab1586d2be4d /chrome/browser/extensions | |
parent | 31ae9addcfc81537293f5c4fc6c12ac9d8006159 (diff) | |
download | chromium_src-bb7f4095f73cf4c0ce0fe8a3cff37def81311d57.zip chromium_src-bb7f4095f73cf4c0ce0fe8a3cff37def81311d57.tar.gz chromium_src-bb7f4095f73cf4c0ce0fe8a3cff37def81311d57.tar.bz2 |
Show crashed extensions on chrome://extensions.
BUG=31535
TEST=updated JSON serialization tests, added check in browsertest, manual test using task manager to terminate an extension process
Review URL: http://codereview.chromium.org/6158001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71263 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/extensions')
-rw-r--r-- | chrome/browser/extensions/extension_crash_recovery_browsertest.cc | 35 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_service.cc | 29 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_service.h | 12 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_ui_unittest.cc | 2 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_ui.cc | 22 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_ui.h | 3 |
6 files changed, 98 insertions, 5 deletions
diff --git a/chrome/browser/extensions/extension_crash_recovery_browsertest.cc b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc index cb489a1..cc5e62e 100644 --- a/chrome/browser/extensions/extension_crash_recovery_browsertest.cc +++ b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc @@ -111,25 +111,38 @@ class ExtensionCrashRecoveryTest : public ExtensionBrowserTest { IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, Basic) { const size_t size_before = GetExtensionService()->extensions()->size(); + const size_t crash_size_before = + GetExtensionService()->terminated_extensions()->size(); LoadTestExtension(); CrashExtension(size_before); ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); + ASSERT_EQ(crash_size_before + 1, + GetExtensionService()->terminated_extensions()->size()); AcceptCrashedExtensionInfobar(0); SCOPED_TRACE("after clicking the infobar"); CheckExtensionConsistency(size_before); + ASSERT_EQ(crash_size_before, + GetExtensionService()->terminated_extensions()->size()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CloseAndReload) { const size_t size_before = GetExtensionService()->extensions()->size(); + const size_t crash_size_before = + GetExtensionService()->terminated_extensions()->size(); LoadTestExtension(); CrashExtension(size_before); ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); + ASSERT_EQ(crash_size_before + 1, + GetExtensionService()->terminated_extensions()->size()); + CancelCrashedExtensionInfobar(0); ReloadExtension(first_extension_id_); SCOPED_TRACE("after reloading"); CheckExtensionConsistency(size_before); + ASSERT_EQ(crash_size_before, + GetExtensionService()->terminated_extensions()->size()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, ReloadIndependently) { @@ -316,12 +329,18 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashSecond) { IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashBothAtOnce) { const size_t size_before = GetExtensionService()->extensions()->size(); + const size_t crash_size_before = + GetExtensionService()->terminated_extensions()->size(); LoadTestExtension(); LoadSecondExtension(); CrashExtension(size_before); ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); + ASSERT_EQ(crash_size_before + 1, + GetExtensionService()->terminated_extensions()->size()); CrashExtension(size_before); ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); + ASSERT_EQ(crash_size_before + 2, + GetExtensionService()->terminated_extensions()->size()); { SCOPED_TRACE("first infobar"); @@ -421,3 +440,19 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CheckExtensionConsistency(size_before + 1); } } + +IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CrashAndUnloadAll) { + const size_t size_before = GetExtensionService()->extensions()->size(); + const size_t crash_size_before = + GetExtensionService()->terminated_extensions()->size(); + LoadTestExtension(); + LoadSecondExtension(); + CrashExtension(size_before); + ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); + ASSERT_EQ(crash_size_before + 1, + GetExtensionService()->terminated_extensions()->size()); + + GetExtensionService()->UnloadAllExtensions(); + ASSERT_EQ(crash_size_before, + GetExtensionService()->terminated_extensions()->size()); +} diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 8140d2b..67cb44e 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -603,6 +603,10 @@ const ExtensionList* ExtensionService::disabled_extensions() const { return &disabled_extensions_; } +const ExtensionList* ExtensionService::terminated_extensions() const { + return &terminated_extensions_; +} + const PendingExtensionMap& ExtensionService::pending_extensions() const { return pending_extensions_; } @@ -1534,6 +1538,8 @@ void ExtensionService::UnloadExtension( void ExtensionService::UnloadAllExtensions() { extensions_.clear(); disabled_extensions_.clear(); + terminated_extension_ids_.clear(); + terminated_extensions_.clear(); extension_runtime_data_.clear(); // TODO(erikkay) should there be a notification for this? We can't use @@ -1590,6 +1596,9 @@ void ExtensionService::OnExtensionLoaded(const Extension* extension) { // The extension is now loaded, remove its data from unloaded extension map. unloaded_extension_paths_.erase(extension->id()); + // If a terminated extension is loaded, remove it from the terminated list. + UntrackTerminatedExtension(extension->id()); + // If the extension was disabled for a reload, then enable it. if (disabled_extension_paths_.erase(extension->id()) > 0) EnableExtension(extension->id()); @@ -1844,6 +1853,25 @@ const Extension* ExtensionService::GetExtensionByIdInternal( return NULL; } +void ExtensionService::TrackTerminatedExtension(const Extension* extension) { + if (terminated_extension_ids_.insert(extension->id()).second) + terminated_extensions_.push_back(make_scoped_refptr(extension)); +} + +void ExtensionService::UntrackTerminatedExtension(const std::string& id) { + if (terminated_extension_ids_.erase(id) <= 0) + return; + + std::string lowercase_id = StringToLowerASCII(id); + for (ExtensionList::iterator iter = terminated_extensions_.begin(); + iter != terminated_extensions_.end(); ++iter) { + if ((*iter)->id() == lowercase_id) { + terminated_extensions_.erase(iter); + return; + } + } +} + const Extension* ExtensionService::GetWebStoreApp() { return GetExtensionById(extension_misc::kWebStoreAppId, false); } @@ -1997,6 +2025,7 @@ void ExtensionService::Observe(NotificationType type, break; ExtensionHost* host = Details<ExtensionHost>(details).ptr(); + TrackTerminatedExtension(host->extension()); // Unload the entire extension. We want it to be in a consistent state: // either fully working or not loaded at all, but never half-crashed. diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h index 9e4206c..21c4a57 100644 --- a/chrome/browser/extensions/extension_service.h +++ b/chrome/browser/extensions/extension_service.h @@ -156,6 +156,7 @@ class ExtensionService // Gets the list of currently installed extensions. virtual const ExtensionList* extensions() const; virtual const ExtensionList* disabled_extensions() const; + virtual const ExtensionList* terminated_extensions() const; // Gets the set of pending extensions. virtual const PendingExtensionMap& pending_extensions() const; @@ -468,6 +469,11 @@ class ExtensionService bool include_enabled, bool include_disabled); + + // Keep track of terminated extensions. + void TrackTerminatedExtension(const Extension* extension); + void UntrackTerminatedExtension(const std::string& id); + // Like AddPendingExtension*() functions above, but assumes an // extension with the same id is not already installed. void AddPendingExtensionInternal( @@ -508,6 +514,12 @@ class ExtensionService // The list of installed extensions that have been disabled. ExtensionList disabled_extensions_; + // The list of installed extensions that have been terminated. + ExtensionList terminated_extensions_; + + // Used to quickly check if an extension was terminated. + std::set<std::string> terminated_extension_ids_; + // The set of pending extensions. PendingExtensionMap pending_extensions_; diff --git a/chrome/browser/extensions/extension_ui_unittest.cc b/chrome/browser/extensions/extension_ui_unittest.cc index 1bda60a..5980109 100644 --- a/chrome/browser/extensions/extension_ui_unittest.cc +++ b/chrome/browser/extensions/extension_ui_unittest.cc @@ -53,7 +53,7 @@ namespace { // Produce test output. scoped_ptr<DictionaryValue> actual_output_data( ExtensionsDOMHandler::CreateExtensionDetailValue(NULL, extension.get(), - pages, true)); + pages, true, false)); // Compare the outputs. return expected_output_data->Equals(actual_output_data.get()); diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc index e191a89..a653ccc 100644 --- a/chrome/browser/extensions/extensions_ui.cc +++ b/chrome/browser/extensions/extensions_ui.cc @@ -123,6 +123,8 @@ void ExtensionsUIHTMLSource::StartDataRequest(const std::string& path, ASCIIToUTF16("'>") + l10n_util::GetStringUTF16(IDS_GET_MORE_EXTENSIONS) + ASCIIToUTF16("</a>")); + localized_strings.SetString("extensionCrashed", + l10n_util::GetStringUTF16(IDS_EXTENSIONS_CRASHED_EXTENSION)); localized_strings.SetString("extensionDisabled", l10n_util::GetStringUTF16(IDS_EXTENSIONS_DISABLED_EXTENSION)); localized_strings.SetString("inDevelopment", @@ -349,7 +351,7 @@ void ExtensionsDOMHandler::HandleRequestExtensionsData(const ListValue* args) { extensions_service_.get(), *extension, GetActivePagesForExtension(*extension), - true)); // enabled + true, false)); // enabled, terminated extension_icons->push_back(PickExtensionIcon(*extension)); } } @@ -361,7 +363,20 @@ void ExtensionsDOMHandler::HandleRequestExtensionsData(const ListValue* args) { extensions_service_.get(), *extension, GetActivePagesForExtension(*extension), - false)); // enabled + false, false)); // enabled, terminated + extension_icons->push_back(PickExtensionIcon(*extension)); + } + } + extensions = extensions_service_->terminated_extensions(); + std::vector<ExtensionPage> empty_pages; + for (ExtensionList::const_iterator extension = extensions->begin(); + extension != extensions->end(); ++extension) { + if (ShouldShowExtension(*extension)) { + extensions_list->Append(CreateExtensionDetailValue( + extensions_service_.get(), + *extension, + empty_pages, // Terminated process has no active pages. + false, true)); // enabled, terminated extension_icons->push_back(PickExtensionIcon(*extension)); } } @@ -790,7 +805,7 @@ static bool ExtensionWantsFileAccess(const Extension* extension) { // Static DictionaryValue* ExtensionsDOMHandler::CreateExtensionDetailValue( ExtensionService* service, const Extension* extension, - const std::vector<ExtensionPage>& pages, bool enabled) { + const std::vector<ExtensionPage>& pages, bool enabled, bool terminated) { DictionaryValue* extension_data = new DictionaryValue(); extension_data->SetString("id", extension->id()); @@ -798,6 +813,7 @@ DictionaryValue* ExtensionsDOMHandler::CreateExtensionDetailValue( extension_data->SetString("description", extension->description()); extension_data->SetString("version", extension->version()->GetString()); extension_data->SetBoolean("enabled", enabled); + extension_data->SetBoolean("terminated", terminated); extension_data->SetBoolean("enabledIncognito", service ? service->IsIncognitoEnabled(extension) : false); extension_data->SetBoolean("wantsFileAccess", diff --git a/chrome/browser/extensions/extensions_ui.h b/chrome/browser/extensions/extensions_ui.h index b7a9b69..37037ef 100644 --- a/chrome/browser/extensions/extensions_ui.h +++ b/chrome/browser/extensions/extensions_ui.h @@ -114,7 +114,8 @@ class ExtensionsDOMHandler ExtensionService* service, const Extension* extension, const std::vector<ExtensionPage>& pages, - bool enabled); + bool enabled, + bool terminated); // ContentScript JSON Struct for page. (static for ease of testing). static DictionaryValue* CreateContentScriptDetailValue( |