Test BlockingLoginTestInstance with enabled webview

Test BlockingLoginTestInstance with enabled webview

BUG=475567
TEST=browser_tests

[Dmitry Polukhin, 2015-04-27 09:30:55]

dpolukhin@chromium.org changed reviewers:
+ nkostylev@chromium.org

[Dmitry Polukhin, 2015-04-27 09:30:56]

PTAL

[Nikita (slow), 2015-04-27 13:05:49]

https://codereview.chromium.org/1107873002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1107873002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:961: //
login_performer_ might be null if error happened during OAuth2 token
nit: update comment
login_performer_ most likely will be null (created only in CompleteLogin)

[Nikita (slow), 2015-04-27 14:40:19]

lgtm

verified that these tests are fixed as well with this CL:
https://codereview.chromium.org/1052683006/

https://codereview.chromium.org/1107873002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1107873002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:234: const
UserContext& get_user_context() const { return user_context_; }
nit: user_context()

https://codereview.chromium.org/1107873002/diff/20001/chromeos/login/auth/aut...
File chromeos/login/auth/auth_status_consumer.h (right):

https://codereview.chromium.org/1107873002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/auth_status_consumer.h:39: FAILED_TO_INITIALIZE_TOKEN,  //
Could not get OAuth2 token.
nit:  FAILED_TO_INITIALIZE_OAUTH2_TOKEN to be more specific

[Dmitry Polukhin, 2015-04-27 14:51:58]

https://codereview.chromium.org/1107873002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/1107873002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/existing_user_controller.cc:961: //
login_performer_ might be null if error happened during OAuth2 token
On 2015/04/27 13:05:49, Nikita Kostylev wrote:
> nit: update comment
> login_performer_ most likely will be null (created only in CompleteLogin)

Done.

https://codereview.chromium.org/1107873002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/session/user_session_manager.h (right):

https://codereview.chromium.org/1107873002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/session/user_session_manager.h:234: const
UserContext& get_user_context() const { return user_context_; }
On 2015/04/27 14:40:19, Nikita Kostylev wrote:
> nit: user_context()

Done.

https://codereview.chromium.org/1107873002/diff/20001/chromeos/login/auth/aut...
File chromeos/login/auth/auth_status_consumer.h (right):

https://codereview.chromium.org/1107873002/diff/20001/chromeos/login/auth/aut...
chromeos/login/auth/auth_status_consumer.h:39: FAILED_TO_INITIALIZE_TOKEN,  //
Could not get OAuth2 token.
On 2015/04/27 14:40:19, Nikita Kostylev wrote:
> nit:  FAILED_TO_INITIALIZE_OAUTH2_TOKEN to be more specific

Done.

[Nikita (slow), 2015-04-27 15:07:21]

FYI, just passing that access token to WhitelistLoginChecker is not enough
https://codereview.chromium.org/1052683006/
see latest patch which includes this CL as well.

[xiyuan, 2015-04-27 16:33:27]

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

[xiyuan, 2015-04-27 16:33:27]

Thank you for looking at this. Let me know what you think. I got the suggestions
from Mattias/Andrew when hacking the bootstrap (EAFE) flow. It would be great
that we could all use RT and merge the post-login code path.

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:220:
.GetAccessToken());
The AT in user context just have oauth login scrope and might not be enough to
use with policy code. 

PolicyOAuth2TokenFetcher wraps the real fetching work and we can update it to
start with a RT. We should probably add a parallel method to
FetchPolicyOAuthTokenUsingSigninContext to start PolicyOAuth2TokenFetcher with
the refresh token.

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
File
chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc
(right):

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc:157:
chromeos::StartupUtils::IsWebviewSigninEnabled()) &&
When UserCloudPolicyManagerChromeOS is smart to start policy fetching using RT,
this is not necessary and the line 156 can be removed too.

[achuithb, 2015-04-27 17:54:36]

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

[achuithb, 2015-04-27 17:54:37]

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:220:
.GetAccessToken());
On 2015/04/27 16:33:27, xiyuan wrote:
> The AT in user context just have oauth login scrope and might not be enough to
> use with policy code. 
> 
> PolicyOAuth2TokenFetcher wraps the real fetching work and we can update it to
> start with a RT. We should probably add a parallel method to
> FetchPolicyOAuthTokenUsingSigninContext to start PolicyOAuth2TokenFetcher with
> the refresh token.
>  

Yup, we need this for the wildcard login check. I was going to tackle this
today.

[achuithb, 2015-04-28 06:47:02]

On 2015/04/27 17:54:37, achuithb wrote:
>
https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
> File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
> (right):
> 
>
https://codereview.chromium.org/1107873002/diff/40001/chrome/browser/chromeos...
> chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:220:
> .GetAccessToken());
> On 2015/04/27 16:33:27, xiyuan wrote:
> > The AT in user context just have oauth login scrope and might not be enough
to
> > use with policy code. 
> > 
> > PolicyOAuth2TokenFetcher wraps the real fetching work and we can update it
to
> > start with a RT. We should probably add a parallel method to
> > FetchPolicyOAuthTokenUsingSigninContext to start PolicyOAuth2TokenFetcher
with
> > the refresh token.
> >  
> 
> Yup, we need this for the wildcard login check. I was going to tackle this
> today.

Dmitry, I think some of your concerns here should be addressed by:
https://codereview.chromium.org/1108983002/