policy: Register OffTheRecordProfile at UserCloudPolicyManagerFactory.

policy: Register OffTheRecordProfile at UserCloudPolicyManagerFactory.

This allows to remove the dependency of UserCloudPolicyManagerFactory on Profile in the next step.

BUG=271392
R=erg@chromium.org, joaodasilva@chromium.org

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

[pneubeck (no reviews), 2013-10-30 12:09:12]

next attempt. PTAL

[Joao da Silva, 2013-11-04 09:53:37]

Looks good, but seems like parts of https://codereview.chromium.org/44083005 got
in here?

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager.h (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager.h:44:
content::BrowserContext* context,
Shouldn't this come in https://codereview.chromium.org/44083005?

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc:60: // also
shared between the incognito Profile and the original Profile.
Update this comment.

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/profiles/...
File chrome/browser/profiles/off_the_record_profile_impl.cc (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/profiles/...
chrome/browser/profiles/off_the_record_profile_impl.cc:106: #endif
Document why this is needed.

[pneubeck (no reviews), 2013-11-04 12:38:50]

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager.h (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager.h:44:
content::BrowserContext* context,
On 2013/11/04 09:53:38, Joao da Silva wrote:
> Shouldn't this come in https://codereview.chromium.org/44083005?

Done.

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/user_cloud_policy_manager_factory.cc:60: // also
shared between the incognito Profile and the original Profile.
On 2013/11/04 09:53:38, Joao da Silva wrote:
> Update this comment.

Done.

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/profiles/...
File chrome/browser/profiles/off_the_record_profile_impl.cc (right):

https://codereview.chromium.org/52343002/diff/170001/chrome/browser/profiles/...
chrome/browser/profiles/off_the_record_profile_impl.cc:106: #endif
On 2013/11/04 09:53:38, Joao da Silva wrote:
> Document why this is needed.

Done.

[pneubeck (no reviews), 2013-11-04 12:40:42]

@Elliot, please take a look at the profile changes.

[Joao da Silva, 2013-11-04 12:53:52]

lgtm

[Elliot Glaysher, 2013-11-04 20:12:36]

https://codereview.chromium.org/52343002/diff/300006/chrome/browser/profiles/...
File chrome/browser/profiles/off_the_record_profile_impl.cc (right):

https://codereview.chromium.org/52343002/diff/300006/chrome/browser/profiles/...
chrome/browser/profiles/off_the_record_profile_impl.cc:107:
this->GetOriginalProfile(), this);
I assume there's going to be more services like this, which live will live in
components/, but require special incognito handling. Do you have any ideas about
how to make something like this generalized? I did a lot of work to pair down
ProfileImpl and would be sad if an equivalent number of
RegisterForOffTheRecordProfile() things started showing up in
OffTheRecordProfileImpl...

[pneubeck (no reviews), 2013-11-04 20:27:56]

On 2013/11/04 20:12:36, Elliot Glaysher wrote:
>
https://codereview.chromium.org/52343002/diff/300006/chrome/browser/profiles/...
> File chrome/browser/profiles/off_the_record_profile_impl.cc (right):
> 
>
https://codereview.chromium.org/52343002/diff/300006/chrome/browser/profiles/...
> chrome/browser/profiles/off_the_record_profile_impl.cc:107:
> this->GetOriginalProfile(), this);
> I assume there's going to be more services like this, which live will live in
> components/, but require special incognito handling. Do you have any ideas
about
> how to make something like this generalized? I did a lot of work to pair down
> ProfileImpl and would be sad if an equivalent number of
> RegisterForOffTheRecordProfile() things started showing up in
> OffTheRecordProfileImpl...

Yes, I see the problem. That's why I originally created that mail thread which
addressed you, the other OWNERS of BCKS, Joi and John about this issue.
I suggest(ed) that we move this registering to the
BrowserContextDependencyManager.
I can setup a CL for this. It would also get rid of that IMO very unhealthy
static_cast<Profile*> in the incognito_helper.

However, I fear that such a more general solution will need a lot more time for
discussion/review and might even be rejected similar to my previous change to
BrowserContext.
On the other hand, the policy stuff is far from being removed from ProfileImpl
anyways. Thus I don't think that this particular CL is really a step backwards.
More the opposite: it actually is a prerequisite for my other CLs which reduce
the dependency on Profile.

[Elliot Glaysher, 2013-11-05 01:27:41]

Taking these out of order...

> However, I fear that such a more general solution will need a lot more time
for
> discussion/review and might even be rejected similar to my previous change to
> BrowserContext.
> On the other hand, the policy stuff is far from being removed from ProfileImpl
> anyways. Thus I don't think that this particular CL is really a step
backwards.
> More the opposite: it actually is a prerequisite for my other CLs which reduce
> the dependency on Profile.

Given the later, sure. lgtm.

> Yes, I see the problem. That's why I originally created that mail thread which
> addressed you, the other OWNERS of BCKS, Joi and John about this issue.
> I suggest(ed) that we move this registering to the
> BrowserContextDependencyManager.

IIRC, the problem was that it leaked the concept of an original profile below
the chrome/ layer. I'd be fine with hooking GetBrowserContextToUse in a way that
allowed users of components to inject the redirection behaviour without leaking
the actual idea of an original profile down to that level.

[commit-bot: I haz the power, 2013-11-05 08:49:22]

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

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

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

[commit-bot: I haz the power, 2013-11-05 11:09:34]

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

[commit-bot: I haz the power, 2013-11-05 14:50:21]

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:03:04]

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

[pneubeck (no reviews), 2013-11-05 19:54:56]

Committed patchset #4 manually as r233062 (presubmit successful).