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 149 matching lines @@
       if (!extension)
         break;
 
       const bool force_installed =
           extensions::Manifest::IsComponentLocation(extension->location()) ||
           extensions::Manifest::IsPolicyLocation(extension->location());
       if (!force_installed) {
         ShowBalloon(extension, profile);
       } else {
         // Restart the extension.
         RestartForceInstalledExtensionOnCrash(extension, profile);

[Devlin, 2016/08/29 20:22:04]

I'm not an OWNER here, but since I'm on the review, I'll add my $0.02. :)  To
me, it makes more sense to handle the component case here, rather than in
RestartForceInstalledExtensionOnCrash().  Before, the restart code was the same
for forced/not-forced, so it made sense.  Now, I'd rather we just put all of it
in one place (possibly in a new function) rather than needing the "force
installed" logic duplicated.

[apacible, 2016/08/29 23:26:34]

Acknowledged. Makes sense; I'll wait for atwilson@'s feedback before making
changes. Thanks!

[Andrew T Wilson (Slow), 2016/08/30 08:24:57]

I want to make sure we only do the backoff logic for component extensions, not
for policy-force-installed extensions, because I don't want people to be able to
effectively disable force-installed extensions by killing them repeatedly and
forcing a long exponential timeout.

So in the end, I think the logic is slightly different, so I'm OK with leaving
it down in the Restart() function.

[apacible, 2016/08/30 23:54:15]

Agree with both comments -- I made some changes to use unique_ptr below and
decided to just keep it in the Restart() function.

       }
       break;
     }
 
     default:
       NOTREACHED();
       break;
   }
 }
 
@@ skipping 70 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 component extension, use exponential backoff when
+  // attempting to reload.
+  if (extensions::Manifest::IsComponentLocation(extension->location())) {
+    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);

[Andrew T Wilson (Slow), 2016/08/30 08:24:57]

I think these BackoffEntry() items leak on shutdown - should we store a
unique_ptr instead?

[apacible, 2016/08/30 23:54:15]

Done. Used get() to return a pointer to the BackoffEntry to avoid releasing
ownership.

+      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);
   }
 }