Attempt Manager Refactor and Opt-In Pipeline Refactor

Attempt Manager Refactor and Opt-In Pipeline Refactor

Refactored the attempt manager to use a more obvious naming scheme as well as
participate in promises for retries.

This also entailed reexamination of all attempt managers in Chrome Now.
Opt-in checking as a result became a first class state change citizen and is
no longer part of the card checking pipeline, fixing a few opt-in aggression
issues. With this change, we don't care how often a push message comes through
that may indicate that Google Now is enabled.

BUG=360649, 362306
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268302

[skare_, 2014-04-29 00:12:10]

lgtm

[rgustafson, 2014-04-29 00:38:54]

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:434: * Calculates the soonest
poll time from a map of groups.
Some mention of absolute (vs relative) time?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:673: }
What happens when this is called from the attempt manager and requestFromServer
rejects because of a bad request?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1182: function pollOptedIn() {
If we're distinguishing between rechecking and polling,t his could use some
changing.

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1199:
requestAndUpdateOptedIn().then(function(optedIn) {
requestAndUpdateOptedIn starts the 7 day poll, so it happens when the quick opt
in polls are done if they're still not opted in, but it's not very explicit.
Would it help to be more explicit about switching from the quick check opt in
poll and the 7 day poll? And separate "getting current status" from starting the
7-day timer?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1295:
instrumented.storage.onChanged.addListener(function(changes, areaName) {
Is this going to trigger so much compared to what we're checking that it's going
to be a kinda performance drag while we are running?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/utility.js (right):

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/utility.js:880: * Schedules the alarm with a
random factor reduce the chance that all clients
to reduce

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/utility.js:920: * Schedules an expoential
backoff retry.
exponential

[robliao, 2014-05-01 01:22:26]

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:434: * Calculates the soonest
poll time from a map of groups.
On 2014/04/29 00:38:55, rgustafson wrote:
> Some mention of absolute (vs relative) time?

Done.

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:673: }
Handled by parent caller now.
On 2014/04/29 00:38:55, rgustafson wrote:
> What happens when this is called from the attempt manager and
requestFromServer
> rejects because of a bad request?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1182: function pollOptedIn() {
On 2014/04/29 00:38:55, rgustafson wrote:
> If we're distinguishing between rechecking and polling,t his could use some
> changing.

Done.

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1199:
requestAndUpdateOptedIn().then(function(optedIn) {
The pollOptedInWithRecheck should only get called if the user is not opted into
Google Now.
This means we might as well reschedule the 7 day check for each answer opt-in =
false answer.

The interesting case is if we mistakenly process an opt-in check when the user
is already opted in and the state is set up where we assume it's opted in AND
the HTTP request fails. We don't want to schedule and opt-in check since the
machinery is already running.

If the user is actually opted out. Then we want to perform a state change to
opted-out.

I've moved this automatic scheduling out.

On 2014/04/29 00:38:55, rgustafson wrote:
> requestAndUpdateOptedIn starts the 7 day poll, so it happens when the quick
opt
> in polls are done if they're still not opted in, but it's not very explicit.
> Would it help to be more explicit about switching from the quick check opt in
> poll and the 7 day poll? And separate "getting current status" from starting
the
> 7-day timer?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:1295:
instrumented.storage.onChanged.addListener(function(changes, areaName) {
The checks here are fast, so we should be okay.
On 2014/04/29 00:38:55, rgustafson wrote:
> Is this going to trigger so much compared to what we're checking that it's
going
> to be a kinda performance drag while we are running?

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/utility.js (right):

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/utility.js:880: * Schedules the alarm with a
random factor reduce the chance that all clients
On 2014/04/29 00:38:55, rgustafson wrote:
> to reduce

Done.

https://codereview.chromium.org/248473003/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/utility.js:920: * Schedules an expoential
backoff retry.
On 2014/04/29 00:38:55, rgustafson wrote:
> exponential

Done.

[rgustafson, 2014-05-03 00:41:20]

lgtm

https://codereview.chromium.org/248473003/diff/40001/chrome/browser/resources...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/248473003/diff/40001/chrome/browser/resources...
chrome/browser/resources/google_now/background.js:653: * Performs the an opt-in
poll without an immediate recheck.
either 'the' or 'an'

[robliao, 2014-05-05 17:24:29]

xiyuan: Please provide owner approval for this CL. Thanks!

https://codereview.chromium.org/248473003/diff/40001/chrome/browser/resources...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/248473003/diff/40001/chrome/browser/resources...
chrome/browser/resources/google_now/background.js:653: * Performs the an opt-in
poll without an immediate recheck.
On 2014/05/03 00:41:21, rgustafson wrote:
> either 'the' or 'an'

Done.

https://codereview.chromium.org/248473003/diff/60001/chrome/browser/resources...
File chrome/browser/resources/google_now/utility.js (right):

https://codereview.chromium.org/248473003/diff/60001/chrome/browser/resources...
chrome/browser/resources/google_now/utility.js:196: if (isInDebugMode())
This is a codesync artifact.

[xiyuan, 2014-05-05 17:46:32]

lgtm

[robliao, 2014-05-05 17:46:47]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-05-05 17:49:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/robliao@chromium.org/248473003/60001

[commit-bot: I haz the power, 2014-05-05 19:37:36]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
  mac_chromium_rel on tryserver.chromium

[commit-bot: I haz the power, 2014-05-05 23:05:47]

Change committed as 268302