Use exponential backoff for component extension reloads.

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 245 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";
 
+// Defines the backoff policy used for attempting to reload extensions.
+const net::BackoffEntry::Policy kExtensionReloadBackoffPolicy = {
+  0,                // Initial errors to ignore before applying backoff.
+  3000,             // Initial delay: 3 seconds.
+  2,                // Multiply factor.
+  0.1,              // Fuzzing percentage.
+  -1,               // Maximum backoff time: -1 for no maximum.
+  -1,               // Entry lifetime: -1 to never discard.
+  false,            // Whether to always use initial delay. No-op as there are
+                    // no initial errors to ignore.
+};
+
 int BackgroundContentsService::restart_delay_in_ms_ = 3000;  // 3 seconds.
 
 BackgroundContentsService::BackgroundContentsService(
     Profile* profile,
     const base::CommandLine* command_line)
     : prefs_(NULL), extension_registry_observer_(this) {
   // Don't load/store preferences if the parent profile is incognito.
   if (!profile->IsOffTheRecord())
     prefs_ = profile->GetPrefs();
 
@@ skipping 240 matching lines @@
   // Make sure the extension-crash balloons are removed when the extension is
   // uninstalled/reloaded. We cannot do this from UNLOADED since a crashed
   // extension is unloaded immediately after the crash, not when user reloads or
   // uninstalls the extension.
   ScheduleCloseBalloon(extension->id(), profile);
 }
 
 void BackgroundContentsService::RestartForceInstalledExtensionOnCrash(
     const Extension* extension,
     Profile* profile) {
+  int restart_delay = restart_delay_in_ms_;
+
+  // If the extension was a non-policy installed, component extension, use

[Andrew T Wilson (Slow), 2016/08/29 07:53:47]

I'm not convinced this is the right approach (adding exponential backoff and
related complexity to component extension restarts).

First of all, I don't understand why we'd treat component extensions differently
from other extensions.

Secondly, the main motivation seems to be to not have this crash crap up our
crash statistics from a few clients.

So, I'd either suggest making this code apply to all extensions, or get rid of
the complexity and don't reload component extensions.

[apacible, 2016/08/29 16:40:11]

Component extensions are loaded automatically (and retried automatically),
whereas other extensions would require user action to try to reinstall.
The other motivation is to avoid an endless reload-attempt loop while the
browser is running (regardless of whether this affects our stats).
If we don't try to reload component extensions, the user just won't be able to
use the feature during the browser session. It could just be a one off crash.

+  // exponential backoff when attempting to reload.
+  if (extensions::Manifest::IsComponentLocation(extension->location()) &&
+      !extensions::Manifest::IsPolicyLocation(extension->location())) {

[Andrew T Wilson (Slow), 2016/08/29 07:53:47]

I don't understand this - what extensions are *both* policy loaded and component
extensions?

[apacible, 2016/08/29 16:40:11]

Good catch, it's just one or the other (as extensions only have one 'source
location'). Updated.

+    ExtensionBackoffEntryMap::const_iterator it =
+        backoff_map_.find(extension->id());
+    net::BackoffEntry* backoff_entry;
+
+    // Create a BackoffEntry if this is the first time we try to reload this
+    // particular extension.
+    if (it == backoff_map_.end()) {
+      backoff_entry = new net::BackoffEntry(&kExtensionReloadBackoffPolicy);
+      backoff_map_.insert(
+          std::pair<extensions::ExtensionId, net::BackoffEntry*>(
+              extension->id(), backoff_entry));
+    } else {
+      backoff_entry = it->second;
+    }
+
+    backoff_entry->InformOfRequest(false);
+    restart_delay = backoff_entry->GetTimeUntilRelease().InMilliseconds();
+  }
+
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::Bind(&ReloadExtension, extension->id(), profile),
-      base::TimeDelta::FromMilliseconds(restart_delay_in_ms_));
+      base::TimeDelta::FromMilliseconds(restart_delay));
 }
 
 // Loads all background contents whose urls have been stored in prefs.
 void BackgroundContentsService::LoadBackgroundContentsFromPrefs(
     Profile* profile) {
   if (!prefs_)
     return;
   const base::DictionaryValue* contents =
       prefs_->GetDictionary(prefs::kRegisteredBackgroundContents);
   if (!contents)
@@ skipping 248 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);
   }
 }