Reload force-installed extensions on crash/force-close

chrome/browser/background/background_contents_service.cc

 // Copyright (c) 2012 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/background/background_contents_service.h"
 
 #include "apps/app_load_service.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/background/background_contents_service_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #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/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/background_info.h"
 #include "chrome/common/extensions/extension.h"
@@ skipping 12 matching lines @@
 #include "ui/gfx/image/image.h"
 
 using content::SiteInstance;
 using content::WebContents;
 using extensions::BackgroundInfo;
 using extensions::Extension;
 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 extensions/app that will

[bartfab (slow), 2013/08/27 15:37:44]

Nit: s:extension/app:app/extension:

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+// 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,
                             const Extension* extension)
       : profile_(profile),
         is_hosted_app_(extension->is_hosted_app()),
         is_platform_app_(extension->is_platform_app()),
         extension_id_(extension->id()) {
   }
 
@@ skipping 20 matching lines @@
         service->LoadBackgroundContentsForExtension(profile_,
                                                     copied_extension_id);
     } else if (is_platform_app_) {
       apps::AppLoadService::Get(profile_)->
           RestartApplication(copied_extension_id);
     } else {
       extensions::ExtensionSystem::Get(profile_)->extension_service()->
           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 {
     return NULL;
   }
 
  private:
   virtual ~CrashNotificationDelegate() {}
 
   Profile* profile_;
   bool is_hosted_app_;
   bool is_platform_app_;
   std::string extension_id_;
 
   DISALLOW_COPY_AND_ASSIGN(CrashNotificationDelegate);
 };
 
+// Empty delegate for the 'app/extension is misbehaving' popup balloon. which is

[bartfab (slow), 2013/08/27 15:37:44]

Nit: s/./,/

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+// 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);
   if (icon.IsEmpty()) {
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
     notification_icon = rb.GetImageNamed(IDR_EXTENSION_DEFAULT_ICON);
   }
   string16 title;  // no notification title
   DesktopNotificationService::AddIconNotification(extension_url,
                                                   title,
                                                   message,
                                                   notification_icon,
                                                   string16(),
                                                   delegate.get(),
                                                   profile);
 }
 #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,
