Update component extension reload backoff logic.

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
Committed: https://crrev.com/1c2329116c6d802429f431ec322b1f8ccc8143f4
Cr-Commit-Position: refs/heads/master@{#417315}

[apacible, 2016-09-02 19:45:59]

Patchset #2 (id:20001) has been deleted

[apacible, 2016-09-02 19:46:02]

Patchset #1 (id:1) has been deleted

[apacible, 2016-09-02 19:46:24]

Description was changed from

==========
BUG=
==========

to

==========
BUG=639104
==========

[apacible, 2016-09-02 19:50:25]

Description was changed from

==========
BUG=639104
==========

to

==========
Make component extension reload backoff logic more robust.

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

Changes include:
- Clearing any remaining BackoffEntrys in the BackgroundContentsService
destructor. Extensions (including components) aren't "uninstalled" or "unloaded"
in situations like closing the browser session. This cleans up any
BackoffEntrys.
- Removing a BackoffEntry for a component extension if it has loaded and has not
'failed' within 60s.

BUG=639104
==========

[apacible, 2016-09-02 19:50:45]

Patchset #1 (id:40001) has been deleted

[apacible, 2016-09-02 19:51:54]

Description was changed from

==========
Make component extension reload backoff logic more robust.

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

Changes include:
- Clearing any remaining BackoffEntrys in the BackgroundContentsService
destructor. Extensions (including components) aren't "uninstalled" or "unloaded"
in situations like closing the browser session. This cleans up any
BackoffEntrys.
- Removing a BackoffEntry for a component extension if it has loaded and has not
'failed' within 60s.

BUG=639104
==========

to

==========
Update component extension reload backoff logic.

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

Changes include:
- Clearing any remaining BackoffEntrys in the BackgroundContentsService
destructor. Extensions (including components) aren't "uninstalled" or "unloaded"
in situations like closing the browser session. This cleans up any
BackoffEntrys.
- Removing a BackoffEntry for a component extension if it has loaded and has not
'failed' within 60s.

BUG=639104
==========

[apacible, 2016-09-02 19:52:06]

apacible@chromium.org changed reviewers:
+ atwilson@chromium.org, wez@chromium.org

[apacible, 2016-09-02 19:52:06]

PTAL, thanks!

[apacible, 2016-09-02 20:57:54]

The CQ bit was checked by apacible@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-02 20:58:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-02 21:53:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-02 21:53:34]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
File chrome/browser/background/background_contents_service.cc (right):

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:301:
backoff_map_.clear();
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.

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:497:
base::Unretained(this), extension->id(), entry->failure_count()),
What happens if you reach here but the BackgroundContentsService gets deleted
before 60s have elapsed?

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:639: // since loading.
You mean no additional failure since expected_failure_count was reached?

[apacible, 2016-09-03 00:58:48]

The CQ bit was checked by apacible@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-03 00:59:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[apacible, 2016-09-03 01:10:47]

Patchset #2 (id:80001) has been deleted

[apacible, 2016-09-03 01:12:53]

The CQ bit was checked by apacible@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-03 01:13:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
File chrome/browser/background/background_contents_service.cc (right):

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:301:
backoff_map_.clear();
On 2016/09/02 23:54:44, Wez wrote:
> 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.

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_|.

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:497:
base::Unretained(this), extension->id(), entry->failure_count()),
On 2016/09/02 23:54:44, Wez wrote:
> What happens if you reach here but the BackgroundContentsService gets deleted
> before 60s have elapsed?

Talked offline -- switching to using weak ptr.

https://codereview.chromium.org/2310463002/diff/60001/chrome/browser/backgrou...
chrome/browser/background/background_contents_service.cc:639: // since loading.
On 2016/09/02 23:54:43, Wez wrote:
> You mean no additional failure since expected_failure_count was reached?

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

[apacible, 2016-09-03 01:15:03]

Description was changed from

==========
Update component extension reload backoff logic.

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

Changes include:
- Clearing any remaining BackoffEntrys in the BackgroundContentsService
destructor. Extensions (including components) aren't "uninstalled" or "unloaded"
in situations like closing the browser session. This cleans up any
BackoffEntrys.
- Removing a BackoffEntry for a component extension if it has loaded and has not
'failed' within 60s.

BUG=639104
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-09-03 02:26:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-03 02:26:41]

Dry run: This issue passed the CQ dry run.

[Wez, 2016-09-03 06:48:34]

lgtm

https://codereview.chromium.org/2310463002/diff/100001/chrome/browser/backgro...
File chrome/browser/background/background_contents_service.cc (right):

https://codereview.chromium.org/2310463002/diff/100001/chrome/browser/backgro...
chrome/browser/background/background_contents_service.cc:496:
base::TimeDelta::FromMilliseconds(60000 /* 60 seconds */));
nit: Just use FromSeconds directly?

[apacible, 2016-09-05 00:33:41]

The CQ bit was checked by apacible@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-05 00:33:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[apacible, 2016-09-05 00:34:38]

https://codereview.chromium.org/2310463002/diff/100001/chrome/browser/backgro...
File chrome/browser/background/background_contents_service.cc (right):

https://codereview.chromium.org/2310463002/diff/100001/chrome/browser/backgro...
chrome/browser/background/background_contents_service.cc:496:
base::TimeDelta::FromMilliseconds(60000 /* 60 seconds */));
On 2016/09/03 06:48:34, Wez wrote:
> nit: Just use FromSeconds directly?

Done.

[commit-bot: I haz the power, 2016-09-05 01:08:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-05 01:08:56]

Dry run: This issue passed the CQ dry run.

[apacible, 2016-09-06 23:48:04]

Friendly ping!

[apacible, 2016-09-08 07:10:05]

atwilson@ - PTAL, thanks!

[Andrew T Wilson (Slow), 2016-09-08 12:00:15]

lgtm

[Andrew T Wilson (Slow), 2016-09-08 12:00:15]

lgtm

lgtm

[apacible, 2016-09-08 16:06:49]

The CQ bit was checked by apacible@chromium.org

[apacible, 2016-09-08 16:06:50]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org
Link to the patchset: https://codereview.chromium.org/2310463002/#ps120001
(title: "Changes per wez@'s comment.")

[commit-bot: I haz the power, 2016-09-08 16:07:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-08 16:50:07]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-09-08 16:50:08]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-09-08 16:52:52]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/1c2329116c6d802429f431ec322b1f8ccc8143f4
Cr-Commit-Position: refs/heads/master@{#417315}
==========

[commit-bot: I haz the power, 2016-09-08 16:52:53]

Patchset 3 (id:??) landed as
https://crrev.com/1c2329116c6d802429f431ec322b1f8ccc8143f4
Cr-Commit-Position: refs/heads/master@{#417315}