Replace chromeos::UserManager::Get() with chromeos::GetUserManager().

Replace chromeos::UserManager::Get() with chromeos::GetUserManager().

This makes it possible to move UserManager to user_manager component but keep getter method in chrome/browser.

New clients that we'll be using user_manager/UserManager instance will migrate to dependecy injection where possible.

BUG=387614
TBR=mek,nyquist,sergeyu,rtenneti,mattm,stevenjb,yoz,bartfab,noms,blundell,bauerb,atwilson,oshima,msw

[Nikita (slow), 2014-07-09 16:59:32]

Denis, please take a look.

[Nikita (slow), 2014-07-09 17:25:21]

TBR'ing OWNERS

mek@chromium.org:
apps/*
chrome/browser/extensions/*

nyquist@chromium.org:
chrome/browser/invalidation/*

sergeyu@chromium.org
chrome/browser/media/*

rtenneti@chromium.org
chrome/browser/metrics/*

mattm@chromium.org
chrome/browser/net/*

stevenjb@chromium.org
chrome/browser/notifications/*

yoz@chromium.org
chrome/browser/performance_monitor/*

bartfab@chromium.org
chrome/browser/policy/*

noms@chromium.org
chrome/browser/profiles/*

blundell@chromium.org
chrome/browser/signin/*

bauerb@chromium.org
chrome/browser/supervised_user/*

atwilson@chromium.org
chrome/browser/sync/*

oshima@chromium.org
chrome/browser/ui/ash/*

msw@chromium.org
chrome/browser/ui/startup/*

[oshima, 2014-07-09 18:27:58]

just curious. Why do we want to keep the getter method in chrome/browser ?

[Denis Kuznetsov (DE-MUC), 2014-07-09 18:33:11]

lgtm with nits

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
File
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc
(right):

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc:33:
using chromeos::UserManager;
Not needed anymore.

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc:330:
GetUserManager()->UserLoggedIn(kTestUserName, kTestUserName, true);
need extra namespace qualification.

[Nikita (slow), 2014-07-09 19:27:40]

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
File
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc
(right):

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc:33:
using chromeos::UserManager;
On 2014/07/09 18:33:10, Denis Kuznetsov wrote:
> Not needed anymore.

Done.

https://chromiumcodereview.appspot.com/375413002/diff/20001/chrome/browser/ex...
chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc:330:
GetUserManager()->UserLoggedIn(kTestUserName, kTestUserName, true);
On 2014/07/09 18:33:10, Denis Kuznetsov wrote:
> need extra namespace qualification.

Done.

[Nikita (slow), 2014-07-09 19:30:36]

The CQ bit was checked by nkostylev@chromium.org

[commit-bot: I haz the power, 2014-07-09 19:32:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nkostylev@chromium.org/375413002/40001

[msw, 2014-07-09 19:40:14]

Please consider Oshima's comment.
chrome/browser/ui/startup/* lgtm.
Odd that you need so many owners...

[Nikita (slow), 2014-07-09 20:11:32]

The CQ bit was unchecked by nkostylev@chromium.org

[Nikita (slow), 2014-07-09 20:13:43]

On 2014/07/09 18:27:58, oshima wrote:
> just curious. Why do we want to keep the getter method in chrome/browser ?

As discussed with blundell@ once UserManager is moved to components
we'll try to move its clients into a proper dependency injection.

So for instance in athena UserManager instance might be owned by some ash::Shell
analogue.

[Nikita (slow), 2014-07-09 20:14:21]

On 2014/07/09 19:40:14, msw wrote:
> Please consider Oshima's comment.
> chrome/browser/ui/startup/* lgtm.
> Odd that you need so many owners...

I could go with sky@ though, just balancing load.

[mattm, 2014-07-09 20:25:41]

chrome/browser/net/nss_context_chromeos_browsertest.cc lgtm

[nyquist, 2014-07-09 22:05:30]

chrome/browser/invalidation lgtm

[oshima, 2014-07-10 07:24:02]

On 2014/07/09 20:13:43, Nikita Kostylev wrote:
> On 2014/07/09 18:27:58, oshima wrote:
> > just curious. Why do we want to keep the getter method in chrome/browser ?
> 
> As discussed with blundell@ once UserManager is moved to components
> we'll try to move its clients into a proper dependency injection.
> 
> So for instance in athena UserManager instance might be owned by some
ash::Shell
> analogue.

There is no equivalent thing in athena and that's deliberate decision. We
believe it wasn't good idea.

* It just adds extra dependency without much benefit (you need to access both
Shell and UserManager)
* Such object tends to become a place that people put random thing.

g_browser_process is another good example.

Another advantage of UserManager::Get is that it provides a uniform way to
access UserManager.
Imagine we have another component that users UserManager and want to share it on
both ash and athena.

[Andrew T Wilson (Slow), 2014-07-10 07:39:52]

sync lgtm

[blundell, 2014-07-10 07:49:19]

//chrome/browser/signin LGTM

You should resolve oshima@'s concern before committing though. Note that it
would be OK to move *both* the declaration and definition of UserManager::Get()
to the component, i.e., have it be a singleton at the component level. What it
comes down to is whether that's the right layering, i.e., whether there are some
embedders that would want to use the user_manager component but would want to
have a different ownership/lifetime model for UserManager.

[Nikita (slow), 2014-07-10 08:20:42]

On 2014/07/10 07:49:19, blundell wrote:
> //chrome/browser/signin LGTM
> 
> You should resolve oshima@'s concern before committing though. Note that it
> would be OK to move *both* the declaration and definition of
UserManager::Get()
> to the component, i.e., have it be a singleton at the component level. What it
> comes down to is whether that's the right layering, i.e., whether there are
some
> embedders that would want to use the user_manager component but would want to
> have a different ownership/lifetime model for UserManager.

Ok, thanks for raised concerns and your comments.
I think at this point I'll close this CL and focus on getting rest of
UserManager* code ready for moving out of src/chrome.

At this point it's not quite clear which other components might need to use
UserManager in both ash/athena as oshima@ pointed out so I may as well stick
with UserManager::Get() in user_manager component at the end.

[bartfab (slow), 2014-07-10 09:46:20]

chrome/browser/policy lgtm