Reload force-installed extensions on crash/force-close

chrome/browser/background/background_contents_service.h

Index: chrome/browser/background/background_contents_service.h
diff --git a/chrome/browser/background/background_contents_service.h b/chrome/browser/background/background_contents_service.h
index 0d99a92bfa02024994266a5295ff8b83db5c6abe..e5ff29455f1f413cca5232fa83e4dd40129847e8 100644
--- a/chrome/browser/background/background_contents_service.h
+++ b/chrome/browser/background/background_contents_service.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_BACKGROUND_BACKGROUND_CONTENTS_SERVICE_H_
 
 #include <map>
+#include <queue>
 #include <string>
 #include <vector>
 
@@ -52,6 +53,9 @@ class BackgroundContentsService : private content::NotificationObserver,
   BackgroundContentsService(Profile* profile, const CommandLine* command_line);
   virtual ~BackgroundContentsService();
 
+  static void SetCrashDelaysForTesting(const base::TimeDelta& restart_delay,

[bartfab (slow), 2013/08/26 13:26:53]

Nit 1: Add comment that explain what this does.

Nit 2: The name may be a bit too generic. Since these delays apply to
force-installed extensions only, it would be best to mention that in the method
name.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+                                       const base::TimeDelta& crash_window);
+
   // Returns the BackgroundContents associated with the passed application id,
   // or NULL if none.
   BackgroundContents* GetAppBackgroundContents(const string16& appid);
@@ -170,6 +174,9 @@ class BackgroundContentsService : private content::NotificationObserver,
   // set of background apps as new background contents are opened/closed).
   void SendChangeNotification(Profile* profile);
 
+  static base::TimeDelta restart_delay_;

[bartfab (slow), 2013/08/26 13:26:53]

Static initializers are not allowed in Chrome. You can have static PODs but not
complex classes. You will have to make the statics PODs (say ints holding the
number of milliseconds or base::TimeDelta::ToInternalValue()s).

[anitawoodruff, 2013/08/26 21:45:23]

Done. Made them ints, and only convert to TimeDelta at the last minute. Should I
still pass the ints by reference (using int&) to the SetRestartDelays method?

[bartfab (slow), 2013/08/27 09:21:29]

For PODs, passing by reference is not necessary.

+  static base::TimeDelta crash_window_;
+
   // PrefService used to store list of background pages (or NULL if this is
   // running under an incognito profile).
   PrefService* prefs_;
@@ -190,6 +197,15 @@ class BackgroundContentsService : private content::NotificationObserver,
   typedef std::map<string16, BackgroundContentsInfo> BackgroundContentsMap;
   BackgroundContentsMap contents_map_;
 
+  // Map associating extension IDs with their most recent crash timestamps.

[bartfab (slow), 2013/08/26 13:26:53]

Nit: Add a note that this is used for force-installed apps/extensions only.

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+  // Key: extension id.
+  // Value: queue containing up to 5 most recent crash timestamps.
+  std::map<std::string, std::queue<base::TimeTicks> > extension_crashlog_map_;

[bartfab (slow), 2013/08/26 13:26:53]

Nit 1: #include <queue>
Nit 2: #include "base/time.h"

[anitawoodruff, 2013/08/26 21:45:23]

Done Nit 2. <queue> I'd already included on line 9. B-)

+
+  // Map containing ids of extensions for which we have already sent
+  // 'Extension is misbehaving' notifications.

[bartfab (slow), 2013/08/26 13:26:53]

Nit 1: Instead of "sent notifications," better say "shown ballons".
Nit 2: Add a note that this applies to force-installed apps/extensions only.

[anitawoodruff, 2013/08/26 21:45:23]

Done.
I've changed the wording of comments to say apps/extensions instead of
extensions everywhere, but should I rename all these variables too? It's gonna
get a bit verbose..

[bartfab (slow), 2013/08/27 09:21:29]

The variable naming is OK. Comments are for verbose stuff. Variable names should
be as-crisp-as-possible. I think you have the right balance here now.

+  std::set<std::string> misbehaving_extensions_;

[bartfab (slow), 2013/08/26 13:26:53]

Nit: #include <set>

[anitawoodruff, 2013/08/26 21:45:23]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(BackgroundContentsService);
 };