Update component extension reload backoff logic.

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 <utility>
 
 #include "apps/app_load_service.h"
 #include "base/bind.h"
@@ skipping 276 matching lines @@
 
   // 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());
+
+  // 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
 void BackgroundContentsService::
     SetRestartDelayForForceInstalledAppsAndExtensionsForTesting(
         int restart_delay_in_ms) {
   restart_delay_in_ms_ = restart_delay_in_ms;
 }
 
 // static
@@ skipping 164 matching lines @@
       // 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),
                              "background",
                              base::UTF8ToUTF16(extension->id()));
     }
   }
 
+  // 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);
 }
 
 void BackgroundContentsService::OnExtensionUnloaded(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension,
     extensions::UnloadedExtensionInfo::Reason reason) {
   switch (reason) {
@@ skipping 106 matching lines @@
   }
 }
 
 void BackgroundContentsService::SendChangeNotification(Profile* profile) {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_BACKGROUND_CONTENTS_SERVICE_CHANGED,
       content::Source<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) {
   // First look if the manifest specifies a background page.
   const Extension* extension =
       extensions::ExtensionSystem::Get(profile)->extension_service()->
           GetExtensionById(extension_id, false);
   DCHECK(!extension || extension->is_hosted_app());
   if (extension && BackgroundInfo::HasBackgroundPage(extension)) {
     LoadBackgroundContents(profile,
@@ skipping 206 matching lines @@
     const gfx::Rect& initial_rect,
     bool user_gesture,
     bool* was_blocked) {
   Browser* browser = chrome::FindLastActiveWithProfile(
       Profile::FromBrowserContext(new_contents->GetBrowserContext()));
   if (browser) {
     chrome::AddWebContents(browser, NULL, new_contents, disposition,
                            initial_rect, user_gesture, was_blocked);
   }
 }