Commit 1c232911 authored by apacible's avatar apacible Committed by Commit bot

Update component extension reload backoff logic.

Responding to wez@'s comments in crrev.com/2281323002.

This change removes a BackoffEntry for a component extension if it has loaded and there has been no additional failures for that extension within 60s.

BUG=639104

Review-Url: https://codereview.chromium.org/2310463002
Cr-Commit-Position: refs/heads/master@{#417315}
parent 2a37eb58
...@@ -280,7 +280,9 @@ int BackgroundContentsService::restart_delay_in_ms_ = 3000; // 3 seconds. ...@@ -280,7 +280,9 @@ int BackgroundContentsService::restart_delay_in_ms_ = 3000; // 3 seconds.
BackgroundContentsService::BackgroundContentsService( BackgroundContentsService::BackgroundContentsService(
Profile* profile, Profile* profile,
const base::CommandLine* command_line) const base::CommandLine* command_line)
: prefs_(NULL), extension_registry_observer_(this) { : prefs_(NULL),
extension_registry_observer_(this),
weak_ptr_factory_(this) {
// Don't load/store preferences if the parent profile is incognito. // Don't load/store preferences if the parent profile is incognito.
if (!profile->IsOffTheRecord()) if (!profile->IsOffTheRecord())
prefs_ = profile->GetPrefs(); prefs_ = profile->GetPrefs();
...@@ -478,6 +480,23 @@ void BackgroundContentsService::OnExtensionLoaded( ...@@ -478,6 +480,23 @@ 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())) {
ComponentExtensionBackoffEntryMap::const_iterator it =
component_backoff_map_.find(extension->id());
if (it != component_backoff_map_.end()) {
net::BackoffEntry* entry = component_backoff_map_[extension->id()].get();
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE,
base::Bind(&BackgroundContentsService::MaybeClearBackoffEntry,
weak_ptr_factory_.GetWeakPtr(), extension->id(),
entry->failure_count()),
base::TimeDelta::FromSeconds(60));
}
}
// Close the crash notification balloon for the app/extension, if any. // Close the crash notification balloon for the app/extension, if any.
ScheduleCloseBalloon(extension->id(), profile); ScheduleCloseBalloon(extension->id(), profile);
SendChangeNotification(profile); SendChangeNotification(profile);
...@@ -540,21 +559,21 @@ void BackgroundContentsService::RestartForceInstalledExtensionOnCrash( ...@@ -540,21 +559,21 @@ void BackgroundContentsService::RestartForceInstalledExtensionOnCrash(
// If the extension was a component extension, use exponential backoff when // If the extension was a component extension, use exponential backoff when
// attempting to reload. // attempting to reload.
if (extensions::Manifest::IsComponentLocation(extension->location())) { if (extensions::Manifest::IsComponentLocation(extension->location())) {
ExtensionBackoffEntryMap::const_iterator it = ComponentExtensionBackoffEntryMap::const_iterator it =
backoff_map_.find(extension->id()); component_backoff_map_.find(extension->id());
// Create a BackoffEntry if this is the first time we try to reload this // Create a BackoffEntry if this is the first time we try to reload this
// particular extension. // particular extension.
if (it == backoff_map_.end()) { if (it == component_backoff_map_.end()) {
std::unique_ptr<net::BackoffEntry> backoff_entry( std::unique_ptr<net::BackoffEntry> backoff_entry(
new net::BackoffEntry(&kExtensionReloadBackoffPolicy)); new net::BackoffEntry(&kExtensionReloadBackoffPolicy));
backoff_map_.insert( component_backoff_map_.insert(
std::pair<extensions::ExtensionId, std::pair<extensions::ExtensionId,
std::unique_ptr<net::BackoffEntry>>( std::unique_ptr<net::BackoffEntry>>(
extension->id(), std::move(backoff_entry))); extension->id(), std::move(backoff_entry)));
} }
net::BackoffEntry* entry = backoff_map_[extension->id()].get(); net::BackoffEntry* entry = component_backoff_map_[extension->id()].get();
entry->InformOfRequest(false); entry->InformOfRequest(false);
restart_delay = entry->GetTimeUntilRelease().InMilliseconds(); restart_delay = entry->GetTimeUntilRelease().InMilliseconds();
} }
...@@ -604,6 +623,22 @@ void BackgroundContentsService::SendChangeNotification(Profile* profile) { ...@@ -604,6 +623,22 @@ void BackgroundContentsService::SendChangeNotification(Profile* profile) {
content::Details<BackgroundContentsService>(this)); content::Details<BackgroundContentsService>(this));
} }
void BackgroundContentsService::MaybeClearBackoffEntry(
const std::string extension_id,
int expected_failure_count) {
ComponentExtensionBackoffEntryMap::const_iterator it =
component_backoff_map_.find(extension_id);
if (it == component_backoff_map_.end())
return;
net::BackoffEntry* entry = component_backoff_map_[extension_id].get();
// Only remove the BackoffEntry if there has has been no failure for
// |extension_id| since loading.
if (entry->failure_count() == expected_failure_count)
component_backoff_map_.erase(it);
}
void BackgroundContentsService::LoadBackgroundContentsForExtension( void BackgroundContentsService::LoadBackgroundContentsForExtension(
Profile* profile, Profile* profile,
const std::string& extension_id) { const std::string& extension_id) {
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/gtest_prod_util.h" #include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "chrome/browser/background/background_contents.h" #include "chrome/browser/background/background_contents.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
...@@ -216,6 +217,11 @@ class BackgroundContentsService : private content::NotificationObserver, ...@@ -216,6 +217,11 @@ class BackgroundContentsService : private content::NotificationObserver,
// set of background apps as new background contents are opened/closed). // set of background apps as new background contents are opened/closed).
void SendChangeNotification(Profile* profile); void SendChangeNotification(Profile* profile);
// Checks whether there has been additional |extension_id| failures. If not,
// delete the BackoffEntry corresponding to |extension_id|, if exists.
void MaybeClearBackoffEntry(const std::string extension_id,
int expected_failure_count);
// Delay (in ms) before restarting a force-installed extension that crashed. // Delay (in ms) before restarting a force-installed extension that crashed.
static int restart_delay_in_ms_; static int restart_delay_in_ms_;
...@@ -243,13 +249,15 @@ class BackgroundContentsService : private content::NotificationObserver, ...@@ -243,13 +249,15 @@ class BackgroundContentsService : private content::NotificationObserver,
// Map associating component extensions that have attempted to reload with a // Map associating component extensions that have attempted to reload with a
// BackoffEntry keeping track of retry timing. // BackoffEntry keeping track of retry timing.
typedef std::map<extensions::ExtensionId, std::unique_ptr<net::BackoffEntry>> typedef std::map<extensions::ExtensionId, std::unique_ptr<net::BackoffEntry>>
ExtensionBackoffEntryMap; ComponentExtensionBackoffEntryMap;
ExtensionBackoffEntryMap backoff_map_; ComponentExtensionBackoffEntryMap component_backoff_map_;
ScopedObserver<extensions::ExtensionRegistry, ScopedObserver<extensions::ExtensionRegistry,
extensions::ExtensionRegistryObserver> extensions::ExtensionRegistryObserver>
extension_registry_observer_; extension_registry_observer_;
base::WeakPtrFactory<BackgroundContentsService> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(BackgroundContentsService); DISALLOW_COPY_AND_ASSIGN(BackgroundContentsService);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment