[signin] Support for CrOS and OAuth2AccessTokenConsumer services.

Support for CrOS and OAuth2AccessTokenConsumer services for the about:signin-internals diagnostic page. 

This CL contains the following changes ->
1. Added support for ChromeOS based signin. AboutSigninInternals
   registers itself as an Observer with OnlineAttempt (in ChromeOS
   builds only), and receives signin information from it.

2. Shows if login used 2-factor authentication.

3. Token information for OAuth2AccessTokenConsumers has been added.
   Note that these are not needed once these services are migrated to
   use OAuth2TokenService instead. 

4. Information from sync_global_error updates chromiumsync service 
   status.

BUG=114613

[vishwath, 2012-12-19 23:10:37]

This CL contains mostly small changes spread out over multiple files, in
relation to crbug.com/114613. 

All changes to OAuth2AccessTokenConsumers are temporary measures to display
signin information in about:signin-internals until the services are migrated to
use OAuth2TokenService instead.

I've highlighted the relevant portions on a per reviewer, per directory basis.
PTAL.

davemoore:
(chrome/browser/chromeos/login/)
OnlineAttempt sends observer notifications to all registered
SigninDiagnosticsObserver objects (practically, just to AboutSigninInternals)

(chrome/browser/profiles/)
Directly sends notifications to AboutSigninInternals on successful and failed
token fetches. Temporary.

miket: (chrome/browser/extensions/)
kochi: (chrome/browser/google_apis/)
bartfab: (chrome/browser/policy/)
The files in all these directories do the same thing - directly access
AboutSigninInternals(through its factory), and send it notifications about
successful (or failed) token fetches. Temporary.

tim: 
(chrome/browser/signin)
Added 2-factor authentication information to about:signin-internals.

(chrome/browser/sync)
Information from SyncGlobalError is also relayed to about:signin-internals.

[miket_OOO, 2012-12-19 23:25:16]

Who should look at what? http://en.wikipedia.org/wiki/Bystander_effect

[vishwath, 2012-12-19 23:27:14]

Sorry, I should have been clearer. Next to each of your names is a
directory within brackets. You only need to review those files. Thanks for
the heads up miket!


On Wed, Dec 19, 2012 at 3:25 PM, <miket@chromium.org> wrote:

> Who should look at what?
http://en.wikipedia.org/wiki/**Bystander_effect<http://en.wikipedia.org/wiki/...
>
>
https://codereview.chromium.**org/11618024/<https://codereview.chromium.org/1...
>

[vishwath, 2012-12-19 23:31:56]

+mnissler: Could you please review the files from chrome/browser/policy/ in lieu
of bartfab (whom I originally added as a reviewer but is out for the holidays)?
Thanks!

[miket_OOO, 2012-12-19 23:39:25]

> Sorry, I should have been clearer. Next to each of your names is a
> directory within brackets. You only need to review those files. Thanks for
> the heads up miket!

I see it now. This has bitten me before, too -- reviewers tend to look only at
the CL description, and not to the content in the initial review request (your
comment #1). You've probably put it in the right place (since you don't need the
review stuff to go into the permanent commit message), so I don't know how to
avoid the problem in the future.

[miket_OOO, 2012-12-19 23:42:54]

chrome/browser/extensions LGTM

[vishwath, 2012-12-19 23:43:35]

Ah, that makes sense. Maybe it would help to publish with an empty message
first (so reviewers can see just the CL description), and quickly follow up
with a second mail explaining who needs to look at what portions.


On Wed, Dec 19, 2012 at 3:39 PM, <miket@chromium.org> wrote:

> Sorry, I should have been clearer. Next to each of your names is a
>> directory within brackets. You only need to review those files. Thanks for
>> the heads up miket!
>>
>
> I see it now. This has bitten me before, too -- reviewers tend to look
> only at
> the CL description, and not to the content in the initial review request
> (your
> comment #1). You've probably put it in the right place (since you don't
> need the
> review stuff to go into the permanent commit message), so I don't know how
> to
> avoid the problem in the future.
>
>
https://codereview.chromium.**org/11618024/<https://codereview.chromium.org/1...
>

[vishwath, 2012-12-20 01:21:11]

mnissler:
oauth_access_token_ was incorrectly not being set in OnGetTokenSuccess(),
I've uploaded a patch that fixes it. PTAL at that patch instead. Thanks!


On Wed, Dec 19, 2012 at 3:43 PM, Vishwath Mohan <vishwath@google.com> wrote:

