Honor owner tap-to-click and mouse buttons swap on login screen

Honor owner tap-to-click and mouse buttons swap on login screen

BUG=144761, 131870, chromium-os:30927
TEST=check default during OOBE (no owner), change setting for owner check that changes are applied on login screen too


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=156607

[achuithb, 2012-09-10 18:07:30]

Please update the description with exactly what manual testing you did.

LGTM provided you add the comments as requested below.

Thank you for doing this!

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/base_login_display_host.cc (right):

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/base_login_display_host.cc:482:
system::mouse_settings::SetPrimaryButtonRight(
Please add a comment here about why this is necessary.

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
File chrome/browser/chromeos/preferences.cc (right):

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
chrome/browser/chromeos/preferences.cc:400: if
(chromeos::UserManager::Get()->IsCurrentUserOwner()) {
Please add a comment here why this is necessary.

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
chrome/browser/chromeos/preferences.cc:452: if
(chromeos::UserManager::Get()->IsCurrentUserOwner()) {
here too.

[Dmitry Polukhin, 2012-09-11 08:01:00]

+ sky for OWNER review in chrome/

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/login/...
File chrome/browser/chromeos/login/base_login_display_host.cc (right):

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/login/...
chrome/browser/chromeos/login/base_login_display_host.cc:482:
system::mouse_settings::SetPrimaryButtonRight(
On 2012/09/10 18:07:30, achuith.bhandarkar wrote:
> Please add a comment here about why this is necessary.

Done.

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
File chrome/browser/chromeos/preferences.cc (right):

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
chrome/browser/chromeos/preferences.cc:400: if
(chromeos::UserManager::Get()->IsCurrentUserOwner()) {
On 2012/09/10 18:07:30, achuith.bhandarkar wrote:
> Please add a comment here why this is necessary. 

Done.

http://codereview.chromium.org/10913163/diff/1/chrome/browser/chromeos/prefer...
chrome/browser/chromeos/preferences.cc:452: if
(chromeos::UserManager::Get()->IsCurrentUserOwner()) {
On 2012/09/10 18:07:30, achuith.bhandarkar wrote:
> here too.

Done.

[sky, 2012-09-11 16:04:02]

http://codereview.chromium.org/10913163/diff/10001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/10913163/diff/10001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main.cc:276:
local_state->RegisterStringPref(prefs::kHardwareKeyboardLayout,
It doesn't scale to add platform specific prefs to chrome_browser_main. Could we
move these to some where closer to chromeos and have chrome_browser_main call a
function to register all the chromeos specific prefs?

[achuithb, 2012-09-11 17:36:30]

http://codereview.chromium.org/10913163/diff/10001/chrome/browser/chrome_brow...
File chrome/browser/chrome_browser_main.cc (right):

http://codereview.chromium.org/10913163/diff/10001/chrome/browser/chrome_brow...
chrome/browser/chrome_browser_main.cc:276:
local_state->RegisterStringPref(prefs::kHardwareKeyboardLayout,
On 2012/09/11 16:04:02, sky wrote:
> It doesn't scale to add platform specific prefs to chrome_browser_main. Could
we
> move these to some where closer to chromeos and have chrome_browser_main call
a
> function to register all the chromeos specific prefs?

Maybe here:
http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/chr...

[Dmitry Polukhin, 2012-09-12 11:32:11]

PTAL

https://chromiumcodereview.appspot.com/10913163/diff/10001/chrome/browser/chr...
File chrome/browser/chrome_browser_main.cc (right):

https://chromiumcodereview.appspot.com/10913163/diff/10001/chrome/browser/chr...
chrome/browser/chrome_browser_main.cc:276:
local_state->RegisterStringPref(prefs::kHardwareKeyboardLayout,
On 2012/09/11 17:36:31, achuith.bhandarkar wrote:
> On 2012/09/11 16:04:02, sky wrote:
> > It doesn't scale to add platform specific prefs to chrome_browser_main.
Could
> we
> > move these to some where closer to chromeos and have chrome_browser_main
call
> a
> > function to register all the chromeos specific prefs?
> 
> Maybe here:
>
http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/chr...

We don't have any prefs registration in chrome_browser_main_chromeos.cc. I moved
all chrome os specific prefs registration to WizardController where we register
several login/OOBE prefs already.

[Dmitry Polukhin, 2012-09-12 14:31:09]

PTAL

https://chromiumcodereview.appspot.com/10913163/diff/10001/chrome/browser/chr...
File chrome/browser/chrome_browser_main.cc (right):

https://chromiumcodereview.appspot.com/10913163/diff/10001/chrome/browser/chr...
chrome/browser/chrome_browser_main.cc:276:
local_state->RegisterStringPref(prefs::kHardwareKeyboardLayout,
On 2012/09/12 11:32:12, Dmitry Polukhin wrote:
> On 2012/09/11 17:36:31, achuith.bhandarkar wrote:
> > On 2012/09/11 16:04:02, sky wrote:
> > > It doesn't scale to add platform specific prefs to chrome_browser_main.
> Could
> > we
> > > move these to some where closer to chromeos and have chrome_browser_main
> call
> > a
> > > function to register all the chromeos specific prefs?
> > 
> > Maybe here:
> >
>
http://code.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/chromeos/chr...
> 
> We don't have any prefs registration in chrome_browser_main_chromeos.cc. I
moved
> all chrome os specific prefs registration to WizardController where we
register
> several login/OOBE prefs already.

Returned kOwnerLocale and kHardwareKeyboardLayout back because kOwnerLocale pref
is used later in this function. So I reverted all changes in
chrome_browser_main.cc.

[Dmitry Polukhin, 2012-09-13 06:35:16]

Just friendly ping.
I would like to commit this change before M23 branch point.

[achuithb, 2012-09-13 10:08:39]

On 2012/09/13 06:35:16, Dmitry Polukhin wrote:
> Just friendly ping.
> I would like to commit this change before M23 branch point.

lgtm from me still, but please wait for Scott.

[achuithb, 2012-09-13 10:08:48]

http://codereview.chromium.org/10913163/diff/9003/chrome/browser/chromeos/pre...
File chrome/browser/chromeos/preferences.cc (right):

http://codereview.chromium.org/10913163/diff/9003/chrome/browser/chromeos/pre...
chrome/browser/chromeos/preferences.cc:402: // Remember owner preference to use
it on login screen.
nit: Let's change this comment to:
// Save owner preference in local state to use on login screen.

Same below.

[Dmitry Polukhin, 2012-09-13 10:16:38]

Scott, please take a look.

http://codereview.chromium.org/10913163/diff/9003/chrome/browser/chromeos/pre...
File chrome/browser/chromeos/preferences.cc (right):

http://codereview.chromium.org/10913163/diff/9003/chrome/browser/chromeos/pre...
chrome/browser/chromeos/preferences.cc:402: // Remember owner preference to use
it on login screen.
On 2012/09/13 10:08:48, achuith.bhandarkar wrote:
> nit: Let's change this comment to:
> // Save owner preference in local state to use on login screen.
> 
> Same below.

Done.

[sky, 2012-09-13 16:05:31]

LGTM

[commit-bot: I haz the power, 2012-09-13 16:26:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpolukhin@chromium.org/10913163/11

[commit-bot: I haz the power, 2012-09-13 19:46:00]

Change committed as 156607