user_manager: Remove deps on session_manager

user_manager: Remove deps on session_manager

Move session started flag and code from user_manager to
session manager and remove the deps on session_manager.

BUG=657149
Committed: https://crrev.com/834f3bc588531d9cea7ee62ffe034ff47c212b05
Cr-Commit-Position: refs/heads/master@{#427773}

[xiyuan, 2016-10-20 17:00:21]

The CQ bit was checked by xiyuan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 17:00:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiyuan, 2016-10-20 17:11:57]

xiyuan@chromium.org changed reviewers:
+ achuith@chromium.org, stevenjb@chromium.org

[xiyuan, 2016-10-20 17:11:57]

No functionality change. Move session started flag from user_manager to
session_manager and get rid of the user_manager's deps on session_manager. Think
session_manager should depend on user_manager instead, e.g. to get user info
such as user type and account id etc.

Next CL would make session_manager keep track of sessions (aka logged in users).
And we could replace the strange call pattern:
  UserManager::UserLoggedIn
  SessionManager::SessionStarted
with something like:
  SessionManager::CreateSession
  SessionManager::SessionStarted

[achuithb, 2016-10-20 17:22:32]

lgtm

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/chrome_session_manager.h (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/chrome_session_manager.h:31: //
session_manager::SessionManager
does this need a period or a colon?

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:1748:
g_browser_process->platform_part()->SessionManager()->SessionStarted();
What's the difference between this and
session_manager::SessionManager::Get()->SessionStarted();

[xiyuan, 2016-10-20 17:26:50]

Thanks for the quick review.

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/chrome_session_manager.h (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/chrome_session_manager.h:31: //
session_manager::SessionManager
On 2016/10/20 17:22:31, achuithb wrote:
> does this need a period or a colon?

Added a colon.

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:1748:
g_browser_process->platform_part()->SessionManager()->SessionStarted();
On 2016/10/20 17:22:31, achuithb wrote:
> What's the difference between this and
> session_manager::SessionManager::Get()->SessionStarted();

They are the same. Line 1138 uses platform_part()->SessionManager() way so I was
trying to be consistent here.

I could change Line 1138 to use session_manager::SessionManager::Get() too.

Let me know which way you prefer.

[achuithb, 2016-10-20 17:39:45]

lgtm

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/session/user_session_manager.cc (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/session/user_session_manager.cc:1748:
g_browser_process->platform_part()->SessionManager()->SessionStarted();
On 2016/10/20 17:26:50, xiyuan wrote:
> On 2016/10/20 17:22:31, achuithb wrote:
> > What's the difference between this and
> > session_manager::SessionManager::Get()->SessionStarted();
> 
> They are the same. Line 1138 uses platform_part()->SessionManager() way so I
was
> trying to be consistent here.
> 
> I could change Line 1138 to use session_manager::SessionManager::Get() too.
> 
> Let me know which way you prefer.

Sure, if the current way is more consistent with the usage in the file, let's
leave it that way.

[commit-bot: I haz the power, 2016-10-20 18:33:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 18:33:43]

Dry run: This issue passed the CQ dry run.

[kuzminruslan, 2016-10-20 18:40:14]

kuzminruslan@icloud.com changed reviewers:
+ kuzminruslan@icloud.com

[kuzminruslan, 2016-10-20 18:40:15]

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/pol...
File chrome/browser/chromeos/policy/device_local_account_browsertest.cc (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/pol...
chrome/browser/chromeos/policy/device_local_account_browsertest.cc:114: #include
"components/user_manager/user_manager.h"
excess include

[xiyuan, 2016-10-20 19:28:12]

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/pol...
File chrome/browser/chromeos/policy/device_local_account_browsertest.cc (right):

https://codereview.chromium.org/2438063002/diff/1/chrome/browser/chromeos/pol...
chrome/browser/chromeos/policy/device_local_account_browsertest.cc:114: #include
"components/user_manager/user_manager.h"
On 2016/10/20 18:40:15, kuzminruslan wrote:
> excess include

This is still used, line 632 in CheckPublicSessionPresent.

[xiyuan, 2016-10-21 17:18:44]

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

[xiyuan, 2016-10-21 17:18:45]

stevenjb@, still need your review or at least owner approval for c/b/ui/ash/*.

dmazzoni@, please approve changes in:
c/b/extensions/api/braille_display_private/braille_display_private_apitest.cc

Thanks.

[stevenjb, 2016-10-21 19:09:36]

LGTM
Nice cleanup, thanks! I was just looking into how to avoid a dependency on
user_manager since it depended on session_manager which has a lot of
dependencies I don't want in src/chromeos, but now the deps list is entirely
reasonable!

[dmazzoni, 2016-10-26 17:47:12]

lgtm

[xiyuan, 2016-10-26 17:51:57]

The CQ bit was checked by xiyuan@chromium.org

[xiyuan, 2016-10-26 17:51:57]

The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/2438063002/#ps20001
(title: "fix nits in PS1")

[commit-bot: I haz the power, 2016-10-26 17:52:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 19:41:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-26 20:00:07]

Description was changed from

==========
user_manager: Remove deps on session_manager

Move session started flag and code from user_manager to
session manager and remove the deps on session_manager.

BUG=657149
==========

to

==========
user_manager: Remove deps on session_manager

Move session started flag and code from user_manager to
session manager and remove the deps on session_manager.

BUG=657149

Committed: https://crrev.com/834f3bc588531d9cea7ee62ffe034ff47c212b05
Cr-Commit-Position: refs/heads/master@{#427773}
==========

[commit-bot: I haz the power, 2016-10-26 20:00:09]

Patchset 2 (id:??) landed as
https://crrev.com/834f3bc588531d9cea7ee62ffe034ff47c212b05
Cr-Commit-Position: refs/heads/master@{#427773}