Setting Guest last-active for the User Manager doesn't create the Guest.

Setting Guest last-active for the User Manager doesn't create the Guest.

This is a problem created by https://codereview.chromium.org/847733005.

BUG=445036
TEST=Lock a Profile, and the User Manager opens properly. Close all
other browsers then close the User Manager, ensure Chrome's completely
shut down, and re-open Chrome. The User Manager opens straight away.

Committed: https://crrev.com/3996e6245d6832ba0c29da91041b83a8e34a48e9
Cr-Commit-Position: refs/heads/master@{#313827}

[Mike Lerman, 2015-01-28 18:01:35]

mlerman@chromium.org changed reviewers:
+ noms@chromium.org

[Mike Lerman, 2015-01-28 18:01:36]

More reviews for noms!

You deserve agif, a super-attack cat!
http://i80.photobucket.com/albums/j164/sallypina/Animated-AttackCat.gif

[noms (inactive), 2015-01-28 19:17:19]

https://codereview.chromium.org/885493005/diff/1/chrome/browser/profiles/prof...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/1/chrome/browser/profiles/prof...
chrome/browser/profiles/profiles_state.cc:190:
active_profile_path.BaseName().MaybeAsASCII());
Shouldn't this be guest_profile_path or something? active_profile_path is defo
not guest (or else we would have returned in L179), so isn't this basically a
no-op?

[Mike Lerman, 2015-01-28 20:19:45]

https://codereview.chromium.org/885493005/diff/1/chrome/browser/profiles/prof...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/1/chrome/browser/profiles/prof...
chrome/browser/profiles/profiles_state.cc:190:
active_profile_path.BaseName().MaybeAsASCII());
On 2015/01/28 19:17:19, Monica Dinculescu wrote:
> Shouldn't this be guest_profile_path or something? active_profile_path is defo
> not guest (or else we would have returned in L179), so isn't this basically a
> no-op?

The best no-op ever!!!

Changed :)

[Mike Lerman, 2015-01-28 21:06:52]

mlerman@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Mike Lerman, 2015-01-28 21:06:52]

Adding Alexei to review the cocoa code.

Thanks!

[Alexei Svitkine (slow), 2015-01-28 21:12:32]

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/user_manager_mac.mm (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:28: void
ChangeAppControllerForProfile(Profile* profile,
Add a comment.

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:37: }
Add "  // namespace" comment.

[Mike Lerman, 2015-01-29 14:01:58]

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/profiles/user_manager_mac.mm (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:28: void
ChangeAppControllerForProfile(Profile* profile,
On 2015/01/28 21:12:32, Alexei Svitkine wrote:
> Add a comment.

Done.

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/profiles/user_manager_mac.mm:37: }
On 2015/01/28 21:12:32, Alexei Svitkine wrote:
> Add "  // namespace" comment.

Done.

[Alexei Svitkine (slow), 2015-01-29 16:10:48]

lgtm

[noms (inactive), 2015-01-29 16:15:13]

profiles lgtm % nits

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profiles_state.cc:179: if (active_profile_path ==
ProfileManager::GetGuestProfilePath())
nit: can we store this into a variable so that we don't do the string
concatenation it does twice?

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profiles_state.h (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profiles_state.h:77: // profile is now guest.
Hmm, this comment is a bit confusing. Why wouldn't it be guest, and is "now"
before or after the function is called? Time travel is hard :(

[Mike Lerman, 2015-01-29 16:46:51]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2015-01-29 16:47:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885493005/80001

[Mike Lerman, 2015-01-29 16:51:40]

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profiles_state.cc:179: if (active_profile_path ==
ProfileManager::GetGuestProfilePath())
On 2015/01/29 16:15:13, Monica Dinculescu wrote:
> nit: can we store this into a variable so that we don't do the string
> concatenation it does twice?

Done.

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
File chrome/browser/profiles/profiles_state.h (right):

https://codereview.chromium.org/885493005/diff/40001/chrome/browser/profiles/...
chrome/browser/profiles/profiles_state.h:77: // profile is now guest.
On 2015/01/29 16:15:13, Monica Dinculescu wrote:
> Hmm, this comment is a bit confusing. Why wouldn't it be guest, and is "now"
> before or after the function is called? Time travel is hard :(

Done.

[commit-bot: I haz the power, 2015-01-29 18:31:28]

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

[commit-bot: I haz the power, 2015-01-29 18:31:29]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike Lerman, 2015-01-29 18:56:07]

Noms, can you glance at changeset #6? had to make a test not fail.

[noms (inactive), 2015-01-29 19:06:10]

On 2015/01/29 18:56:07, Mike Lerman wrote:
> Noms, can you glance at changeset #6? had to make a test not fail.

hmm, that makes sense, but why isn't the profile in the PIC in the test? We
already check that it's not the guest profile...

[Mike Lerman, 2015-01-29 19:45:27]

On 2015/01/29 19:06:10, Monica Dinculescu wrote:
> On 2015/01/29 18:56:07, Mike Lerman wrote:
> > Noms, can you glance at changeset #6? had to make a test not fail.
> 
> hmm, that makes sense, but why isn't the profile in the PIC in the test? We
> already check that it's not the guest profile...

The "last active" profile isn't set as a pref in the test, so we look for
"Default", which isn't in the PIC (or anywhere in this test).

[Mike Lerman, 2015-01-29 22:16:08]

New patchset, the test doesn't fail anymore, and more nicely :) SG?

[noms (inactive), 2015-01-29 22:18:11]

lgtm (there's a nit. sorry!)

https://codereview.chromium.org/885493005/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profiles_state.cc:187: }
nit: noesss, no {}

[Mike Lerman, 2015-01-29 22:22:11]

https://codereview.chromium.org/885493005/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profiles_state.cc (right):

https://codereview.chromium.org/885493005/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profiles_state.cc:187: }
On 2015/01/29 22:18:11, Monica Dinculescu wrote:
> nit: noesss, no {}

Done.

[Mike Lerman, 2015-01-29 22:22:24]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2015-01-29 22:23:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885493005/140001

[commit-bot: I haz the power, 2015-01-29 23:33:59]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-01-29 23:35:23]

Patchset 8 (id:??) landed as
https://crrev.com/3996e6245d6832ba0c29da91041b83a8e34a48e9
Cr-Commit-Position: refs/heads/master@{#313827}