diff options
author | sadrul@chromium.org <sadrul@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-01 18:40:11 +0000 |
---|---|---|
committer | sadrul@chromium.org <sadrul@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-01 18:40:11 +0000 |
commit | b2bcbfe9e2eb8f9f855b52eb48fcf0685a81fbe4 (patch) | |
tree | 1ba47530882df1b894eec92ef7a4fa572a3539d5 | |
parent | b550a3dcd79d555c1b7f1049e56b558b377a61c7 (diff) | |
download | chromium_src-b2bcbfe9e2eb8f9f855b52eb48fcf0685a81fbe4.zip chromium_src-b2bcbfe9e2eb8f9f855b52eb48fcf0685a81fbe4.tar.gz chromium_src-b2bcbfe9e2eb8f9f855b52eb48fcf0685a81fbe4.tar.bz2 |
Revert 80128 - apps: Notify the user if an app's background page crashes.
Add a new notification for when an app's background page crashes, and use this
notification message to show a message to the user and allow to restart the app.
Extension-crashes also create balloon notifications instead of infobars.
BUG=none
TEST=existing ExtensionCrashRecoveryTest.* tests.
Review URL: http://codereview.chromium.org/6731038
TBR=sadrul@chromium.org
Review URL: http://codereview.chromium.org/6788018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80193 0039d316-1c4b-4281-b951-d872f2087c98
18 files changed, 244 insertions, 263 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index e976280..3361c2b 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -217,12 +217,6 @@ Other platform defines such as use_titlecase are declared in build/common.gypi. <message name="IDS_BACKGROUND_APP_INSTALLED_BALLOON_BODY" desc="The contents of the balloon that is displayed when a background app is installed"> <ph name="APP_NAME">$1<ex>Background App</ex></ph> will launch at system startup and continue to run in the background even once you've closed all other <ph name="PRODUCT_NAME">$2<ex>Google Chrome</ex></ph> windows. </message> - <message name="IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE" desc="The contents of the balloon that is displayed when a background app crashes"> - <ph name="APP_NAME">$1<ex>Background App</ex></ph> has crashed. Click on this balloon to restart the app. - </message> - <message name="IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE" desc="The contents of the balloon that is displayed when an extension crashes"> - <ph name="APP_NAME">$1<ex>Extension</ex></ph> has crashed. Click on this balloon to reload the extension. - </message> <message name="IDS_SHOWFULLHISTORY_LINK" desc="The label of the Show Full History link at the bottom of the back/forward menu."> Show Full History @@ -3516,6 +3510,14 @@ Other platform defines such as use_titlecase are declared in build/common.gypi. Notification: <ph name="NOTIFICATION_NAME">$1<ex>http://www.domain.com</ex></ph> </message> + <!-- Extension Crashed Info Bar--> + <message name="IDS_EXTENSION_CRASHED_INFOBAR_RELOAD_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> + </message> + <!-- Theme preview info bar --> <message name="IDS_THEME_INSTALL_INFOBAR_LABEL" desc="Text displayed on an infobar when a theme has been installed."> Installed theme "<ph name="THEME_NAME">$1<ex>Snowflake Theme</ex></ph>" diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc index 05c5932..fbeca19 100644 --- a/chrome/browser/automation/testing_automation_provider.cc +++ b/chrome/browser/automation/testing_automation_provider.cc @@ -2291,7 +2291,8 @@ ListValue* TestingAutomationProvider::GetInfobarsInfo(TabContents* tc) { DictionaryValue* infobar_item = new DictionaryValue; InfoBarDelegate* infobar = tc->GetInfoBarDelegateAt(i); if (infobar->AsConfirmInfoBarDelegate()) { - // Also covers ThemeInstalledInfoBarDelegate. + // Also covers ThemeInstalledInfoBarDelegate and + // CrashedExtensionInfoBarDelegate. infobar_item->SetString("type", "confirm_infobar"); ConfirmInfoBarDelegate* confirm_infobar = infobar->AsConfirmInfoBarDelegate(); diff --git a/chrome/browser/background_contents_service.cc b/chrome/browser/background_contents_service.cc index d3e028e..4c63bdc 100644 --- a/chrome/browser/background_contents_service.cc +++ b/chrome/browser/background_contents_service.cc @@ -9,12 +9,7 @@ #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "base/values.h" -#include "chrome/browser/browser_process.h" -#include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/notifications/desktop_notification_service.h" -#include "chrome/browser/notifications/notification.h" -#include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/profiles/profile.h" @@ -28,67 +23,6 @@ #include "content/browser/tab_contents/tab_contents.h" #include "content/common/notification_service.h" #include "content/common/notification_type.h" -#include "grit/generated_resources.h" -#include "ui/base/l10n/l10n_util.h" - -namespace { - -class CrashNotificationDelegate : public NotificationDelegate { - public: - CrashNotificationDelegate(Profile* profile, const Extension* extension) - : profile_(profile), - is_app_(extension->is_app()), - extension_id_(extension->id()) { - } - - ~CrashNotificationDelegate() { - } - - void Display() {} - - void Error() {} - - void Close(bool by_user) {} - - void Click() { - if (is_app_) { - profile_->GetBackgroundContentsService()-> - LoadBackgroundContentsForExtension(profile_, extension_id_); - } else { - profile_->GetExtensionService()->ReloadExtension(extension_id_); - } - - g_browser_process->notification_ui_manager()->CancelById(id()); - } - - std::string id() const { - return "app.background.crashed." + extension_id_; - } - - private: - Profile* profile_; - bool is_app_; - std::string extension_id_; - - DISALLOW_COPY_AND_ASSIGN(CrashNotificationDelegate); -}; - -void ShowBalloon(const Extension* extension, Profile* profile) { - string16 message = l10n_util::GetStringFUTF16( - extension->is_app() ? IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE : - IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE, - UTF8ToUTF16(extension->name())); - string16 content_url = DesktopNotificationService::CreateDataUrl( - extension->GetIconURL(Extension::EXTENSION_ICON_SMALLISH, - ExtensionIconSet::MATCH_BIGGER), - string16(), message, WebKit::WebTextDirectionDefault); - Notification notification( - extension->url(), GURL(content_url), string16(), - string16(), new CrashNotificationDelegate(profile, extension)); - g_browser_process->notification_ui_manager()->Add(notification, profile); -} - -} // Keys for the information we store about individual BackgroundContents in // prefs. There is one top-level DictionaryValue (stored at @@ -151,19 +85,11 @@ void BackgroundContentsService::StartObserving(Profile* profile) { // during shutdown or if the render process dies). registrar_.Add(this, NotificationType::BACKGROUND_CONTENTS_DELETED, Source<Profile>(profile)); - // Track when the BackgroundContents navigates to a new URL so we can update // our persisted information as appropriate. registrar_.Add(this, NotificationType::BACKGROUND_CONTENTS_NAVIGATED, Source<Profile>(profile)); - // Track when the extensions crash so that the user can be notified - // about it, and the crashed contents can be restarted. - registrar_.Add(this, NotificationType::EXTENSION_PROCESS_TERMINATED, - Source<Profile>(profile)); - registrar_.Add(this, NotificationType::BACKGROUND_CONTENTS_TERMINATED, - Source<Profile>(profile)); - // Listen for extensions to be unloaded so we can shutdown associated // BackgroundContents. registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, @@ -188,36 +114,7 @@ void BackgroundContentsService::Observe(NotificationType type, DCHECK(IsTracked(Details<BackgroundContents>(details).ptr())); RegisterBackgroundContents(Details<BackgroundContents>(details).ptr()); break; - - case NotificationType::EXTENSION_PROCESS_TERMINATED: - case NotificationType::BACKGROUND_CONTENTS_TERMINATED: { - Profile* profile = Source<Profile>(source).ptr(); - const Extension* extension = NULL; - if (type.value == NotificationType::BACKGROUND_CONTENTS_TERMINATED) { - BackgroundContents* bg = - Details<BackgroundContents>(details).ptr(); - const std::string ext_id = UTF16ToASCII(profile-> - GetBackgroundContentsService()->GetParentApplicationId(bg)); - extension = - profile->GetExtensionService()->GetExtensionById(ext_id, false); - } else { - ExtensionHost* extension_host = Details<ExtensionHost>(details).ptr(); - extension = extension_host->extension(); - } - - if (!extension) - break; - - // When an extension crashes, EXTENSION_PROCESS_TERMINATED is followed by - // an EXTENSION_UNLOADED notification. This UNLOADED signal causes all the - // notifications for this extension to be cancelled by - // DesktopNotificationService. For this reason, instead of showing the - // balloon right now, we schedule it to show a little later. - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - NewRunnableFunction(&ShowBalloon, extension, profile)); - break; - } - case NotificationType::EXTENSION_UNLOADED: + case NotificationType::EXTENSION_UNLOADED: switch (Details<UnloadedExtensionInfo>(details)->reason) { case UnloadedExtensionInfo::DISABLE: // Intentionally fall through. case UnloadedExtensionInfo::UNINSTALL: @@ -255,9 +152,17 @@ void BackgroundContentsService::LoadBackgroundContentsFromPrefs( DCHECK(extensions_service); for (DictionaryValue::key_iterator it = contents->begin_keys(); it != contents->end_keys(); ++it) { + DictionaryValue* dict; + contents->GetDictionaryWithoutPathExpansion(*it, &dict); + string16 frame_name; + std::string url; + dict->GetString(kUrlKey, &url); + dict->GetString(kFrameNameKey, &frame_name); + // Check to make sure that the parent extension is still enabled. const Extension* extension = extensions_service->GetExtensionById( *it, false); + if (!extension) { // We should never reach here - it should not be possible for an app // to become uninstalled without the associated BackgroundContents being @@ -267,43 +172,13 @@ void BackgroundContentsService::LoadBackgroundContentsFromPrefs( << *it; return; } - LoadBackgroundContentsFromDictionary(profile, *it, contents); + LoadBackgroundContents(profile, + GURL(url), + frame_name, + UTF8ToUTF16(*it)); } } -void BackgroundContentsService::LoadBackgroundContentsForExtension( - Profile* profile, - const std::string& extension_id) { - if (!prefs_) - return; - const DictionaryValue* contents = - prefs_->GetDictionary(prefs::kRegisteredBackgroundContents); - if (!contents) - return; - LoadBackgroundContentsFromDictionary(profile, extension_id, contents); -} - -void BackgroundContentsService::LoadBackgroundContentsFromDictionary( - Profile* profile, - const std::string& extension_id, - const DictionaryValue* contents) { - ExtensionService* extensions_service = profile->GetExtensionService(); - DCHECK(extensions_service); - - DictionaryValue* dict; - contents->GetDictionaryWithoutPathExpansion(extension_id, &dict); - if (dict == NULL) - return; - string16 frame_name; - std::string url; - dict->GetString(kUrlKey, &url); - dict->GetString(kFrameNameKey, &frame_name); - LoadBackgroundContents(profile, - GURL(url), - frame_name, - UTF8ToUTF16(extension_id)); -} - void BackgroundContentsService::LoadBackgroundContents( Profile* profile, const GURL& url, diff --git a/chrome/browser/background_contents_service.h b/chrome/browser/background_contents_service.h index 55bc365..1723ae7 100644 --- a/chrome/browser/background_contents_service.h +++ b/chrome/browser/background_contents_service.h @@ -18,7 +18,6 @@ #include "webkit/glue/window_open_disposition.h" class CommandLine; -class DictionaryValue; class PrefService; class Profile; class TabContents; @@ -75,11 +74,6 @@ class BackgroundContentsService : private NotificationObserver, const string16& frame_name, const string16& application_id); - // Load the registered BackgroundContents for the specified extension. This - // is typically used to reload a crashed background page. - void LoadBackgroundContentsForExtension(Profile* profile, - const std::string& extension_id); - private: friend class BackgroundContentsServiceTest; friend class MockBackgroundContents; @@ -104,12 +98,6 @@ class BackgroundContentsService : private NotificationObserver, // Loads all registered BackgroundContents at startup. void LoadBackgroundContentsFromPrefs(Profile* profile); - // Load a BackgroundContent; the settings are read from the provided - // dictionary. - void LoadBackgroundContentsFromDictionary(Profile* profile, - const std::string& extension_id, - const DictionaryValue* contents); - // Creates a single BackgroundContents associated with the specified |appid|, // creates an associated RenderView with the name specified by |frame_name|, // and navigates to the passed |url|. diff --git a/chrome/browser/extensions/crashed_extension_infobar.cc b/chrome/browser/extensions/crashed_extension_infobar.cc new file mode 100644 index 0000000..4ee6bbd --- /dev/null +++ b/chrome/browser/extensions/crashed_extension_infobar.cc @@ -0,0 +1,69 @@ +// Copyright (c) 2011 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 "base/utf_string_conversions.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/common/extensions/extension.h" +#include "grit/generated_resources.h" +#include "grit/theme_resources.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/base/resource/resource_bundle.h" + +CrashedExtensionInfoBarDelegate::CrashedExtensionInfoBarDelegate( + TabContents* tab_contents, + ExtensionService* 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()); +} + +CrashedExtensionInfoBarDelegate::~CrashedExtensionInfoBarDelegate() { +} + +bool CrashedExtensionInfoBarDelegate::ShouldExpire( + const NavigationController::LoadCommittedDetails& details) const { + return false; +} + +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); +} + +CrashedExtensionInfoBarDelegate* + CrashedExtensionInfoBarDelegate::AsCrashedExtensionInfoBarDelegate() { + return this; +} + +string16 CrashedExtensionInfoBarDelegate::GetMessageText() const { + return l10n_util::GetStringFUTF16(IDS_EXTENSION_CRASHED_INFOBAR_MESSAGE, + UTF8ToUTF16(extension_name_)); +} + +int CrashedExtensionInfoBarDelegate::GetButtons() const { + return BUTTON_OK; +} + +string16 CrashedExtensionInfoBarDelegate::GetButtonLabel( + InfoBarButton button) const { + DCHECK_EQ(BUTTON_OK, button); + return l10n_util::GetStringUTF16( + IDS_EXTENSION_CRASHED_INFOBAR_RELOAD_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..d6ec360 --- /dev/null +++ b/chrome/browser/extensions/crashed_extension_infobar.h @@ -0,0 +1,53 @@ +// Copyright (c) 2011 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_ +#pragma once + +#include <string> + +#include "base/basictypes.h" +#include "chrome/browser/tab_contents/confirm_infobar_delegate.h" + +class Extension; +class ExtensionService; +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 ExtensionService which manages that extension. + CrashedExtensionInfoBarDelegate(TabContents* tab_contents, + ExtensionService* extensions_service, + const Extension* extension); + + const std::string extension_id() { return extension_id_; } + + private: + virtual ~CrashedExtensionInfoBarDelegate(); + + // ConfirmInfoBarDelegate: + virtual bool ShouldExpire( + const NavigationController::LoadCommittedDetails& details) const; + virtual void InfoBarClosed(); + virtual SkBitmap* GetIcon() const; + virtual CrashedExtensionInfoBarDelegate* AsCrashedExtensionInfoBarDelegate(); + virtual string16 GetMessageText() const; + virtual int GetButtons() const; + virtual string16 GetButtonLabel(InfoBarButton button) const; + virtual bool Accept(); + + ExtensionService* 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_crash_recovery_browsertest.cc b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc index d57e16d..bc1b719 100644 --- a/chrome/browser/extensions/extension_crash_recovery_browsertest.cc +++ b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc @@ -3,15 +3,13 @@ // found in the LICENSE file. #include "base/process_util.h" -#include "chrome/browser/browser_process.h" +#include "chrome/browser/extensions/crashed_extension_infobar.h" #include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/extensions/extension_process_manager.h" #include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/notifications/balloon_host.h" -#include "chrome/browser/notifications/notification_delegate.h" -#include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/tab_contents/confirm_infobar_delegate.h" #include "chrome/browser/tabs/tab_strip_model.h" #include "chrome/browser/ui/browser.h" #include "chrome/test/ui_test_utils.h" @@ -30,33 +28,23 @@ class ExtensionCrashRecoveryTest : public ExtensionBrowserTest { return browser()->profile()->GetExtensionProcessManager(); } - Balloon* GetNotificationDelegate(size_t index) { - NotificationUIManager* manager = - g_browser_process->notification_ui_manager(); - BalloonCollection::Balloons balloons = - manager->balloon_collection()->GetActiveBalloons(); - return balloons.at(index); + ConfirmInfoBarDelegate* GetInfoBarDelegate(size_t index) { + TabContents* current_tab = browser()->GetSelectedTabContents(); + EXPECT_LT(index, current_tab->infobar_count()); + return current_tab->GetInfoBarDelegateAt(index)->AsConfirmInfoBarDelegate(); } - void AcceptNotification(size_t index) { - Balloon* balloon = GetNotificationDelegate(index); - ASSERT_TRUE(balloon); - balloon->OnClick(); + void AcceptInfoBar(size_t index) { + ConfirmInfoBarDelegate* infobar = GetInfoBarDelegate(index); + ASSERT_TRUE(infobar); + infobar->Accept(); WaitForExtensionLoad(); } - void CancelNotification(size_t index) { - Balloon* balloon = GetNotificationDelegate(index); - ASSERT_TRUE(balloon); - balloon->view()->GetHost()->Shutdown(); - } - - size_t CountBalloons() { - NotificationUIManager* manager = - g_browser_process->notification_ui_manager(); - BalloonCollection::Balloons balloons = - manager->balloon_collection()->GetActiveBalloons(); - return balloons.size(); + void CancelInfoBar(size_t index) { + ConfirmInfoBarDelegate* infobar = GetInfoBarDelegate(index); + ASSERT_TRUE(infobar); + infobar->Cancel(); } void CrashExtension(size_t index) { @@ -126,9 +114,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, Basic) { ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); ASSERT_EQ(crash_size_before + 1, GetExtensionService()->terminated_extensions()->size()); - AcceptNotification(0); + AcceptInfoBar(0); - SCOPED_TRACE("after clicking the balloon"); + SCOPED_TRACE("after clicking the infobar"); CheckExtensionConsistency(size_before); ASSERT_EQ(crash_size_before, GetExtensionService()->terminated_extensions()->size()); @@ -146,12 +134,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, MAYBE_CloseAndReload) { 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()); - CancelNotification(0); + CancelInfoBar(0); ReloadExtension(first_extension_id_); SCOPED_TRACE("after reloading"); @@ -174,9 +161,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, ReloadIndependently) { TabContents* current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(current_tab); - // The balloon should automatically hide after the extension is successfully + // The infobar should automatically hide after the extension is successfully // reloaded. - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, current_tab->infobar_count()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, @@ -188,23 +175,23 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TabContents* original_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(original_tab); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, original_tab->infobar_count()); - // Open a new tab, but the balloon will still be there. + // Open a new tab so the info bar will not be in the current tab. browser()->NewTab(); TabContents* new_current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(new_current_tab); ASSERT_NE(new_current_tab, original_tab); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(0U, new_current_tab->infobar_count()); ReloadExtension(first_extension_id_); SCOPED_TRACE("after reloading"); CheckExtensionConsistency(size_before); - // The balloon should automatically hide after the extension is successfully + // The infobar should automatically hide after the extension is successfully // reloaded. - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, original_tab->infobar_count()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, @@ -216,13 +203,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TabContents* current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(current_tab); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab->infobar_count()); // Navigate to another page. ui_test_utils::NavigateToURL(browser(), ui_test_utils::GetTestUrl(FilePath(FilePath::kCurrentDirectory), FilePath(FILE_PATH_LITERAL("title1.html")))); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab->infobar_count()); ReloadExtension(first_extension_id_); @@ -231,7 +218,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, // The infobar should automatically hide after the extension is successfully // reloaded. - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, current_tab->infobar_count()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, @@ -247,11 +234,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TabContents* current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(current_tab); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab->infobar_count()); TabContents* current_tab2 = browser2->GetSelectedTabContents(); ASSERT_TRUE(current_tab2); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab2->infobar_count()); ReloadExtension(first_extension_id_); @@ -260,8 +247,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, // Both infobars should automatically hide after the extension is successfully // reloaded. - ASSERT_EQ(0U, CountBalloons()); - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, current_tab->infobar_count()); + ASSERT_EQ(0U, current_tab2->infobar_count()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, @@ -277,11 +264,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TabContents* current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(current_tab); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab->infobar_count()); TabContents* current_tab2 = browser2->GetSelectedTabContents(); ASSERT_TRUE(current_tab2); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab2->infobar_count()); // Move second window into first browser so there will be multiple tabs // with the info bar for the same extension in one browser. @@ -289,7 +276,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, browser2->tabstrip_model()->DetachTabContentsAt(0); browser()->tabstrip_model()->AppendTabContents(contents, true); current_tab2 = browser()->GetSelectedTabContents(); - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab2->infobar_count()); ASSERT_NE(current_tab2, current_tab); ReloadExtension(first_extension_id_); @@ -299,10 +286,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, // Both infobars should automatically hide after the extension is successfully // reloaded. - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, current_tab2->infobar_count()); browser()->SelectPreviousTab(); ASSERT_EQ(current_tab, browser()->GetSelectedTabContents()); - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, current_tab->infobar_count()); } // Make sure that when we don't do anything about the crashed extension @@ -321,9 +308,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashFirst) { LoadSecondExtension(); CrashExtension(size_before); ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); - AcceptNotification(0); + AcceptInfoBar(0); - SCOPED_TRACE("after clicking the balloon"); + SCOPED_TRACE("after clicking the infobar"); CheckExtensionConsistency(size_before); CheckExtensionConsistency(size_before + 1); } @@ -334,9 +321,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsCrashSecond) { LoadSecondExtension(); CrashExtension(size_before + 1); ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); - AcceptNotification(0); + AcceptInfoBar(0); - SCOPED_TRACE("after clicking the balloon"); + SCOPED_TRACE("after clicking the infobar"); CheckExtensionConsistency(size_before); CheckExtensionConsistency(size_before + 1); } @@ -358,14 +345,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, GetExtensionService()->terminated_extensions()->size()); { - SCOPED_TRACE("first balloon"); - AcceptNotification(0); + SCOPED_TRACE("first infobar"); + AcceptInfoBar(0); CheckExtensionConsistency(size_before); } { - SCOPED_TRACE("second balloon"); - AcceptNotification(0); + SCOPED_TRACE("second infobar"); + AcceptInfoBar(0); CheckExtensionConsistency(size_before); CheckExtensionConsistency(size_before + 1); } @@ -383,14 +370,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); { - SCOPED_TRACE("first balloon"); - AcceptNotification(0); + SCOPED_TRACE("first infobar"); + AcceptInfoBar(0); CheckExtensionConsistency(size_before); } { - SCOPED_TRACE("second balloon"); - AcceptNotification(0); + SCOPED_TRACE("second infobar"); + AcceptInfoBar(0); CheckExtensionConsistency(size_before); CheckExtensionConsistency(size_before + 1); } @@ -419,12 +406,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TwoExtensionsIgnoreFirst) { CrashExtension(size_before); ASSERT_EQ(size_before, GetExtensionService()->extensions()->size()); - CancelNotification(0); - // Cancelling the balloon at 0 will close the balloon, and the balloon in - // index 1 will move into index 0. - AcceptNotification(0); + CancelInfoBar(0); + AcceptInfoBar(1); - SCOPED_TRACE("balloons done"); + SCOPED_TRACE("infobars done"); ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); CheckExtensionConsistency(size_before); } @@ -444,16 +429,16 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, TabContents* current_tab = browser()->GetSelectedTabContents(); ASSERT_TRUE(current_tab); // At the beginning we should have one infobar displayed for each extension. - ASSERT_EQ(2U, CountBalloons()); + ASSERT_EQ(2U, current_tab->infobar_count()); ReloadExtension(first_extension_id_); // One of the infobars should hide after the extension is reloaded. - ASSERT_EQ(1U, CountBalloons()); + ASSERT_EQ(1U, current_tab->infobar_count()); CheckExtensionConsistency(size_before); } { - SCOPED_TRACE("second: balloon"); - AcceptNotification(0); + SCOPED_TRACE("second: infobar"); + AcceptInfoBar(0); CheckExtensionConsistency(size_before); CheckExtensionConsistency(size_before + 1); } @@ -470,15 +455,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CrashAndUninstall) { ASSERT_EQ(crash_size_before + 1, GetExtensionService()->terminated_extensions()->size()); - ASSERT_EQ(1U, CountBalloons()); UninstallExtension(first_extension_id_); - MessageLoop::current()->RunAllPending(); SCOPED_TRACE("after uninstalling"); ASSERT_EQ(size_before + 1, GetExtensionService()->extensions()->size()); ASSERT_EQ(crash_size_before, GetExtensionService()->terminated_extensions()->size()); - ASSERT_EQ(0U, CountBalloons()); + ASSERT_EQ(0U, browser()->GetSelectedTabContents()->infobar_count()); } IN_PROC_BROWSER_TEST_F(ExtensionCrashRecoveryTest, CrashAndUnloadAll) { diff --git a/chrome/browser/tab_contents/background_contents.cc b/chrome/browser/tab_contents/background_contents.cc index 933d2da..b8c24d6 100644 --- a/chrome/browser/tab_contents/background_contents.cc +++ b/chrome/browser/tab_contents/background_contents.cc @@ -183,12 +183,6 @@ void BackgroundContents::Close(RenderViewHost* render_view_host) { void BackgroundContents::RenderViewGone(RenderViewHost* rvh, base::TerminationStatus status, int error_code) { - Profile* profile = rvh->process()->profile(); - NotificationService::current()->Notify( - NotificationType::BACKGROUND_CONTENTS_TERMINATED, - Source<Profile>(profile), - Details<BackgroundContents>(this)); - // Our RenderView went away, so we should go away also, so killing the process // via the TaskManager doesn't permanently leave a BackgroundContents hanging // around the system, blocking future instances from being created diff --git a/chrome/browser/tab_contents/infobar_delegate.cc b/chrome/browser/tab_contents/infobar_delegate.cc index ec77e24..ac5c757 100644 --- a/chrome/browser/tab_contents/infobar_delegate.cc +++ b/chrome/browser/tab_contents/infobar_delegate.cc @@ -44,6 +44,11 @@ ConfirmInfoBarDelegate* InfoBarDelegate::AsConfirmInfoBarDelegate() { return NULL; } +CrashedExtensionInfoBarDelegate* + InfoBarDelegate::AsCrashedExtensionInfoBarDelegate() { + return NULL; +} + ExtensionInfoBarDelegate* InfoBarDelegate::AsExtensionInfoBarDelegate() { return NULL; } diff --git a/chrome/browser/tab_contents/infobar_delegate.h b/chrome/browser/tab_contents/infobar_delegate.h index 8310d9f..f1e3e6f3 100644 --- a/chrome/browser/tab_contents/infobar_delegate.h +++ b/chrome/browser/tab_contents/infobar_delegate.h @@ -12,6 +12,7 @@ #include "webkit/glue/window_open_disposition.h" class ConfirmInfoBarDelegate; +class CrashedExtensionInfoBarDelegate; class ExtensionInfoBarDelegate; class InfoBar; class LinkInfoBarDelegate; @@ -88,6 +89,7 @@ class InfoBarDelegate { // Type-checking downcast routines: virtual ConfirmInfoBarDelegate* AsConfirmInfoBarDelegate(); + virtual CrashedExtensionInfoBarDelegate* AsCrashedExtensionInfoBarDelegate(); virtual ExtensionInfoBarDelegate* AsExtensionInfoBarDelegate(); virtual LinkInfoBarDelegate* AsLinkInfoBarDelegate(); virtual PluginInstallerInfoBarDelegate* AsPluginInstallerInfoBarDelegate(); diff --git a/chrome/browser/task_manager/task_manager_browsertest.cc b/chrome/browser/task_manager/task_manager_browsertest.cc index d5854b9..32ae49a 100644 --- a/chrome/browser/task_manager/task_manager_browsertest.cc +++ b/chrome/browser/task_manager/task_manager_browsertest.cc @@ -8,6 +8,7 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/background_contents_service.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/extensions/crashed_extension_infobar.h" #include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/notifications/desktop_notification_service.h" diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 2289b93..7a0369f 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -24,7 +24,6 @@ #include "base/utf_string_conversions.h" #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/autofill/autofill_manager.h" -#include "chrome/browser/background_contents_service.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_utils.h" #include "chrome/browser/browser_list.h" @@ -41,6 +40,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/crx_installer.h" #include "chrome/browser/extensions/extension_browser_event_router.h" #include "chrome/browser/extensions/extension_disabled_infobar_delegate.h" @@ -56,7 +56,6 @@ #include "chrome/browser/metrics/user_metrics.h" #include "chrome/browser/net/browser_url_util.h" #include "chrome/browser/net/url_fixer_upper.h" -#include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/platform_util.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/printing/cloud_print/cloud_print_setup_flow.h" @@ -68,7 +67,6 @@ #include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/sync/sync_ui_util.h" #include "chrome/browser/tab_closeable_state_watcher.h" -#include "chrome/browser/tab_contents/background_contents.h" #include "chrome/browser/tab_contents/simple_alert_infobar_delegate.h" #include "chrome/browser/tabs/tab_finder.h" #include "chrome/browser/tabs/tab_strip_model.h" @@ -3384,25 +3382,38 @@ void Browser::Observe(NotificationType type, case NotificationType::EXTENSION_PROCESS_TERMINATED: { window()->GetLocationBar()->InvalidatePageActions(); + + TabContents* tab_contents = GetSelectedTabContents(); + if (!tab_contents) + break; + ExtensionService* extensions_service = + Source<Profile>(source).ptr()->GetExtensionService(); + ExtensionHost* extension_host = Details<ExtensionHost>(details).ptr(); + tab_contents->AddInfoBar(new CrashedExtensionInfoBarDelegate( + tab_contents, extensions_service, extension_host->extension())); break; } case NotificationType::EXTENSION_UNINSTALLED: { window()->GetLocationBar()->UpdatePageActions(); - // Remove any "This extension has crashed" balloons. + // If any "This extension has crashed" InfoBarDelegates are around for + // this extension, it means that it has been uninstalled + // so just remove the remaining CrashedExtensionInfoBarDelegate objects. const UninstalledExtensionInfo* uninstalled_extension = Details<const UninstalledExtensionInfo>(details).ptr(); - RemoveCrashedExtensionBalloon(uninstalled_extension->extension_id); + RemoveCrashedExtensionInfoBar(uninstalled_extension->extension_id); break; } case NotificationType::EXTENSION_LOADED: { window()->GetLocationBar()->UpdatePageActions(); - // Remove any "This extension has crashed" balloons. + // If any "This extension has crashed" InfoBarDelegates are around for + // this extension, it means that it has been reloaded in another window + // so just remove the remaining CrashedExtensionInfoBarDelegate objects. const Extension* extension = Details<const Extension>(details).ptr(); - RemoveCrashedExtensionBalloon(extension->id()); + RemoveCrashedExtensionInfoBar(extension->id()); break; } @@ -3468,9 +3479,19 @@ void Browser::Observe(NotificationType type, } } -void Browser::RemoveCrashedExtensionBalloon(const std::string& extension_id) { - g_browser_process->notification_ui_manager()->CancelAllBySourceOrigin( - Extension::GetBaseURLFromExtensionId(extension_id)); +void Browser::RemoveCrashedExtensionInfoBar(const std::string& extension_id) { + TabStripModel* model = tab_handler_->GetTabStripModel(); + for (int m = 0; m < model->count(); ++m) { + TabContents* tab_contents = model->GetTabContentsAt(m)->tab_contents(); + for (size_t i = 0; i < tab_contents->infobar_count(); ) { + CrashedExtensionInfoBarDelegate* delegate = tab_contents-> + GetInfoBarDelegateAt(i)->AsCrashedExtensionInfoBarDelegate(); + if (delegate && delegate->extension_id() == extension_id) + tab_contents->RemoveInfoBar(delegate); + else + ++i; + } + } } /////////////////////////////////////////////////////////////////////////////// diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h index 9af0b4c..60c2d12 100644 --- a/chrome/browser/ui/browser.h +++ b/chrome/browser/ui/browser.h @@ -839,7 +839,7 @@ class Browser : public TabHandlerDelegate, const NotificationSource& source, const NotificationDetails& details); - void RemoveCrashedExtensionBalloon(const std::string& id); + void RemoveCrashedExtensionInfoBar(const std::string& id); // Overridden from ProfileSyncServiceObserver: virtual void OnStateChanged(); diff --git a/chrome/browser/ui/gtk/notifications/balloon_view_gtk.cc b/chrome/browser/ui/gtk/notifications/balloon_view_gtk.cc index 3883acd..102507c 100644 --- a/chrome/browser/ui/gtk/notifications/balloon_view_gtk.cc +++ b/chrome/browser/ui/gtk/notifications/balloon_view_gtk.cc @@ -108,11 +108,6 @@ BalloonViewImpl::BalloonViewImpl(BalloonCollection* collection) } BalloonViewImpl::~BalloonViewImpl() { - if (frame_container_) { - GtkWidget* widget = frame_container_; - frame_container_ = NULL; - gtk_widget_hide(widget); - } } void BalloonViewImpl::Close(bool by_user) { diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index c4bda76..c2f26ed 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -827,6 +827,8 @@ 'browser/extensions/convert_user_script.h', 'browser/extensions/convert_web_app.cc', 'browser/extensions/convert_web_app.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/default_apps.cc', diff --git a/chrome/test/testing_browser_process.cc b/chrome/test/testing_browser_process.cc index 516dd8e..4f78299 100644 --- a/chrome/test/testing_browser_process.cc +++ b/chrome/test/testing_browser_process.cc @@ -7,7 +7,6 @@ #include "base/string_util.h" #include "base/synchronization/waitable_event.h" #include "chrome/browser/google/google_url_tracker.h" -#include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/policy/browser_policy_connector.h" #include "chrome/browser/policy/configuration_policy_pref_store.h" #include "chrome/browser/policy/configuration_policy_provider.h" @@ -144,10 +143,7 @@ TestingBrowserProcess::extension_event_router_forwarder() { } NotificationUIManager* TestingBrowserProcess::notification_ui_manager() { - if (!notification_ui_manager_.get()) - notification_ui_manager_.reset( - NotificationUIManager::Create(local_state())); - return notification_ui_manager_.get(); + return NULL; } GoogleURLTracker* TestingBrowserProcess::google_url_tracker() { diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h index 33af9f86e..605291f 100644 --- a/chrome/test/testing_browser_process.h +++ b/chrome/test/testing_browser_process.h @@ -21,7 +21,6 @@ class IOThread; class GoogleURLTracker; -class NotificationUIManager; class PrefService; class WatchDogThread; @@ -155,7 +154,6 @@ class TestingBrowserProcess : public BrowserProcess { scoped_ptr<policy::BrowserPolicyConnector> browser_policy_connector_; scoped_ptr<GoogleURLTracker> google_url_tracker_; scoped_ptr<ProfileManager> profile_manager_; - scoped_ptr<NotificationUIManager> notification_ui_manager_; DISALLOW_COPY_AND_ASSIGN(TestingBrowserProcess); }; diff --git a/content/common/notification_type.h b/content/common/notification_type.h index 20edba4..e04c0b9 100644 --- a/content/common/notification_type.h +++ b/content/common/notification_type.h @@ -566,10 +566,6 @@ class NotificationType { // parent Profile, and the details are the BackgroundContents being deleted. BACKGROUND_CONTENTS_DELETED, - // The background contents has crashed. The source is the parent Profile, - // and the details are the BackgroundContents. - BACKGROUND_CONTENTS_TERMINATED, - // Child Processes --------------------------------------------------------- // This notification is sent when a child process host has connected to a |