Make the toggle touchscreen/touchpad shortcuts apply per-user

Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.

Committed: https://crrev.com/e2bc0da541c5601fbb3bb1bc7d93a0617827ca76
Cr-Commit-Position: refs/heads/master@{#431011}

[afakhry, 2016-11-03 01:13:54]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 01:14:40]

Dry run: 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-11-03 03:08:45]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2016-11-03 03:08:46]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[afakhry, 2016-11-03 15:56:22]

Description was changed from

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
==========

to

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.
==========

[afakhry, 2016-11-03 16:52:46]

[email protected] changed reviewers:
+ [email protected]

[afakhry, 2016-11-03 16:52:47]

Steven, could you please review? Thank you.

[stevenjb, 2016-11-04 22:55:58]

lgtm

[afakhry, 2016-11-07 17:08:46]

[email protected] changed reviewers:
+ [email protected]

[afakhry, 2016-11-07 17:08:47]

[email protected]: Please review changes in browser_prefs.cc

[afakhry, 2016-11-07 17:09:49]

[email protected] changed reviewers:
- [email protected]

[afakhry, 2016-11-07 17:10:47]

[email protected] changed reviewers:
+ [email protected]

[afakhry, 2016-11-07 17:10:48]

bauerb is OOO. gab@ please review changes in browser_prefs.cc

[gab, 2016-11-07 21:47:13]

https://codereview.chromium.org/2467023004/diff/1/chrome/browser/prefs/browse...
File chrome/browser/prefs/browser_prefs.cc (left):

https://codereview.chromium.org/2467023004/diff/1/chrome/browser/prefs/browse...
chrome/browser/prefs/browser_prefs.cc:428:
chromeos::system::InputDeviceSettings::RegisterPrefs(registry);
Cleanup unused state in MigrateObsoleteBrowserPrefs() below -- see paradigm used
in MigrateObsoleteProfilePrefs()

[afakhry, 2016-11-08 21:30:19]

The CQ bit was checked by [email protected] to run a CQ dry run

[afakhry, 2016-11-08 21:30:37]

https://codereview.chromium.org/2467023004/diff/1/chrome/browser/prefs/browse...
File chrome/browser/prefs/browser_prefs.cc (left):

https://codereview.chromium.org/2467023004/diff/1/chrome/browser/prefs/browse...
chrome/browser/prefs/browser_prefs.cc:428:
chromeos::system::InputDeviceSettings::RegisterPrefs(registry);
On 2016/11/07 21:47:13, gab wrote:
> Cleanup unused state in MigrateObsoleteBrowserPrefs() below -- see paradigm
used
> in MigrateObsoleteProfilePrefs() 

Done.

[commit-bot: I haz the power, 2016-11-08 21:30:57]

Dry run: 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-11-08 22:28:23]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2016-11-08 22:28:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[afakhry, 2016-11-08 22:36:16]

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:712:
local_state->ClearPref(prefs::kTouchPadEnabled);
Apparently the prefs have to be registered in the local state registry, but I
moved the registration to the profile registry.

This is causing multiple DCHECKS.

[gab, 2016-11-09 16:30:16]

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:712:
local_state->ClearPref(prefs::kTouchPadEnabled);
On 2016/11/08 22:36:16, afakhry wrote:
> Apparently the prefs have to be registered in the local state registry, but I
> moved the registration to the profile registry.
> 
> This is causing multiple DCHECKS.

Right, look at the paradigm for say kShownAutoLaunchInfobarDeprecated below. You
need to suppress the definition of the variable in pref_names.h, move it here in
anonymous namespace. Then still register above and clear it here.

At the same time, our practice is to have people that add such cleanup to also
remove 1+ year old cleanups (i.e. kShownAutoLaunchInfobarDeprecated
specifically) so we naturally cycle cleanups, mind doing that? Thanks

[gab, 2016-11-09 16:31:56]

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:712:
local_state->ClearPref(prefs::kTouchPadEnabled);
On 2016/11/09 16:30:16, gab wrote:
> On 2016/11/08 22:36:16, afakhry wrote:
> > Apparently the prefs have to be registered in the local state registry, but
I
> > moved the registration to the profile registry.
> > 
> > This is causing multiple DCHECKS.
> 
> Right, look at the paradigm for say kShownAutoLaunchInfobarDeprecated below.
You
> need to suppress the definition of the variable in pref_names.h, move it here
in
> anonymous namespace. Then still register above and clear it here.
> 
> At the same time, our practice is to have people that add such cleanup to also
> remove 1+ year old cleanups (i.e. kShownAutoLaunchInfobarDeprecated
> specifically) so we naturally cycle cleanups, mind doing that? Thanks

I'm sorry, in your case you don't want to suppress it from pref_names.h but
you'll still want to move it to the Local State section of the files:
https://cs.chromium.org/chromium/src/chrome/common/pref_names.cc?dr&q=%22****...