+                 const 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,
   // so we'll bind all the pertinent data here.
   extensions::ImageLoader::Get(profile)->LoadImageAsync(
       extension,
       resource,
       gfx::Size(size, size),
       base::Bind(
           &NotificationImageReady,
@@ skipping 15 matching lines @@
 //
 // kRegisteredBackgroundContents:
 //    DictionaryValue {
 //       <appid_1>: { "url": <url1>, "name": <frame_name> },
 //       <appid_2>: { "url": <url2>, "name": <frame_name> },
 //         ... etc ...
 //    }
 const char kUrlKey[] = "url";
 const char kFrameNameKey[] = "name";
 
+int BackgroundContentsService::restart_delay_millis_ = 3000;  // 3 seconds.

[bartfab (slow), 2013/08/27 15:37:44]

Nit: s/millis/in_ms/

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+int BackgroundContentsService::crash_window_millis_ = 1000;  // 1 second.

[bartfab (slow), 2013/08/27 15:37:44]

Nit: s/millis/in_ms/

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+
 BackgroundContentsService::BackgroundContentsService(
     Profile* profile, const CommandLine* command_line)
-    : prefs_(NULL) {
+    : prefs_(NULL), weak_factory_(this) {
   // Don't load/store preferences if the proper switch is not enabled, or if
   // the parent profile is incognito.
   if (!profile->IsOffTheRecord() &&
       !command_line->HasSwitch(switches::kDisableRestoreBackgroundContents))
     prefs_ = profile->GetPrefs();
 
   // Listen for events to tell us when to load/unload persisted background
   // contents.
   StartObserving(profile);
 }
 
 BackgroundContentsService::~BackgroundContentsService() {
   // BackgroundContents should be shutdown before we go away, as otherwise
   // our browser process refcount will be off.
   DCHECK(contents_map_.empty());
 }
 
+// static
+void BackgroundContentsService::
+    SetCrashDelaysForForceInstalledAppsAndExtensionsForTesting(
+        const int restart_delay_in_ms,
+        const int crash_window_in_ms) {
+  restart_delay_millis_ = restart_delay_in_ms;
+  crash_window_millis_ = crash_window_in_ms;
+}
+
 std::vector<BackgroundContents*>
 BackgroundContentsService::GetBackgroundContents() const
 {
   std::vector<BackgroundContents*> contents;
   for (BackgroundContentsMap::const_iterator it = contents_map_.begin();
        it != contents_map_.end(); ++it)
     contents.push_back(it->second.contents);
   return contents;
 }
 
@@ skipping 104 matching lines @@
         if (service && service->is_ready()) {
           // Now load the manifest-specified background page. If service isn't
           // ready, then the background page will be loaded from the
           // EXTENSIONS_READY callback.
           LoadBackgroundContents(profile,
                                  BackgroundInfo::GetBackgroundURL(extension),
                                  ASCIIToUTF16("background"),
                                  UTF8ToUTF16(extension->id()));
         }
       }
-

[bartfab (slow), 2013/08/27 15:37:44]

Nit: Any reason to remove this line?

[anitawoodruff, 2013/08/27 17:34:29]

Replaced.

       // Remove any "This extension has crashed" balloons.
-      ScheduleCloseBalloon(extension->id());
+      ScheduleCloseCrashBalloon(extension->id());
       SendChangeNotification(profile);
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_PROCESS_TERMINATED:
     case chrome::NOTIFICATION_BACKGROUND_CONTENTS_TERMINATED: {
       Profile* profile = content::Source<Profile>(source).ptr();
       const Extension* extension = NULL;
       if (type == chrome::NOTIFICATION_BACKGROUND_CONTENTS_TERMINATED) {
         BackgroundContents* bg =
             content::Details<BackgroundContents>(details).ptr();
         std::string extension_id = UTF16ToASCII(
             BackgroundContentsServiceFactory::GetForProfile(profile)->
                 GetParentApplicationId(bg));
         extension =
           extensions::ExtensionSystem::Get(profile)->extension_service()->
               GetExtensionById(extension_id, false);
       } else {
         extensions::ExtensionHost* extension_host =
             content::Details<extensions::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.
-      base::MessageLoop::current()->PostTask(
-          FROM_HERE, base::Bind(&ShowBalloon, extension, profile));
+      // DesktopNotificationService. For this reason, we post the crash handling
+      // code as a task here so that it is not executed before this event.

[bartfab (slow), 2013/08/27 15:37:44]

Actually, this comment applies to the !force_installed case only. Could you move
it inside the if-block?

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+      const bool force_installed = extension->location() ==
+          extensions::Manifest::EXTERNAL_POLICY_DOWNLOAD;
+      if (!force_installed) {
+        // Notify user extension has crashed.
+        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:
       switch (content::Details<UnloadedExtensionInfo>(details)->reason) {
         case extension_misc::UNLOAD_REASON_DISABLE:    // Fall through.
         case extension_misc::UNLOAD_REASON_TERMINATE:  // Fall through.
         case extension_misc::UNLOAD_REASON_UNINSTALL:  // Fall through.
         case extension_misc::UNLOAD_REASON_BLACKLIST:
           ShutdownAssociatedBackgroundContents(
               ASCIIToUTF16(content::Details<UnloadedExtensionInfo>(details)->
@@ skipping 16 matching lines @@
         default:
           NOTREACHED();
           ShutdownAssociatedBackgroundContents(
               ASCIIToUTF16(content::Details<UnloadedExtensionInfo>(details)->
                   extension->id()));
           break;
       }
       break;
 
     case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
+      const std::string& extension_id =
+          content::Details<const Extension>(details).ptr()->id();
       // Remove any "This extension has crashed" balloons.

[bartfab (slow), 2013/08/27 15:37:44]

Nit: Update comment.

[anitawoodruff, 2013/08/27 17:34:29]

Done.

-      ScheduleCloseBalloon(
-          content::Details<const Extension>(details).ptr()->id());
+      ScheduleCloseBalloons(extension_id);
+      misbehaving_extensions_.erase(extension_id);
+      extension_crashlog_map_.erase(extension_id);
       break;
     }
 
     default:
       NOTREACHED();
       break;
   }
 }
 
+void BackgroundContentsService::RestartForceInstalledExtensionOnCrash(

[bartfab (slow), 2013/08/27 15:37:44]

The method order in the .cc file does not seem to match that in the .h file.

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+    const Extension* extension, Profile* profile) {
+  const std::string& extension_id = extension->id();
+  const bool alreadyNotified = misbehaving_extensions_.find(extension_id) !=

[bartfab (slow), 2013/08/27 15:37:44]

s/alreadyNotified/already_notified/

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+      misbehaving_extensions_.end();
+  std::queue<base::TimeTicks>& crashes = extension_crashlog_map_[extension_id];
+  const base::TimeDelta duration =

[bartfab (slow), 2013/08/27 15:37:44]

Nit: |duration| is fairly generic. How about |crash_loop_window|? Or maybe I a
just being too verbose.

[anitawoodruff, 2013/08/27 17:34:29]

Changed to 'recent_time_window'.

+      base::TimeDelta::FromMilliseconds(kMisbehaveCrashCountThreshold *
+          (restart_delay_millis_ + crash_window_millis_));
+  if (!alreadyNotified) {
+    // Show a notification if the threshold number of crashes has occurred within
+    // a given duration.
+    const bool shouldNotify =

[bartfab (slow), 2013/08/27 15:37:44]

s/shouldNotify/should_notify/

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+        crashes.size() == kMisbehaveCrashCountThreshold - 1 &&
+        base::TimeTicks::Now() - crashes.front() < duration;
+    if (shouldNotify) {
+      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() > duration) {
+        crashes.pop(); // Remove old timestamps.

[bartfab (slow), 2013/08/27 15:37:44]

Nit: Put the comment above the source line.

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+      }
+      crashes.push(base::TimeTicks::Now());
+      if (crashes.size() == kMisbehaveCrashCountThreshold)
+        crashes.pop();
+    }
+  }
+  base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
+      base::Bind(&BackgroundContentsService::ReloadExtension,
+                 weak_factory_.GetWeakPtr(), extension_id, profile),
+      base::TimeDelta::FromMilliseconds(restart_delay_millis_));
+}
+
+void BackgroundContentsService::ReloadExtension(
+    const std::string extension_id, Profile* profile) {
+  extensions::ExtensionSystem* extensionSystem =
+      extensions::ExtensionSystem::Get(profile);
+  if (!extensionSystem || !extensionSystem->extension_service())
+    return;
+  if (!extensionSystem->extension_service()->
+          GetExtensionById(extension_id, false)) {
+    // Force-installed app/extension has been removed by policy.

[bartfab (slow), 2013/08/27 15:37:44]

Maybe better:

Policy has changed. The app/extension is no longer force-installed.

[anitawoodruff, 2013/08/27 17:34:29]

Done.

+    return;
+  }
+  extensionSystem->extension_service()->ReloadExtension(extension_id);
+}
+
 // Loads all background contents whose urls have been stored in prefs.
 void BackgroundContentsService::LoadBackgroundContentsFromPrefs(
     Profile* profile) {
   if (!prefs_)
     return;
   const DictionaryValue* contents =
       prefs_->GetDictionary(prefs::kRegisteredBackgroundContents);
   if (!contents)
     return;
   ExtensionService* extensions_service =
@@ skipping 202 matching lines @@
 }
 
 void BackgroundContentsService::BackgroundContentsOpened(
     BackgroundContentsOpenedDetails* details) {
   // Add the passed object to our list. Should not already be tracked.
   DCHECK(!IsTracked(details->contents));
   DCHECK(!details->application_id.empty());
   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
 // BackgroundContents is being tracked by this instance.
 bool BackgroundContentsService::IsTracked(
     BackgroundContents* background_contents) const {
   return !GetParentApplicationId(background_contents).empty();
 }
 
 void BackgroundContentsService::BackgroundContentsShutdown(
@@ skipping 27 matching lines @@
     bool user_gesture,
     bool* was_blocked) {
   Browser* browser = chrome::FindLastActiveWithProfile(
       Profile::FromBrowserContext(new_contents->GetBrowserContext()),
       chrome::GetActiveDesktop());
   if (browser) {
     chrome::AddWebContents(browser, NULL, new_contents, disposition,
                            initial_pos, user_gesture, was_blocked);
   }
 }