policy: Remove SigninManager from UserCloudPolicyStore.

policy: Remove SigninManager from UserCloudPolicyStore.

Instead of accessing the SigninManager singleton from UserCloudPolicyStore, the username from signin is injected to UserCloudPolicyStore.

BUG=271392
(For change on Android:)

R=atwilson@chromium.org, dconnelly@chromium.org, joaodasilva@chromium.org
TBR=tedchoc@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233065

[pneubeck (no reviews), 2013-10-29 10:32:25]

@Joao, this is not finished, but please take a look whether it matches your
expectations / whether the change is reasonable.

Manually running Chrome and fetching policy worked fine. However the tests
CloudPolicyTest.FetchPolicy and CloudPolicyTest.InvalidatePolicy fail, but they
seem broken anyways (I see
UserPolicySigninServiceBase::ShutdownUserCloudPolicyManager being run).

[Joao da Silva, 2013-10-29 11:42:24]

Looks OK. Notice that the UserPolicySigninService[Base] can use the
SigninManager[Factory], so the username doesn't have to be passed from the
one_click_signin_sync_starter.

[pneubeck (no reviews), 2013-10-29 14:24:52]

On 2013/10/29 11:42:24, Joao da Silva wrote:
> Looks OK. Notice that the UserPolicySigninService[Base] can use the
> SigninManager[Factory], so the username doesn't have to be passed from the
> one_click_signin_sync_starter.

Not sure that UserPolicySigninService[Base] can always distinguish when to user
GetAuthenticatedUsername (used by UserPolicySigninService[Base]) or
GetUsernameForAuthInProgress() (used by OnceClickSigninSyncStarter).

[Joao da Silva, 2013-10-30 08:07:59]

On 2013/10/29 14:24:52, pneubeck wrote:
> On 2013/10/29 11:42:24, Joao da Silva wrote:
> > Looks OK. Notice that the UserPolicySigninService[Base] can use the
> > SigninManager[Factory], so the username doesn't have to be passed from the
> > one_click_signin_sync_starter.
> 
> Not sure that UserPolicySigninService[Base] can always distinguish when to
user
> GetAuthenticatedUsername (used by UserPolicySigninService[Base]) or
> GetUsernameForAuthInProgress() (used by OnceClickSigninSyncStarter).

Makes sense. Is this ready for reviewing and landing?

[pneubeck (no reviews), 2013-10-30 08:23:08]

On 2013/10/30 08:07:59, Joao da Silva wrote:
> On 2013/10/29 14:24:52, pneubeck wrote:
> > On 2013/10/29 11:42:24, Joao da Silva wrote:
> > > Looks OK. Notice that the UserPolicySigninService[Base] can use the
> > > SigninManager[Factory], so the username doesn't have to be passed from the
> > > one_click_signin_sync_starter.
> > 
> > Not sure that UserPolicySigninService[Base] can always distinguish when to
> user
> > GetAuthenticatedUsername (used by UserPolicySigninService[Base]) or
> > GetUsernameForAuthInProgress() (used by OnceClickSigninSyncStarter).
> 
> Makes sense. Is this ready for reviewing and landing?

No. I'll have to complete the other platforms (only ChromeOS I think) at first.

[dconnelly, 2013-10-31 09:04:11]

pretty straightforward. looks good to me so far

[pneubeck (no reviews), 2013-11-04 08:36:03]

ok. this should be good now. PTAL.

[Joao da Silva, 2013-11-04 09:39:52]

lgtm

[Joao da Silva, 2013-11-04 09:40:26]

+Drew to have a quick glance at this

[Joao da Silva, 2013-11-04 09:58:32]

Just noticed that the store doesn't need the Profile anymore.

https://codereview.chromium.org/49783006/diff/230001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_store.cc (right):

https://codereview.chromium.org/49783006/diff/230001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_store.cc:102: profile_(profile),
This can now be removed, please do it.

[pneubeck (no reviews), 2013-11-04 10:59:46]

https://codereview.chromium.org/49783006/diff/230001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_store.cc (right):

https://codereview.chromium.org/49783006/diff/230001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_store.cc:102: profile_(profile),
On 2013/11/04 09:58:32, Joao da Silva wrote:
> This can now be removed, please do it.

Done.

[Joao da Silva, 2013-11-04 11:02:19]

lgtm

[dconnelly, 2013-11-04 15:24:00]

lgtm

[Andrew T Wilson (Slow), 2013-11-04 17:01:25]

lgtm

[commit-bot: I haz the power, 2013-11-04 18:52:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/49783006/380001

[commit-bot: I haz the power, 2013-11-04 19:55:42]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-04 20:54:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/49783006/470002

[commit-bot: I haz the power, 2013-11-04 21:50:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-05 12:42:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/49783006/1000001

[commit-bot: I haz the power, 2013-11-05 15:19:44]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-05 16:23:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@chromium.org/49783006/1000001

[commit-bot: I haz the power, 2013-11-05 20:02:46]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[pneubeck (no reviews), 2013-11-05 20:08:14]

Committed patchset #7 manually as r233065 (presubmit successful).