> Ah, that makes sense. Maybe it would help to publish with an empty message
> first (so reviewers can see just the CL description), and quickly follow up
> with a second mail explaining who needs to look at what portions.
>
>
> On Wed, Dec 19, 2012 at 3:39 PM, <miket@chromium.org> wrote:
>
>>  Sorry, I should have been clearer. Next to each of your names is a
>>> directory within brackets. You only need to review those files. Thanks
>>> for
>>> the heads up miket!
>>>
>>
>> I see it now. This has bitten me before, too -- reviewers tend to look
>> only at
>> the CL description, and not to the content in the initial review request
>> (your
>> comment #1). You've probably put it in the right place (since you don't
>> need the
>> review stuff to go into the permanent commit message), so I don't know
>> how to
>> avoid the problem in the future.
>>
>>
https://codereview.chromium.**org/11618024/<https://codereview.chromium.org/1...
>>
>
>

[kochi, 2012-12-20 06:37:24]

chrome/browser/google_apis LGTM with a nit.

https://chromiumcodereview.appspot.com/11618024/diff/6007/chrome/browser/goog...
File chrome/browser/google_apis/DEPS (right):

https://chromiumcodereview.appspot.com/11618024/diff/6007/chrome/browser/goog...
chrome/browser/google_apis/DEPS:6: "+chrome/browser/profiles",
Please order alphabetically.

[Mattias Nissler (ping if slow), 2012-12-20 09:16:10]

LGTM with nits for chrome/browser/policy, adding Drew FYI who wrote the code.

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
File chrome/browser/policy/user_policy_signin_service.cc (right):

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
chrome/browser/policy/user_policy_signin_service.cc:297: DCHECK(profile_);
Any good reason for this DCHECK? If |profile_| is NULL, you'll get a crash
anyways.

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
chrome/browser/policy/user_policy_signin_service.cc:311: // Pass along the new
access token to the CloudPolicyClient.
nit: This comment is stale and probably a merge artifact. Please remove.

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
chrome/browser/policy/user_policy_signin_service.cc:317: DCHECK(profile_);
same here.

[Andrew T Wilson (Slow), 2012-12-21 11:42:28]

LGTM with one nit.

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
File chrome/browser/policy/user_policy_signin_service.cc (right):

https://codereview.chromium.org/11618024/diff/6007/chrome/browser/policy/user...
chrome/browser/policy/user_policy_signin_service.cc:316: // OAuth2TokenService
instead.
Remove this comment, as we are not going to refactor this class to use
OAuth2TokenService (the reason is that this code is executed as part of profile
startup, so it needs to run before either TokenService or OAuth2TokenService is
initialized).

[commit-bot: I haz the power, 2012-12-21 20:56:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vishwath@google.com/11618024/14001

[commit-bot: I haz the power, 2012-12-21 20:56:16]

Presubmit check for 11618024-14001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome
    chrome/browser/chromeos/login
    chrome/browser/profiles

Presubmit checks took 1.7s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[DaveMoore, 2012-12-21 21:26:24]

login / profile* lgtm

[commit-bot: I haz the power, 2012-12-21 21:28:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vishwath@google.com/11618024/14001

[commit-bot: I haz the power, 2012-12-21 21:28:19]

Presubmit check for 11618024-14001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 2.8s to calculate.

[tim (not reviewing), 2012-12-21 21:32:35]

On 2012/12/21 21:28:19, I haz the power (commit-bot) wrote:
> Presubmit check for 11618024-14001 failed and returned exit status 1.
> 
> 
> Running presubmit commit checks ...
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for files in these directories:
>     chrome
> 
> Presubmit checks took 2.8s to calculate.

sync/ LGTM

[vishwath, 2012-12-21 21:38:53]

+jhawkins

James:
My commit fails saying there's no OWNER for chrome/. But my CL doesn't actually
have any files in that dir. So i'm unsure what you need to LGTM. Far be it from
me to disagree with Rietveld though!

[James Hawkins, 2012-12-21 23:31:04]

LGTM with nits.

https://codereview.chromium.org/11618024/diff/14001/chrome/browser/google_api...
File chrome/browser/google_apis/auth_service.cc (right):

https://codereview.chromium.org/11618024/diff/14001/chrome/browser/google_api...
chrome/browser/google_apis/auth_service.cc:78: // needs a profile var. Remove
and refactor ctors()/call-sites accordingly.
nit: Document the member variable (also, state 'weak pointer' I imagine) in
addition to the TODO.

https://codereview.chromium.org/11618024/diff/14001/chrome/browser/signin/sig...
File chrome/browser/signin/signin_internals_util.h (right):

https://codereview.chromium.org/11618024/diff/14001/chrome/browser/signin/sig...
chrome/browser/signin/signin_internals_util.h:25: extern const char
kOAuth2MintTokenFlowToken[];
nit: Are these supposed to be sorted now?  kUser comes after kProfile if so.

[commit-bot: I haz the power, 2012-12-21 23:54:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vishwath@google.com/11618024/20002

[commit-bot: I haz the power, 2012-12-22 03:27:43]

Retried try job too often on win_rel for step(s) browser_tests

[Andrew T Wilson (Slow), 2013-08-21 10:11:20]

closing this issue since it's undoubtedly bitrotted over the last 8 months.