(I know it's a bit of a mess...)

[gab, 2016-11-09 16:33:31]

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:712:
local_state->ClearPref(prefs::kTouchPadEnabled);
On 2016/11/09 16:31:55, gab wrote:
> On 2016/11/09 16:30:16, gab wrote:
> > On 2016/11/08 22:36:16, afakhry wrote:
> > > Apparently the prefs have to be registered in the local state registry,
but
> I
> > > moved the registration to the profile registry.
> > > 
> > > This is causing multiple DCHECKS.
> > 
> > Right, look at the paradigm for say kShownAutoLaunchInfobarDeprecated below.
> You
> > need to suppress the definition of the variable in pref_names.h, move it
here
> in
> > anonymous namespace. Then still register above and clear it here.
> > 
> > At the same time, our practice is to have people that add such cleanup to
also
> > remove 1+ year old cleanups (i.e. kShownAutoLaunchInfobarDeprecated
> > specifically) so we naturally cycle cleanups, mind doing that? Thanks
> 
> I'm sorry, in your case you don't want to suppress it from pref_names.h but
> you'll still want to move it to the Local State section of the files:
>
https://cs.chromium.org/chromium/src/chrome/common/pref_names.cc?dr&q=%22****...
> 
> (I know it's a bit of a mess...)

Err.. you want to move it to the PROFILE PREFS section:
https://cs.chromium.org/chromium/src/chrome/common/pref_names.cc?dr&q=%22****...

Interestingly, it already appears to be there. So I guess it was poorly placed
before and is now made correct..!

[afakhry, 2016-11-09 17:21:28]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 17:21:43]

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

[afakhry, 2016-11-09 17:21:46]

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2467023004/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:712:
local_state->ClearPref(prefs::kTouchPadEnabled);
On 2016/11/09 16:33:31, gab wrote:
> On 2016/11/09 16:31:55, gab wrote:
> > On 2016/11/09 16:30:16, gab wrote:
> > > On 2016/11/08 22:36:16, afakhry wrote:
> > > > Apparently the prefs have to be registered in the local state registry,
> but
> > I
> > > > moved the registration to the profile registry.
> > > > 
> > > > This is causing multiple DCHECKS.
> > > 
> > > Right, look at the paradigm for say kShownAutoLaunchInfobarDeprecated
below.
> > You
> > > need to suppress the definition of the variable in pref_names.h, move it
> here
> > in
> > > anonymous namespace. Then still register above and clear it here.
> > > 
> > > At the same time, our practice is to have people that add such cleanup to
> also
> > > remove 1+ year old cleanups (i.e. kShownAutoLaunchInfobarDeprecated
> > > specifically) so we naturally cycle cleanups, mind doing that? Thanks
> > 
> > I'm sorry, in your case you don't want to suppress it from pref_names.h but
> > you'll still want to move it to the Local State section of the files:
> >
>
https://cs.chromium.org/chromium/src/chrome/common/pref_names.cc?dr&q=%22****...
> > 
> > (I know it's a bit of a mess...)
> 
> Err.. you want to move it to the PROFILE PREFS section:
>
https://cs.chromium.org/chromium/src/chrome/common/pref_names.cc?dr&q=%22****...
> 
> Interestingly, it already appears to be there. So I guess it was poorly placed
> before and is now made correct..!

Done. Thanks for the clarification!
kShownAutoLaunchInfobarDeprecated is removed, and the prefs are registered in
both local state and profile until migration is complete.

[commit-bot: I haz the power, 2016-11-09 18:11:59]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2016-11-09 18:11:59]

Dry run: This issue passed the CQ dry run.

[gab, 2016-11-09 19:19:48]

prefs lgtm

[afakhry, 2016-11-09 19:36:27]

The CQ bit was checked by [email protected]

[afakhry, 2016-11-09 19:36:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
[email protected]
Link to the patchset: https://codereview.chromium.org/2467023004/#ps60001
(title: "Doing the migration right")

[commit-bot: I haz the power, 2016-11-09 19:37:01]

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-11-09 19:42:30]

Description was changed from

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.
==========

to

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.
==========

[commit-bot: I haz the power, 2016-11-09 19:42:31]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-09 19:44:53]

Description was changed from

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.
==========

to

==========
Make the toggle touchscreen/touchpad shortcuts apply per-user

These are debug shortcuts behing a flag, if one user, who has the flag
enabled, disables the touchscreen, other signed-in users will be affected
too. It can be very confusing for users who have the debug shortcut flags
disabled.

This CL makes these settings apply per user.

BUG=597535
TEST=(1) Login with user_1, enabled ash-debug-shortcuts flag, Hit
Ctrl+Shift+T to disable touchscreen. Signout, signin, touchscreen
is still disabled.
(2) Signin with user_2, touchscreen is still enabled for user_2.
(3) Signin with user_1 and user_2 at the same time with multi-user
signin, switch between them using Ctrl+Alt+>, user_1 should have a
disabled touchscreen, user_2 has an enabled touchscreen.

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

[commit-bot: I haz the power, 2016-11-09 19:44:54]

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