Add public accounts to UserManager

Add public accounts to UserManager

This is a re-land of CL 11419184:

This CL extends the UserManager to handle public accounts defined through
policy. User pods are dynamically added and removed when the list of
public accounts in policy changes. Any data belonging to obsolete accounts
is also removed, taking care not to remove it prematurely if a user is
currently logged into the account.

The CL also makes the user list handling more robust by checking for
duplicate entries in the user list prefs and logging these as errors.

The pods added for public accounts are not functional yet. The login flow
for public accounts will be the topic of another CL.

BUG=158509, 163690
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170943

[bartfab (slow), 2012-12-03 10:39:21]

CL 11419184 broke the tree and was reverted. This is a fixed-up CL that
addresses the issues encountered. Could you please re-review? I uploaded CL
11419184 as patchset 1 so that you can see the changes are minimal.

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager.h (right):

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager.h:165: virtual bool
IsLoggedInAsRegularUser() const = 0;
This line was missing, causing a compile failure on clang platforms.

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:187: logged_in_user_ = NULL;
This makes things more robust: The |logged_in_user_| should be deleted iff that
user is not deleted as part of the |users_| clean-up. Rather than relying on a
somewhat unrelated flag that happens to be |true| if the user is not on the
list, explicitly compare the |users_| entries to |logged_in_user_|.

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:231: logged_in_user_ =
RemoveRegularUserFromList(email);
This caused a memory leak because my reading of the original code was mistaken:
The logged-in user should be added to both the persistent list (in prefs) and
the |users_| list (in memory). Lines 246 and 247 take care of this now.

[bartfab (slow), 2012-12-03 10:40:12]

Hi Julian,

I would need a policy rubber-stamp for this. Nothing changed in the policy part
of this CL but the CQ is unforgiving.

[Ivan Korotkov, 2012-12-03 11:04:44]

lgtm

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:187: logged_in_user_ = NULL;
Hmm, so it looks like now |logged_in_user| can point to somewhere else outside
of |users_| or a static stub user? In that case, please update it's comment in
header file.

[bartfab (slow), 2012-12-03 11:34:12]

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/11411314/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/user_manager_impl.cc:187: logged_in_user_ = NULL;
On 2012/12/03 11:04:45, Ivan Korotkov wrote:
> Hmm, so it looks like now |logged_in_user| can point to somewhere else outside
> of |users_| or a static stub user? In that case, please update it's comment in
> header file.

The description in user_manager_impl.h is still accurate. The |logged_in_user_|
can be one of the |users_| instances or an ephemeral user. There were three
types of ephemeral users before this CL: stub, guest, users made ephemeral by
policy. This CL adds a fourth type of ephemeral user, the public account user.

I still think it is safer to base the deletion on whether the user is found in
the |users_| list or not. That way, we can guarantee ro clean up all users and
we can ensure that there will be no double-free accidents here.

[pastarmovj, 2012-12-03 11:39:03]

lgtm

[Nikita (slow), 2012-12-04 08:22:31]

lgtm

I've executed bunch of trybots

[commit-bot: I haz the power, 2012-12-04 10:21:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/11411314/2001

[commit-bot: I haz the power, 2012-12-04 12:18:15]

Change committed as 170943