Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(73)

Unified Diff: chrome/browser/background/background_contents_service.cc

Issue 23427003: Reload force-installed extensions on crash/force-close (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: good2.crx was committed manually, this should now work! Created 7 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/background/background_contents_service.cc
diff --git a/chrome/browser/background/background_contents_service.cc b/chrome/browser/background/background_contents_service.cc
index 1e81d648bb7e383048e299743f62cf688b09ae99..8b308e227590bdd6a60c2a85095e5184eee013e3 100644
--- a/chrome/browser/background/background_contents_service.cc
+++ b/chrome/browser/background/background_contents_service.cc
@@ -22,9 +22,11 @@
#include "chrome/browser/extensions/image_loader.h"
#include "chrome/browser/notifications/desktop_notification_service.h"
#include "chrome/browser/notifications/notification.h"
+#include "chrome/browser/notifications/notification_delegate.h"
#include "chrome/browser/notifications/notification_ui_manager.h"
#include "chrome/browser/prefs/scoped_user_pref_update.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_tabstrip.h"
@@ -54,19 +56,39 @@ using extensions::UnloadedExtensionInfo;
namespace {
-const char kNotificationPrefix[] = "app.background.crashed.";
+const char kCrashNotificationPrefix[] = "app.background.crashed.";
+const char kMisbehaveNotificationPrefix[] = "app.background.misbehaved.";
-void CloseBalloon(const std::string& id) {
- g_browser_process->notification_ui_manager()->CancelById(id);
+// Number of recent crashes of a force-installed app/extension that will
+// trigger an 'App/Extension is misbehaving' balloon.
+const unsigned int kMisbehaveCrashCountThreshold = 5;
+
+void CloseBalloon(const std::string& balloon_id) {
+ g_browser_process->notification_ui_manager()->
+ CancelById(balloon_id);
}
-void ScheduleCloseBalloon(const std::string& extension_id) {
+// Closes the balloon with this id.
+void ScheduleCloseBalloon(const std::string& balloon_id) {
if (!base::MessageLoop::current()) // For unit_tests
return;
base::MessageLoop::current()->PostTask(
- FROM_HERE, base::Bind(&CloseBalloon, kNotificationPrefix + extension_id));
+ FROM_HERE, base::Bind(&CloseBalloon, balloon_id));
+}
+
+// Closes the crash notification balloon for the app/extension with this id.
+void ScheduleCloseCrashBalloon(const std::string& extension_id) {
+ ScheduleCloseBalloon(kCrashNotificationPrefix + extension_id);
}
+// Closes all notification balloons relating to the app/extension with this id.
+void ScheduleCloseBalloons(const std::string& extension_id) {
+ ScheduleCloseBalloon(kMisbehaveNotificationPrefix + extension_id);
+ ScheduleCloseBalloon(kCrashNotificationPrefix + extension_id);
+}
+
+// Delegate for the 'app/extension has crashed' popup balloon. Restarts the
+// app/extension when the balloon is clicked.
class CrashNotificationDelegate : public NotificationDelegate {
public:
CrashNotificationDelegate(Profile* profile,
@@ -107,15 +129,15 @@ class CrashNotificationDelegate : public NotificationDelegate {
ReloadExtension(copied_extension_id);
}
- // Closing the balloon here should be OK, but it causes a crash on Mac
- // http://crbug.com/78167
- ScheduleCloseBalloon(copied_extension_id);
+ // Closing the 'app/extension has crashed' balloon here should be OK, but it
+ // causes a crash on Mac (http://crbug.com/78167).
+ ScheduleCloseCrashBalloon(copied_extension_id);
}
virtual bool HasClickedListener() OVERRIDE { return true; }
virtual std::string id() const OVERRIDE {
- return kNotificationPrefix + extension_id_;
+ return kCrashNotificationPrefix + extension_id_;
}
virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE {
@@ -133,12 +155,48 @@ class CrashNotificationDelegate : public NotificationDelegate {
DISALLOW_COPY_AND_ASSIGN(CrashNotificationDelegate);
};
+// Empty delegate for the 'app/extension is misbehaving' popup balloon, which is
+// triggered if a force-installed app/extension gets stuck in a crash/reload
+// cycle. Doesn't do anything on click because force-installed apps/extensions
+// get restarted automatically.
+class MisbehaveNotificationDelegate : public NotificationDelegate {
+ public:
+ explicit MisbehaveNotificationDelegate(const Extension* extension)
+ : extension_id_(extension->id()) {
+ }
+
+ virtual void Display() OVERRIDE {}
+
+ virtual void Error() OVERRIDE {}
+
+ virtual void Close(bool by_user) OVERRIDE {}
+
+ virtual void Click() OVERRIDE {}
+
+ virtual bool HasClickedListener() OVERRIDE { return true; }
+
+ virtual std::string id() const OVERRIDE {
+ return kMisbehaveNotificationPrefix + extension_id_;
+ }
+
+ virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE {
+ return NULL;
+ }
+
+ private:
+ virtual ~MisbehaveNotificationDelegate() {}
+
+ std::string extension_id_;
+
+ DISALLOW_COPY_AND_ASSIGN(MisbehaveNotificationDelegate);
+};
+
#if defined(ENABLE_NOTIFICATIONS)
void NotificationImageReady(
const std::string extension_name,
const string16 message,
const GURL extension_url,
- scoped_refptr<CrashNotificationDelegate> delegate,
+ scoped_refptr<NotificationDelegate> delegate,
Profile* profile,
const gfx::Image& icon) {
gfx::Image notification_icon(icon);
@@ -157,19 +215,32 @@ void NotificationImageReady(
}
#endif
-void ShowBalloon(const Extension* extension, Profile* profile) {
+// Show a popup notification balloon with a crash message for a given app/
+// extension. If |force_installed| is true we show an 'App/extension
+// is misbehaving' message instead of a crash message.
+void ShowBalloon(const Extension* extension, Profile* profile,
+ bool force_installed) {
#if defined(ENABLE_NOTIFICATIONS)
- string16 message = l10n_util::GetStringFUTF16(
- extension->is_app() ? IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE :
- IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE,
- UTF8ToUTF16(extension->name()));
-
+ string16 message;
+ scoped_refptr<NotificationDelegate> delegate;
+ if (force_installed) {
+ message = l10n_util::GetStringFUTF16(
+ extension->is_app() ?
+ IDS_BACKGROUND_MISBEHAVING_APP_BALLOON_MESSAGE :
+ IDS_BACKGROUND_MISBEHAVING_EXTENSION_BALLOON_MESSAGE,
+ UTF8ToUTF16(extension->name()));
+ delegate = new MisbehaveNotificationDelegate(extension);
+ } else {
+ message = l10n_util::GetStringFUTF16(
+ extension->is_app() ? IDS_BACKGROUND_CRASHED_APP_BALLOON_MESSAGE :
+ IDS_BACKGROUND_CRASHED_EXTENSION_BALLOON_MESSAGE,
+ UTF8ToUTF16(extension->name()));
+ delegate = new CrashNotificationDelegate(profile, extension);
+ }
extension_misc::ExtensionIcons size(extension_misc::EXTENSION_ICON_MEDIUM);
extensions::ExtensionResource resource =
extensions::IconsInfo::GetIconResource(
extension, size, ExtensionIconSet::MATCH_SMALLER);
- scoped_refptr<CrashNotificationDelegate> delegate =
- new CrashNotificationDelegate(profile, extension);
// We can't just load the image in the Observe method below because, despite
// what this method is called, it may call the callback synchronously.
// However, it's possible that the extension went away during the interim,
@@ -188,6 +259,25 @@ void ShowBalloon(const Extension* extension, Profile* profile) {
#endif
}
+void ReloadExtension(
+ const std::string& extension_id, Profile* profile) {
+ if (g_browser_process->IsShuttingDown() ||
+ !g_browser_process->profile_manager()->IsValidProfile(profile)) {
+ return;
+ }
+ extensions::ExtensionSystem* extension_system =
+ extensions::ExtensionSystem::Get(profile);
+ if (!extension_system || !extension_system->extension_service())
+ return;
+ if (!extension_system->extension_service()->
+ GetTerminatedExtension(extension_id)) {
+ // Either the app/extension was uninstalled by policy or it has since
+ // been restarted successfully by someone else (the user).
+ return;
+ }
+ extension_system->extension_service()->ReloadExtension(extension_id);
+}
+
} // namespace
// Keys for the information we store about individual BackgroundContents in
@@ -205,6 +295,9 @@ void ShowBalloon(const Extension* extension, Profile* profile) {
const char kUrlKey[] = "url";
const char kFrameNameKey[] = "name";
+int BackgroundContentsService::restart_delay_in_ms_ = 3000; // 3 seconds.
+int BackgroundContentsService::crash_window_in_ms_ = 1000; // 1 second.
+
BackgroundContentsService::BackgroundContentsService(
Profile* profile, const CommandLine* command_line)
: prefs_(NULL) {
@@ -225,6 +318,14 @@ BackgroundContentsService::~BackgroundContentsService() {
DCHECK(contents_map_.empty());
}
+// static
+void BackgroundContentsService::
+ SetCrashDelaysForForceInstalledAppsAndExtensionsForTesting(
+ int restart_delay_in_ms, int crash_window_in_ms) {
+ restart_delay_in_ms_ = restart_delay_in_ms;
+ crash_window_in_ms_ = crash_window_in_ms;
+}
+
std::vector<BackgroundContents*>
BackgroundContentsService::GetBackgroundContents() const
{
@@ -350,8 +451,8 @@ void BackgroundContentsService::Observe(
}
}
- // Remove any "This extension has crashed" balloons.
- ScheduleCloseBalloon(extension->id());
+ // Remove any "app/extension has crashed" balloons.
+ ScheduleCloseCrashBalloon(extension->id());
SendChangeNotification(profile);
break;
}
@@ -376,13 +477,21 @@ void BackgroundContentsService::Observe(
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.
- base::MessageLoop::current()->PostTask(
- FROM_HERE, base::Bind(&ShowBalloon, extension, profile));
+ const bool force_installed = extension->location() ==
+ extensions::Manifest::EXTERNAL_POLICY_DOWNLOAD;
+ if (!force_installed) {
+ // 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, we post the crash
+ // handling code as a task here so that it is not executed before this
+ // event.
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(&ShowBalloon, extension, profile, false));
+ } else {
+ // Restart the extension; notify user if crash recurs frequently.
+ RestartForceInstalledExtensionOnCrash(extension, profile);
+ }
break;
}
case chrome::NOTIFICATION_EXTENSION_UNLOADED:
@@ -419,9 +528,12 @@ void BackgroundContentsService::Observe(
break;
case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
- // Remove any "This extension has crashed" balloons.
- ScheduleCloseBalloon(
- content::Details<const Extension>(details).ptr()->id());
+ const std::string& extension_id =
+ content::Details<const Extension>(details).ptr()->id();
+ // Remove any balloons shown for this app/extension.
+ ScheduleCloseBalloons(extension_id);
+ misbehaving_extensions_.erase(extension_id);
+ extension_crashlog_map_.erase(extension_id);
break;
}
@@ -431,6 +543,42 @@ void BackgroundContentsService::Observe(
}
}
+void BackgroundContentsService::RestartForceInstalledExtensionOnCrash(
+ const Extension* extension, Profile* profile) {
+ const std::string& extension_id = extension->id();
+ const bool already_notified = misbehaving_extensions_.find(extension_id) !=
+ misbehaving_extensions_.end();
+ std::queue<base::TimeTicks>& crashes = extension_crashlog_map_[extension_id];
+ const base::TimeDelta recent_time_window =
+ base::TimeDelta::FromMilliseconds(kMisbehaveCrashCountThreshold *
+ (restart_delay_in_ms_ + crash_window_in_ms_));
+ if (!already_notified) {
+ // Show a notification if the threshold number of crashes has occurred
+ // within a recent time period.
+ const bool should_notify =
+ crashes.size() == kMisbehaveCrashCountThreshold - 1 &&
+ base::TimeTicks::Now() - crashes.front() < recent_time_window;
+ if (should_notify) {
+ base::MessageLoop::current()->PostTask(FROM_HERE,
+ base::Bind(&ShowBalloon, extension, profile, true));
+ misbehaving_extensions_.insert(extension_id);
+ extension_crashlog_map_.erase(extension_id);
+ } else {
+ while (!crashes.empty() &&
+ base::TimeTicks::Now() - crashes.front() > recent_time_window) {
+ // Remove old timestamps.
+ crashes.pop();
+ }
+ crashes.push(base::TimeTicks::Now());
+ if (crashes.size() == kMisbehaveCrashCountThreshold)
+ crashes.pop();
+ }
+ }
+ base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+ base::Bind(&ReloadExtension, extension_id, profile),
+ base::TimeDelta::FromMilliseconds(restart_delay_in_ms_));
+}
+
// Loads all background contents whose urls have been stored in prefs.
void BackgroundContentsService::LoadBackgroundContentsFromPrefs(
Profile* profile) {
@@ -653,7 +801,7 @@ void BackgroundContentsService::BackgroundContentsOpened(
contents_map_[details->application_id].contents = details->contents;
contents_map_[details->application_id].frame_name = details->frame_name;
- ScheduleCloseBalloon(UTF16ToASCII(details->application_id));
+ ScheduleCloseCrashBalloon(UTF16ToASCII(details->application_id));
}
// Used by test code and debug checks to verify whether a given
« no previous file with comments | « chrome/browser/background/background_contents_service.h ('k') | chrome/browser/policy/policy_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698