GCMS has automatic checks for CheckExternalConnections. It will be

GCMS has automatic checks for CheckExternalConnections. It will be
called automatically for all callers of AddAccountToCookie. The
invokation is hidden within the GCMS.

BUG=466799
Committed: https://crrev.com/485c1f9c06b4cfae87789c564a7337317dd01ee6
Cr-Commit-Position: refs/heads/master@{#324627}

[Mike Lerman, 2015-04-08 19:55:59]

mlerman@chromium.org changed reviewers:
+ rogerta@chromium.org

[Mike Lerman, 2015-04-08 19:56:00]

Hi Roger,

PTAL.

Thanks!

[Roger Tawa OOO till Jul 10th, 2015-04-08 21:24:17]

lgtm, a couple of comments below.

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
File components/signin/core/browser/gaia_cookie_manager_service.cc (right):

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
components/signin/core/browser/gaia_cookie_manager_service.cc:197:
FireGetCheckConnectionInfoCompleted(false);
Since we are doing this only once now, should we add retry with exponential back
off here too?  Can probably just use the same backoff helper instance
|gaia_auth_fetcher_backoff_| from GaiaCookieManagerService.

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
components/signin/core/browser/gaia_cookie_manager_service.cc:263:
FireGetCheckConnectionInfoCompleted(bool succeeded) {
Nit: is this function name still appropriate?

[Mike Lerman, 2015-04-09 14:40:09]

Thanks Roger! Comments addressed.

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
File components/signin/core/browser/gaia_cookie_manager_service.cc (right):

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
components/signin/core/browser/gaia_cookie_manager_service.cc:197:
FireGetCheckConnectionInfoCompleted(false);
On 2015/04/08 21:24:17, Roger Tawa wrote:
> Since we are doing this only once now, should we add retry with exponential
back
> off here too?  Can probably just use the same backoff helper instance
> |gaia_auth_fetcher_backoff_| from GaiaCookieManagerService.

Ok and done. Will use the same gaia_auth_fetcher_ too as the GCMS.

https://codereview.chromium.org/1070563004/diff/20001/components/signin/core/...
components/signin/core/browser/gaia_cookie_manager_service.cc:263:
FireGetCheckConnectionInfoCompleted(bool succeeded) {
On 2015/04/08 21:24:17, Roger Tawa wrote:
> Nit: is this function name still appropriate?

Removed "Fire". It's cleaner.

[Mike Lerman, 2015-04-09 14:40:16]

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

[Mike Lerman, 2015-04-09 14:40:16]

The patchset sent to the CQ was uploaded after l-g-t-m from rogerta@chromium.org
Link to the patchset: https://codereview.chromium.org/1070563004/#ps40001
(title: "ExternalCCFetcher uses backoff, and the GCMS' gaia_fetcher objects.")

[commit-bot: I haz the power, 2015-04-09 14:41:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070563004/40001

[commit-bot: I haz the power, 2015-04-09 16:31:20]

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

[commit-bot: I haz the power, 2015-04-09 16:31:21]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike Lerman, 2015-04-09 17:57:44]

Had to tweak the account_reconcilor_unittests since StartReconcile() now calls
ListAccounts rather than ExternalCCCheckResult, so we need a different
FakeResponse for the TestUrlFetcher.

[Mike Lerman, 2015-04-09 17:57:50]

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

[Mike Lerman, 2015-04-09 17:57:50]

The patchset sent to the CQ was uploaded after l-g-t-m from rogerta@chromium.org
Link to the patchset: https://codereview.chromium.org/1070563004/#ps60001
(title: "AccountReconcilor tests need a fake list_accounts response, not a fake
check_cc_results response.")

[commit-bot: I haz the power, 2015-04-09 17:58:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070563004/60001

[commit-bot: I haz the power, 2015-04-09 19:49:33]

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

[commit-bot: I haz the power, 2015-04-09 19:49:34]

This issue passed the CQ dry run.

[Mike Lerman, 2015-04-10 14:18:02]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2015-04-10 14:18:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070563004/60001

[commit-bot: I haz the power, 2015-04-10 14:19:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-10 14:20:42]

Patchset 4 (id:??) landed as
https://crrev.com/485c1f9c06b4cfae87789c564a7337317dd01ee6
Cr-Commit-Position: refs/heads/master@{#324627}