Profile path is sent instead of an index to focus a user pod

Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
Committed: https://crrev.com/c91db6cc7f37dd9d28c539a437423834fa058d10
Cr-Commit-Position: refs/heads/master@{#384339}

[lwchkg, 2016-03-24 04:05:44]

Description was changed from

==========
Profile path is sent to chrome://user-manager instead of the index in
ProfileInfoCache to indicate selected pod

BUG=596280
==========

to

==========
Profile path is sent to chrome://user-manager instead of the index to
select a user pod

BUG=596280
==========

[lwchkg, 2016-03-24 04:13:40]

Description was changed from

==========
Profile path is sent to chrome://user-manager instead of the index to
select a user pod

BUG=596280
==========

to

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug when SendUserList in user_manager_screen_handler.cc sent the user
list in a different order (to enable locale-aware sorting).

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

[lwchkg, 2016-03-24 04:15:49]

Description was changed from

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug when SendUserList in user_manager_screen_handler.cc sent the user
list in a different order (to enable locale-aware sorting).

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

to

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

[lwchkg, 2016-03-24 04:15:49]

lwchkg@gmail.com changed reviewers:
+ achuith@chromium.org, mlerman@chromium.org

[lwchkg, 2016-03-24 04:21:52]

Dear Mike and achuith@,

  PTAL. This CL fixes bug 596280, which is introduced in
https://crrev.com/376977

Regards,
WC Leung.

c.c. Anthony, Roger.

[achuithb, 2016-03-24 18:12:32]

achuith@chromium.org changed reviewers:
+ dzhioev@chromium.org

[achuithb, 2016-03-24 18:12:32]

Pasha is more familiar with this code

[lwchkg, 2016-03-24 19:20:14]

lwchkg@gmail.com changed reviewers:
+ dbeam@chromium.org
- achuith@chromium.org

[lwchkg, 2016-03-24 19:24:13]

+dbeam@

Dear Mike, dzhioev@, Dan Beam,

  PTAL. This CL fixes bug 596280, which is introduced in
https://crrev.com/376977

  Note to Dan Beam: added you to the CL because a new file is added to
chrome/test/data/webui.

Regards,
WC Leung.

c.c. Anthony, Roger.

[dzhioev (left Google), 2016-03-25 22:02:48]

user_pod_row.js LGTM

[Mike Lerman, 2016-03-29 13:40:03]

lgtm

[lwchkg, 2016-03-29 15:01:03]

On 2016/03/29 13:40:03, Mike Lerman wrote:
> lgtm

Should I make a merge request? If yes, which milestone should I request for?

[Mike Lerman, 2016-03-29 16:59:21]

On 2016/03/29 15:01:03, lwchkg wrote:
> On 2016/03/29 13:40:03, Mike Lerman wrote:
> > lgtm
> 
> Should I make a merge request? If yes, which milestone should I request for?

I don't think this is merge-worthy, especially since we branched over a month
ago.

[lwchkg, 2016-03-30 16:03:10]

dbeam@: ping. I need your review on chrome/test/data/webui/*. Thanks a lot!

[Dan Beam, 2016-03-30 17:53:29]

i don't think you're actually running the file you've added...

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/profile_window_browsertest.js (right):

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:5: function
testNoPodFocuesd() {
Focused

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:10: function
testPodFocuesd(profile_path) {
Focused

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:10: function
testPodFocuesd(profile_path) {
jsVarsLikeThis (i.e. profile_path -> profilePath)

https://codereview.chromium.org/1828143002/diff/20001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/1828143002/diff/20001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:3010: for (i = 0; pod = this.pods[i];
++i) {
nit: i would just put

  for (var i = 0, pod; ...

here instead of at the top of the method

[lwchkg, 2016-03-31 14:43:37]

The CQ bit was checked by lwchkg@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 14:43:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1828143002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1828143002/40001

[lwchkg, 2016-03-31 14:48:17]

> i don't think you're actually running the file you've added...

New patch uploaded. Anyway the added file is called by
profile_window_browsertest.cc.

https://codereview.chromium.org/1828143002/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/profile_window_browsertest.cc (right):

https://codereview.chromium.org/1828143002/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:268:
EXPECT_TRUE(RunJavascriptTest("testNoPodFocuesd"));
The js file is called here and also in a similar statement below. Sadly the same
typo occured (and the test are "correctly" run.)

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/profile_window_browsertest.js (right):

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:5: function
testNoPodFocuesd() {
On 2016/03/30 17:53:29, Dan Beam wrote:
> Focused

Done.

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:10: function
testPodFocuesd(profile_path) {
On 2016/03/30 17:53:29, Dan Beam wrote:
> Focused

Done.

https://codereview.chromium.org/1828143002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/profile_window_browsertest.js:10: function
testPodFocuesd(profile_path) {
On 2016/03/30 17:53:28, Dan Beam wrote:
> jsVarsLikeThis (i.e. profile_path -> profilePath)

Done.

https://codereview.chromium.org/1828143002/diff/20001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/1828143002/diff/20001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:3010: for (i = 0; pod = this.pods[i];
++i) {
On 2016/03/30 17:53:29, Dan Beam wrote:
> nit: i would just put
> 
>   for (var i = 0, pod; ...
> 
> here instead of at the top of the method

Done. However I do worry about non-JS coders thinking that putting var here
declares block scoped variables.

Anyway, WDYT about using "let" instead of "var"?

[commit-bot: I haz the power, 2016-03-31 16:04:35]

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

[commit-bot: I haz the power, 2016-03-31 16:04:36]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-03-31 17:51:01]

lgtm

https://codereview.chromium.org/1828143002/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/profile_window_browsertest.cc (right):

https://codereview.chromium.org/1828143002/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/profile_window_browsertest.cc:268:
EXPECT_TRUE(RunJavascriptTest("testNoPodFocuesd"));
On 2016/03/31 14:48:16, lwchkg wrote:
> The js file is called here and also in a similar statement below. Sadly the
same
> typo occured (and the test are "correctly" run.)

ah, i see, sorry for missing this

[lwchkg, 2016-03-31 18:18:29]

The CQ bit was checked by lwchkg@gmail.com

[lwchkg, 2016-03-31 18:18:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
dzhioev@chromium.org, mlerman@chromium.org
Link to the patchset: https://codereview.chromium.org/1828143002/#ps40001
(title: "Fix typo and nits")

[commit-bot: I haz the power, 2016-03-31 18:19:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1828143002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1828143002/40001

[commit-bot: I haz the power, 2016-03-31 18:27:02]

Description was changed from

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

to

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

[commit-bot: I haz the power, 2016-03-31 18:27:03]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-31 18:28:35]

Description was changed from

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280
==========

to

==========
Profile path is sent instead of the index to focus a user pod

Currently the index in ProfileInfoCache is sent to chrome://user-manager
to initially select the user pod when we use "Exit and childlock"
function. This was a fragile implementation, and unfortunately led to a
bug (596280) when SendUserList in user_manager_screen_handler.cc sent
the user list in a different order.

Now a profile path, which does not depend on any order, is sent instead.

BUG=596280

Committed: https://crrev.com/c91db6cc7f37dd9d28c539a437423834fa058d10
Cr-Commit-Position: refs/heads/master@{#384339}
==========

[commit-bot: I haz the power, 2016-03-31 18:28:37]

Patchset 3 (id:??) landed as
https://crrev.com/c91db6cc7f37dd9d28c539a437423834fa058d10
Cr-Commit-Position: refs/heads/master@{#384339}