cros: Fix multiple LoadCredentials on network change

cros: Fix multiple LoadCredentials on network change

TokenService::LoadCredentials does not handle multiple concurrent
load requests. However, OAuth2LoginManager::ContinueSessionRestore
calls it when there is a network change and could end up having
multiple load requests in fly. This would cause the token of the
primary account being wrongly revoked. Adding a bool flag in
OAuth2LoginManager to prevent overlapping LoadCredentials calls.

BUG=682695
Review-Url: https://codereview.chromium.org/2650653007
Cr-Commit-Position: refs/heads/master@{#446180}
Committed: https://chromium.googlesource.com/chromium/src/+/29db500ff5473c212ebe3f1cebbc00ec87a023b8

[xiyuan, 2017-01-26 00:05:34]

xiyuan@chromium.org changed reviewers:
+ jdufault@chromium.org

[xiyuan, 2017-01-26 00:05:35]

jdufault@, could you help to review this? Thanks.

[xiyuan, 2017-01-26 00:05:41]

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

[commit-bot: I haz the power, 2017-01-26 00:06:31]

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

[jdufault, 2017-01-26 00:25:55]

lgtm, but should ProfileOAuth2TokenService be verifying that there's not already
an inflight network call instead of OAuth2LoginManager?

[commit-bot: I haz the power, 2017-01-26 00:44:12]

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

[commit-bot: I haz the power, 2017-01-26 00:44:12]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2017-01-26 00:44:48]

On 2017/01/26 00:25:55, jdufault wrote:
> lgtm, but should ProfileOAuth2TokenService be verifying that there's not
already
> an inflight network call instead of OAuth2LoginManager?

It verifies but in the form of DCHECK [1] and does not help because network
change only happens when running on a real device and use a release build.

[1]:
https://cs.chromium.org/chromium/src/chrome/browser/signin/mutable_profile_oa...

[xiyuan, 2017-01-26 00:45:32]

The CQ bit was checked by xiyuan@chromium.org

[commit-bot: I haz the power, 2017-01-26 00:46:38]

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, 2017-01-26 00:55:59]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485391532657370, "parent_rev":
"ab97148e15f1fecbdef8f50bed35987d8c6423ab", "commit_rev":
"29db500ff5473c212ebe3f1cebbc00ec87a023b8"}

[commit-bot: I haz the power, 2017-01-26 00:56:30]

Description was changed from

==========
cros: Fix multiple LoadCredentials on network change

TokenService::LoadCredentials does not handle multiple concurrent
load requests. However, OAuth2LoginManager::ContinueSessionRestore
calls it when there is a network change and could end up having
multiple load requests in fly. This would cause the token of the
primary account being wrongly revoked. Adding a bool flag in
OAuth2LoginManager to prevent overlapping LoadCredentials calls.

BUG=682695
==========

to

==========
cros: Fix multiple LoadCredentials on network change

TokenService::LoadCredentials does not handle multiple concurrent
load requests. However, OAuth2LoginManager::ContinueSessionRestore
calls it when there is a network change and could end up having
multiple load requests in fly. This would cause the token of the
primary account being wrongly revoked. Adding a bool flag in
OAuth2LoginManager to prevent overlapping LoadCredentials calls.

BUG=682695

Review-Url: https://codereview.chromium.org/2650653007
Cr-Commit-Position: refs/heads/master@{#446180}
Committed:
https://chromium.googlesource.com/chromium/src/+/29db500ff5473c212ebe3f1cebbc...
==========

[commit-bot: I haz the power, 2017-01-26 00:56:31]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/29db500ff5473c212ebe3f1cebbc...

[Daniel Erat, 2017-01-26 01:18:32]

derat@chromium.org changed reviewers:
+ derat@chromium.org

[Daniel Erat, 2017-01-26 01:18:33]

drive-by: is there any way to add a test for this to oauth_browsertest.cc? :-)

[xiyuan, 2017-01-26 16:26:40]

On 2017/01/26 01:18:33, Daniel Erat wrote:
> drive-by: is there any way to add a test for this to oauth_browsertest.cc? :-)

Let me see what I can do there.