[Desktop] Add diagnostics about loading the refresh tokens.

[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

TBR=pkasting@chromium.org

BUG=686485
Review-Url: https://codereview.chromium.org/2672833002
Cr-Commit-Position: refs/heads/master@{#449290}
Committed: https://chromium.googlesource.com/chromium/src/+/fd932ab936187fa497e6744b9a299755a7311af5

[msarda, 2017-02-02 17:09:46]

Description was changed from

==========
Add diagnostics about loading the refresh tokens

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

to

==========
Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

BUG=686485
==========

[msarda, 2017-02-02 17:10:53]

Description was changed from

==========
Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

BUG=686485
==========

to

==========
Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

BUG=686485
==========

[msarda, 2017-02-02 17:12:42]

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

[msarda, 2017-02-02 17:12:42]

Roger,

This CL is in an early stage as I have not adapted/changed unit tests (and I did
not yet changed the mobile part), however I'd like an early review to make sure
this is on the right track.

Please take a look.

[Roger Tawa OOO till Jul 10th, 2017-02-02 18:58:25]

Looks fine, one comment below.

https://codereview.chromium.org/2672833002/diff/40001/chrome/browser/signin/m...
File chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
(right):

https://codereview.chromium.org/2672833002/diff/40001/chrome/browser/signin/m...
chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc:320: //
as well?
I'm not sure why you wouldn't fire the event here too.  The error may be
unknown, but an error happened.  And the event is fired for errors.  Was this
causing specific problems elsewhere?

[msarda, 2017-02-03 09:07:09]

https://codereview.chromium.org/2672833002/diff/40001/chrome/browser/signin/m...
File chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
(right):

https://codereview.chromium.org/2672833002/diff/40001/chrome/browser/signin/m...
chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc:320: //
as well?
On 2017/02/02 18:58:25, Roger Tawa wrote:
> I'm not sure why you wouldn't fire the event here too.  The error may be
> unknown, but an error happened.  And the event is fired for errors.  Was this
> causing specific problems elsewhere?

It did not cause any specific errors. I did not fire the event here as that
would be a behaviour change for the token service and I did not want that to be
part of this CL.

But I'll probably do that change after your comment.

[msarda, 2017-02-03 15:42:47]

Description was changed from

==========
Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

BUG=686485
==========

to

==========
[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

BUG=686485
==========

[msarda, 2017-02-03 15:44:38]

Roger,

I have added a coupe of tests for the regular cases, but not for all of them as
the TokenWebData is not mockable.

Please take a look.

[msarda, 2017-02-07 13:22:09]

Roger: Please take a look.

[Roger Tawa OOO till Jul 10th, 2017-02-07 15:13:07]

lgtm

[msarda, 2017-02-08 08:59:43]

msarda@chromium.org changed reviewers:
+ pkasting@chromium.org

[msarda, 2017-02-08 08:59:43]

pkasting@chromium.org: Please do an owners review of
components/webdata/common/web_data_results.h

[msarda, 2017-02-08 13:21:11]

Description was changed from

==========
[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

BUG=686485
==========

to

==========
[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

TBR=pkasting@chromium.org

BUG=686485
==========

[msarda, 2017-02-08 13:21:55]

API change approved by OWNER. TBR-ing pkasting@chromium.org for change in the
file components/webdata/common/web_data_results.h

[msarda, 2017-02-08 13:22:11]

The CQ bit was checked by msarda@chromium.org

[commit-bot: I haz the power, 2017-02-08 13:22:28]

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-02-08 13:24:21]

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

[commit-bot: I haz the power, 2017-02-08 13:24:22]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[msarda, 2017-02-08 15:31:09]

The CQ bit was checked by msarda@chromium.org

[msarda, 2017-02-08 15:31:09]

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/2672833002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-08 15:32:27]

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-02-08 15:44:00]

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

[commit-bot: I haz the power, 2017-02-08 15:44:01]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msarda, 2017-02-09 09:03:22]

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

[commit-bot: I haz the power, 2017-02-09 09:03:42]

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, 2017-02-09 09:12:06]

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

[commit-bot: I haz the power, 2017-02-09 09:12:07]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[msarda, 2017-02-09 09:44:10]

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

[commit-bot: I haz the power, 2017-02-09 09:44:27]

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, 2017-02-09 09:54:21]

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

[commit-bot: I haz the power, 2017-02-09 09:54:22]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[msarda, 2017-02-09 10:26:56]

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

[msarda, 2017-02-09 10:27:01]

Patchset #6 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-02-09 10:27:04]

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

[msarda, 2017-02-09 10:27:42]

Patchset #6 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-02-09 10:47:49]

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

[commit-bot: I haz the power, 2017-02-09 10:47:50]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[msarda, 2017-02-09 13:42:25]

Patchset #6 (id:140001) has been deleted

[msarda, 2017-02-09 13:43:03]

The CQ bit was checked by msarda@chromium.org

[msarda, 2017-02-09 13:43:04]

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/2672833002/#ps160001
(title: "Fix compile")

[commit-bot: I haz the power, 2017-02-09 13:43:26]

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-02-09 14:57:19]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1486647783991490,
"parent_rev": "6a917fd8663e32b65066fee2afdb2d463ee357eb", "commit_rev":
"fd932ab936187fa497e6744b9a299755a7311af5"}

[commit-bot: I haz the power, 2017-02-09 14:57:51]

Description was changed from

==========
[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

TBR=pkasting@chromium.org

BUG=686485
==========

to

==========
[Desktop] Add diagnostics about loading the refresh tokens.

This CL adds a new line in the sign-in internals that logs the state
of the last load of credentials. This would enable debugging cases
when the profiles enters an invalid credentials auth error state.

Screenshot:
https://drive.google.com/open?id=0Bw1MJ8m7U5kbOV9Yb2hUTVpwMjA

TBR=pkasting@chromium.org

BUG=686485

Review-Url: https://codereview.chromium.org/2672833002
Cr-Commit-Position: refs/heads/master@{#449290}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd932ab936187fa497e6744b9a29...
==========

[commit-bot: I haz the power, 2017-02-09 14:57:52]

Committed patchset #6 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/fd932ab936187fa497e6744b9a29...

[Peter Kasting, 2017-02-10 01:34:07]

LGTM