Update component extension reload backoff logic.

chrome/browser/background/background_contents_service.cc

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 52d401d5ecf1f37403302fe11cb02a187c7b7067..cd4a4dd7ba3e34c137a12bba7fcf87bc18ceeda8 100644
--- a/chrome/browser/background/background_contents_service.cc
+++ b/chrome/browser/background/background_contents_service.cc
@@ -294,6 +294,11 @@ BackgroundContentsService::~BackgroundContentsService() {
   // BackgroundContents should be shutdown before we go away, as otherwise
   // our browser process refcount will be off.
   DCHECK(contents_map_.empty());
+
+  // Clear any remaining BackoffEntry objects for extensions. This resets the
+  // backoff with a change in profile (e.g. when the browser restarts), as
+  // |this| is a keyed service.
+  backoff_map_.clear();

[Wez, 2016/09/02 23:54:44]

This will just clear the map on browser shutdown, which will happen anyway since
it's a member of this class - what I had in mind was actually clearing the
backoff entry if the extension is e.g. uninstalled, stopped, etc, so you don't
"leak" the BackoffEntrys over time while running.

[apacible, 2016/09/03 01:14:26]

Component extensions are "installed" but are loaded (when they should start
running) and unloaded (when they crash/terminate), rather than "uninstalled".
User-controlled extensions can be installed/uninstalled.

Talked offline with wez@ - removing this line and renaming |backoff_map_| to |
component_backoff_map_|.

 }
 
 // static
@@ -478,6 +483,22 @@ void BackgroundContentsService::OnExtensionLoaded(
     }
   }
 
+  // If there is an existing BackoffEntry for the extension, clear it if
+  // the component extension stays loaded for 60 seconds. This avoids the
+  // situation of effectively disabling an extension for the entire browser
+  // session if there was a periodic crash (sometimes caused by another source).
+  if (extensions::Manifest::IsComponentLocation(extension->location())) {
+    ExtensionBackoffEntryMap::const_iterator it =
+        backoff_map_.find(extension->id());
+    if (it != backoff_map_.end()) {
+      net::BackoffEntry* entry = backoff_map_[extension->id()].get();
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE,
+          base::Bind(&BackgroundContentsService::MaybeClearBackoffEntry,
+              base::Unretained(this), extension->id(), entry->failure_count()),

[Wez, 2016/09/02 23:54:44]

What happens if you reach here but the BackgroundContentsService gets deleted
before 60s have elapsed?

[apacible, 2016/09/03 01:14:27]

Talked offline -- switching to using weak ptr.

+          base::TimeDelta::FromMilliseconds(60000 /* 60 seconds */));
+    }
+  }
+
   // Close the crash notification balloon for the app/extension, if any.
   ScheduleCloseBalloon(extension->id(), profile);
   SendChangeNotification(profile);
@@ -604,6 +625,22 @@ void BackgroundContentsService::SendChangeNotification(Profile* profile) {
       content::Details<BackgroundContentsService>(this));
 }
 
+void BackgroundContentsService::MaybeClearBackoffEntry(
+    const std::string extension_id,
+    int expected_failure_count) {
+  ExtensionBackoffEntryMap::const_iterator it =
+      backoff_map_.find(extension_id);
+  if (it == backoff_map_.end())
+    return;
+
+  net::BackoffEntry* entry = backoff_map_[extension_id].get();
+
+  // Only remove the BackoffEntry if there has has been no extension failure
+  // since loading.

[Wez, 2016/09/02 23:54:43]

You mean no additional failure since expected_failure_count was reached?

[apacible, 2016/09/03 01:14:27]

Correct, or more specifically, "no additional failure for this particular
extension".

+  if (entry->failure_count() == expected_failure_count)
+    backoff_map_.erase(it);
+}
+
 void BackgroundContentsService::LoadBackgroundContentsForExtension(
     Profile* profile,
     const std::string& extension_id) {