diff options
author | phajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-08-25 15:22:46 +0000 |
---|---|---|
committer | phajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-08-25 15:22:46 +0000 |
commit | 371ed7a0e83c44764fef64ddbf00996b5385271e (patch) | |
tree | 5328f866d0c39ab82837ff7c91d1a3741ab97174 | |
parent | 856c26be2a9a419033f93d79ba9ee43e04b20458 (diff) | |
download | chromium_src-371ed7a0e83c44764fef64ddbf00996b5385271e.zip chromium_src-371ed7a0e83c44764fef64ddbf00996b5385271e.tar.gz chromium_src-371ed7a0e83c44764fef64ddbf00996b5385271e.tar.bz2 |
Fix "crashed extension" infobar browser crashes.
This is a general rework of how "crashed extension" infobar works
and how the extension is actually recovered after the crash.
This is a subset of original http://codereview.chromium.org/164151/ .
I had to remove the part which unloads the entire extension on crash,
because it interacts badly with other parts of the browser. I'm fixing that.
TEST=See bug.
http://crbug.com/15888
Review URL: http://codereview.chromium.org/173314
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24231 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/app/generated_resources.grd | 4 | ||||
-rw-r--r-- | chrome/browser/browser.cc | 15 | ||||
-rw-r--r-- | chrome/browser/extensions/crashed_extension_infobar.cc | 56 | ||||
-rw-r--r-- | chrome/browser/extensions/crashed_extension_infobar.h | 46 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_host.cc | 77 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_host.h | 4 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_prefs.cc | 12 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_prefs.h | 3 | ||||
-rw-r--r-- | chrome/browser/extensions/extensions_service.cc | 11 | ||||
-rw-r--r-- | chrome/browser/views/extensions/extension_view.cc | 5 | ||||
-rw-r--r-- | chrome/browser/views/extensions/extension_view.h | 4 | ||||
-rw-r--r-- | chrome/chrome.gyp | 2 | ||||
-rw-r--r-- | chrome/common/notification_type.h | 2 |
13 files changed, 144 insertions, 97 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index d9354ca..70e50b4 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -2076,8 +2076,8 @@ each locale. --> </message> <!-- Extension Crashed Info Bar--> - <message name="IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON" desc="Title of the restart button in the extension crashed infobar. After the button is clicked, the extension process will be restarted."> - Restart + <message name="IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON" desc="Title of the reload button in the extension crashed infobar. After the button is clicked, the extension will be reloaded."> + Reload </message> <message name="IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE" desc="Message displayed on the extension crashed infobar."> The following extension has crashed : <ph name="EXTENSION_NAME">$1<ex>Buildbot Monitor</ex></ph> diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index ccfe665..43a232c 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -24,6 +24,7 @@ #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_shelf.h" #include "chrome/browser/download/download_started_animation.h" +#include "chrome/browser/extensions/crashed_extension_infobar.h" #include "chrome/browser/extensions/extension_disabled_infobar_delegate.h" #include "chrome/browser/find_bar.h" #include "chrome/browser/find_bar_controller.h" @@ -195,6 +196,8 @@ Browser::Browser(Type type, Profile* profile) NotificationService::AllSources()); registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, NotificationService::AllSources()); + registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED, + NotificationService::AllSources()); registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED, NotificationService::AllSources()); @@ -2095,6 +2098,18 @@ void Browser::Observe(NotificationType type, break; } + case NotificationType::EXTENSION_PROCESS_CRASHED: { + TabContents* tab_contents = GetSelectedTabContents(); + if (!tab_contents) + break; + ExtensionsService* extensions_service = + Source<ExtensionsService>(source).ptr(); + ExtensionHost* extension_host = Details<ExtensionHost>(details).ptr(); + tab_contents->AddInfoBar(new CrashedExtensionInfoBarDelegate( + tab_contents, extensions_service, extension_host->extension())); + break; + } + case NotificationType::BROWSER_THEME_CHANGED: window()->UserChangedTheme(); break; diff --git a/chrome/browser/extensions/crashed_extension_infobar.cc b/chrome/browser/extensions/crashed_extension_infobar.cc new file mode 100644 index 0000000..80ab865 --- /dev/null +++ b/chrome/browser/extensions/crashed_extension_infobar.cc @@ -0,0 +1,56 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/extensions/crashed_extension_infobar.h" + +#include "app/l10n_util.h" +#include "app/resource_bundle.h" +#include "chrome/browser/extensions/extensions_service.h" +#include "chrome/common/extensions/extension.h" +#include "grit/browser_resources.h" +#include "grit/generated_resources.h" +#include "grit/theme_resources.h" + +CrashedExtensionInfoBarDelegate::CrashedExtensionInfoBarDelegate( + TabContents* tab_contents, + ExtensionsService* extensions_service, + const Extension* extension) + : ConfirmInfoBarDelegate(tab_contents), + extensions_service_(extensions_service), + extension_id_(extension->id()), + extension_name_(extension->name()) { + DCHECK(extensions_service_); + DCHECK(!extension_id_.empty()); +} + +std::wstring CrashedExtensionInfoBarDelegate::GetMessageText() const { + return l10n_util::GetStringF(IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE, + UTF8ToWide(extension_name_)); +} + +void CrashedExtensionInfoBarDelegate::InfoBarClosed() { + delete this; +} + +SkBitmap* CrashedExtensionInfoBarDelegate::GetIcon() const { + // TODO(erikkay): Create extension-specific icon. http://crbug.com/14591 + return ResourceBundle::GetSharedInstance().GetBitmapNamed( + IDR_INFOBAR_PLUGIN_CRASHED); +} + +int CrashedExtensionInfoBarDelegate::GetButtons() const { + return BUTTON_OK; +} + +std::wstring CrashedExtensionInfoBarDelegate::GetButtonLabel( + ConfirmInfoBarDelegate::InfoBarButton button) const { + if (button == BUTTON_OK) + return l10n_util::GetString(IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON); + return ConfirmInfoBarDelegate::GetButtonLabel(button); +} + +bool CrashedExtensionInfoBarDelegate::Accept() { + extensions_service_->ReloadExtension(extension_id_); + return true; +} diff --git a/chrome/browser/extensions/crashed_extension_infobar.h b/chrome/browser/extensions/crashed_extension_infobar.h new file mode 100644 index 0000000..4a4e625 --- /dev/null +++ b/chrome/browser/extensions/crashed_extension_infobar.h @@ -0,0 +1,46 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_ +#define CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_ + +#include <string> + +#include "base/basictypes.h" +#include "chrome/browser/tab_contents/infobar_delegate.h" + +class Extension; +class ExtensionsService; +class SkBitmap; + +// This infobar will be displayed when an extension process crashes. It allows +// the user to reload the crashed extension. +class CrashedExtensionInfoBarDelegate : public ConfirmInfoBarDelegate { + public: + // |tab_contents| should point to the TabContents the infobar will be added + // to. |extension| should be the crashed extension, and |extensions_service| + // the ExtensionsService which manages that extension. + CrashedExtensionInfoBarDelegate(TabContents* tab_contents, + ExtensionsService* extensions_service, + const Extension* extension); + + // ConfirmInfoBarDelegate + virtual std::wstring GetMessageText() const; + virtual void InfoBarClosed(); + virtual SkBitmap* GetIcon() const; + virtual int GetButtons() const; + virtual std::wstring GetButtonLabel( + ConfirmInfoBarDelegate::InfoBarButton button) const; + virtual bool Accept(); + + private: + ExtensionsService* extensions_service_; + + const std::string extension_id_; + const std::string extension_name_; + + DISALLOW_COPY_AND_ASSIGN(CrashedExtensionInfoBarDelegate); +}; + +#endif // CHROME_BROWSER_EXTENSIONS_CRASHED_EXTENSION_INFOBAR_H_ diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index e43df14..304a399 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -4,7 +4,6 @@ #include "chrome/browser/extensions/extension_host.h" -#include "app/l10n_util.h" #include "app/resource_bundle.h" #include "base/string_util.h" #include "chrome/browser/browser.h" @@ -18,9 +17,6 @@ #include "chrome/browser/renderer_host/render_widget_host.h" #include "chrome/browser/renderer_host/render_widget_host_view.h" #include "chrome/browser/renderer_host/site_instance.h" -#include "chrome/browser/tab_contents/infobar_delegate.h" -#include "chrome/browser/tab_contents/tab_contents.h" -#include "chrome/browser/tab_contents/tab_contents_view.h" #include "chrome/common/bindings_policy.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/notification_service.h" @@ -31,57 +27,9 @@ #include "chrome/common/url_constants.h" #include "grit/browser_resources.h" -#include "grit/generated_resources.h" -#include "grit/theme_resources.h" #include "webkit/glue/context_menu.h" -namespace { - -class CrashedExtensionInfobarDelegate : public ConfirmInfoBarDelegate { - public: - CrashedExtensionInfobarDelegate(TabContents* tab_contents, - ExtensionHost* extension_host) - : ConfirmInfoBarDelegate(tab_contents), - extension_host_(extension_host) { - } - - virtual std::wstring GetMessageText() const { - return l10n_util::GetStringF(IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE, - UTF8ToWide(extension_host_->extension()->name())); - } - - virtual SkBitmap* GetIcon() const { - // TODO(erikkay): Create extension-specific icon. http://crbug.com/14591 - return ResourceBundle::GetSharedInstance().GetBitmapNamed( - IDR_INFOBAR_PLUGIN_CRASHED); - } - - virtual int GetButtons() const { - return BUTTON_OK; - } - - virtual std::wstring GetButtonLabel( - ConfirmInfoBarDelegate::InfoBarButton button) const { - if (button == BUTTON_OK) - return l10n_util::GetString(IDS_EXTENSION_CRASHED_INFOBAR_RESTART_BUTTON); - return ConfirmInfoBarDelegate::GetButtonLabel(button); - } - - virtual bool Accept() { - extension_host_->RecoverCrashedExtension(); - return true; - } - - private: - ExtensionHost* extension_host_; - - DISALLOW_COPY_AND_ASSIGN(CrashedExtensionInfobarDelegate); -}; - -} // namespace - - // static bool ExtensionHost::enable_dom_automation_ = false; @@ -165,21 +113,6 @@ void ExtensionHost::Observe(NotificationType type, NavigateToURL(url_); } -void ExtensionHost::RecoverCrashedExtension() { - DCHECK(!IsRenderViewLive()); -#if defined(TOOLKIT_VIEWS) - if (view_.get()) { - // The view should call us back to CreateRenderView, which is the place - // where we create the render process and fire notification. - view_->RecoverCrashedExtension(); - } else { - CreateRenderView(NULL); - } -#else - CreateRenderView(NULL); -#endif -} - void ExtensionHost::UpdatePreferredWidth(int pref_width) { #if defined(TOOLKIT_VIEWS) || defined(OS_LINUX) if (view_.get()) @@ -189,17 +122,9 @@ void ExtensionHost::UpdatePreferredWidth(int pref_width) { void ExtensionHost::RenderViewGone(RenderViewHost* render_view_host) { DCHECK_EQ(render_view_host_, render_view_host); - Browser* browser = GetBrowser(); - if (browser) { - TabContents* current_tab = browser->GetSelectedTabContents(); - if (current_tab) { - current_tab->AddInfoBar( - new CrashedExtensionInfobarDelegate(current_tab, this)); - } - } NotificationService::current()->Notify( NotificationType::EXTENSION_PROCESS_CRASHED, - Source<Profile>(profile_), + Source<ExtensionsService>(profile_->GetExtensionsService()), Details<ExtensionHost>(this)); } diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index 1d42ead..72aa7af 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -76,10 +76,6 @@ class ExtensionHost : public RenderViewHostDelegate, // Sets |url_| and navigates |render_view_host_|. void NavigateToURL(const GURL& url); - // Restarts extension's renderer process. Can be called only if the renderer - // process crashed. - void RecoverCrashedExtension(); - // RenderViewHostDelegate implementation. virtual RenderViewHostDelegate::View* GetViewDelegate(); virtual const GURL& GetURL() const { return url_; } diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index 4583944..d638847 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -389,6 +389,18 @@ void ExtensionPrefs::MigrateToPrefs(Extension* extension) { extension->manifest_value()->DeepCopy()); } +FilePath ExtensionPrefs::GetExtensionPath(const std::string& extension_id) { + const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref); + if (!dict || dict->GetSize() == 0) + return FilePath(); + + std::wstring path; + if (!dict->GetString(ASCIIToWide(extension_id) + L"." + kPrefPath, &path)) + return FilePath(); + + return install_directory_.Append(FilePath::FromWStringHack(path)); +} + bool ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id, const std::wstring& key, Value* data_value) { diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 59adfd8..11c7e31 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -56,6 +56,9 @@ class ExtensionPrefs { // Ensure old extensions have fully up-to-date prefs values. void MigrateToPrefs(Extension* extension); + // Returns extension path based on extension ID, or empty FilePath on error. + FilePath GetExtensionPath(const std::string& extension_id); + // Returns base extensions install directory. const FilePath& install_directory() const { return install_directory_; } diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index 0331366..377423f 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -168,11 +168,13 @@ void ExtensionsService::UpdateExtension(const std::string& id, } void ExtensionsService::ReloadExtension(const std::string& extension_id) { - Extension* extension = GetExtensionById(extension_id); - FilePath extension_path = extension->path(); + // Unload the extension if it's loaded. + if (GetExtensionById(extension_id)) + UnloadExtension(extension_id); - UnloadExtension(extension_id); - LoadExtension(extension_path); + // At this point we have to reconstruct the path from prefs, because + // we have no information about this extension in memory. + LoadExtension(extension_prefs_->GetExtensionPath(extension_id)); } void ExtensionsService::UninstallExtension(const std::string& extension_id, @@ -531,7 +533,6 @@ void ExtensionsService::OnExternalExtensionFound(const std::string& id, NULL); // no client (silent install) } - // ExtensionsServicesBackend ExtensionsServiceBackend::ExtensionsServiceBackend( diff --git a/chrome/browser/views/extensions/extension_view.cc b/chrome/browser/views/extensions/extension_view.cc index 63f37db..7b39336 100644 --- a/chrome/browser/views/extensions/extension_view.cc +++ b/chrome/browser/views/extensions/extension_view.cc @@ -145,11 +145,6 @@ void ExtensionView::ViewHierarchyChanged(bool is_add, CreateWidgetHostView(); } -void ExtensionView::RecoverCrashedExtension() { - CleanUp(); - CreateWidgetHostView(); -} - void ExtensionView::HandleMouseEvent() { if (container_) container_->OnExtensionMouseEvent(this); diff --git a/chrome/browser/views/extensions/extension_view.h b/chrome/browser/views/extensions/extension_view.h index 75f7a3f..1e08f79 100644 --- a/chrome/browser/views/extensions/extension_view.h +++ b/chrome/browser/views/extensions/extension_view.h @@ -59,10 +59,6 @@ class ExtensionView : public views::NativeViewHost { virtual void ViewHierarchyChanged(bool is_add, views::View *parent, views::View *child); - // Call after extension process crash to re-initialize view, so that - // extension content can be rendered again. - void RecoverCrashedExtension(); - private: friend class ExtensionHost; diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index c97fbf4..bd616d3 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -1042,6 +1042,8 @@ 'browser/download/save_types.h', 'browser/encoding_menu_controller.cc', 'browser/encoding_menu_controller.h', + 'browser/extensions/crashed_extension_infobar.cc', + 'browser/extensions/crashed_extension_infobar.h', 'browser/extensions/crx_installer.cc', 'browser/extensions/crx_installer.h', 'browser/extensions/extension_bookmarks_module.cc', diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index 0b351a6..ba74687 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -636,7 +636,7 @@ class NotificationType { EXTENSION_PROCESS_CREATED, // Sent when extension render process crashes. The details are - // an ExtensionHost*. + // an ExtensionHost* and the source is an ExtensionsService*. EXTENSION_PROCESS_CRASHED, // Sent when the contents or order of toolstrips in the shelf model change. |