Enabled/disable touch screen in TabletPowerButtonController

Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for TouchScreenEnabled, we can disable touch screen events when backlights are forced off so that no screen UI events can be triggered and enable touch screen when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user pref, backlights-forced-off related touch screen status is per device pref. The final hardware touchscreen status is determined by the rule: enabled_in_local_state && enabled_in_user_pref

Committed: https://crrev.com/4c8baedbf7a0b83ec7b5fd77dc1f5d5627fe6dd7
Cr-Commit-Position: refs/heads/master@{#436177}

[Qiang(Joe) Xu, 2016-11-29 19:50:37]

Description was changed from

==========
ToggleTouchScreen in TabletPowerButtonController

BUG=655388
==========

to

==========
ToggleTouchScreen in TabletPowerButtonController

By exposing IsTouchscreenEnabled in input_device_settings.h, we can disable
touch screen events when backlights are forced off so that no screen UI events
can be triggered and enable touch screen when stopping backlights forced off is
called.

Please see the last comment in the bug link for more info.

BUG=655388
TEST=manual test on device, and add test coverage.
==========

[Qiang(Joe) Xu, 2016-11-29 19:50:37]

warx@chromium.org changed reviewers:
+ derat@chromium.org, stevenjb@chromium.org

[Qiang(Joe) Xu, 2016-11-29 20:01:17]

Hi Daniel & Steven, this cl is proposed to solve the issue described in the bug
link.

Based on the suggestion from Daniel, this issue probably can also be solved on
powerd side. Another puzzle for me is cyan/minnie don't have this issue.

[Daniel Erat, 2016-11-29 21:03:37]

can you also get a review from whoever wrote the input_device_settings code?

https://codereview.chromium.org/2533373002/diff/20001/ash/common/shell_delega...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/20001/ash/common/shell_delega...
ash/common/shell_delegate.h:140: virtual void ToggleTouchpad() {}
"toggle" methods always seem a bit strange to me, unless the thing that they're
doing is asynchronous. assuming that this is synchronous, could you add a
SetTouchscreenEnabled(bool) method and call that instead?

https://codereview.chromium.org/2533373002/diff/20001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/20001/ash/system/chromeos/pow...
ash/system/chromeos/power/tablet_power_button_controller.cc:177: void
TabletPowerButtonController::SetBacklightsForcedOff(bool forced_off) {
this should probably be renamed now, as it does more than just controlling
whether the backlight is forced off. maybe rename it to e.g. SetDisplayForcedOff
and document that it controls both the backlight and the touchscreen?

[Qiang(Joe) Xu, 2016-11-29 23:40:51]

Patchset #3 (id:40001) has been deleted

[Qiang(Joe) Xu, 2016-11-30 20:01:42]

Patchset #3 (id:60001) has been deleted

[Qiang(Joe) Xu, 2016-11-30 21:56:51]

Description was changed from

==========
ToggleTouchScreen in TabletPowerButtonController

By exposing IsTouchscreenEnabled in input_device_settings.h, we can disable
touch screen events when backlights are forced off so that no screen UI events
can be triggered and enable touch screen when stopping backlights forced off is
called.

Please see the last comment in the bug link for more info.

BUG=655388
TEST=manual test on device, and add test coverage.
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing IsTouchscreenEnabled/SetTouchscreenEnabled api in
input_device_settings.h, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called.

Please see the last comment in the bug link for more info.

BUG=655388
TEST=manual test on device, and add test coverage.
==========

[Qiang(Joe) Xu, 2016-11-30 21:58:28]

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

[Qiang(Joe) Xu, 2016-11-30 22:05:02]

Patchset #3 (id:80001) has been deleted

[Qiang(Joe) Xu, 2016-11-30 22:09:11]

+achuith@ for review on input_device_settings.h related changes.

Because I add IsTouchscreenEnabled/SetTouchscreenEnabled and remove
ToggleTouchscreen. Please see if that is a reasonable change. And if that is
reasonable, I think we need to rename the private method SetTouchscreensEnabled
for clarification, right? Maybe SetInternalTouchscreenEnabled. But I will leave
it for the next change once this change sounds good to you.

The new patch also makes TabletPowerButtonController overrides
OnLoginStateChanged method for the reason described in the inline comments.

https://codereview.chromium.org/2533373002/diff/20001/ash/common/shell_delega...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/20001/ash/common/shell_delega...
ash/common/shell_delegate.h:140: virtual void ToggleTouchpad() {}
On 2016/11/29 21:03:36, Daniel Erat wrote:
> "toggle" methods always seem a bit strange to me, unless the thing that
they're
> doing is asynchronous. assuming that this is synchronous, could you add a
> SetTouchscreenEnabled(bool) method and call that instead?

done

https://codereview.chromium.org/2533373002/diff/20001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/20001/ash/system/chromeos/pow...
ash/system/chromeos/power/tablet_power_button_controller.cc:177: void
TabletPowerButtonController::SetBacklightsForcedOff(bool forced_off) {
On 2016/11/29 21:03:36, Daniel Erat wrote:
> this should probably be renamed now, as it does more than just controlling
> whether the backlight is forced off. maybe rename it to e.g.
SetDisplayForcedOff
> and document that it controls both the backlight and the touchscreen?

Done.

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.h (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.h:67: void
OnLoginStateChanged(LoginStatus status) override;
This change is to fix the bug for this:
On user logged in state, SetBacklightsForcedOff by pressing power button.
Touchscreen is disabled and saved to the prefs; backlights are forced off and
managed by powerd.

Now deploy_chrome, and when finished, click the touchpad to stop the backlights
forced off. However, since now no user is logged in, touchscreen enabled is not
saved to the previous prefs, but saved to the offtherecord-profile. When user is
logged in, touchscreen is still disabled.

Solution here is when OnLoginStateChanged is called, making a sync call to sync
the touchscreen state with backlights state.

[Daniel Erat, 2016-11-30 22:58:07]

https://codereview.chromium.org/2533373002/diff/100001/ash/common/accelerator...
File ash/common/accelerators/debug_commands.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/common/accelerator...
ash/common/accelerators/debug_commands.cc:133:
delegate->SetTouchscreenEnabled(!touch_screen_status);
nit: i'd just inline this, e.g.

  delegate->SetTouchscreenEnabled(!delegate->IsTouchscreenEnabled());

https://codereview.chromium.org/2533373002/diff/100001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/common/shell_deleg...
ash/common/shell_delegate.h:140: virtual void SetTouchscreenEnabled(bool
enabled) {}
should this also be abstract? it feels a bit strange to have an abstract
IsTouchscreenEnabled() method but this possibly-unimplemented
SetTouchscreensEnabled() method. (i realize that this is probably due to the old
ToggleTouchscreen() method having an empty definition here.)

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:148:
WmShell::Get()->delegate()->SetTouchscreenEnabled(touch_screen_enabled);
is there a possibility of a race here? is it guaranteed that the profile's state
will be applied before this method is called and the forced-off state is
applied?

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:197: // Set the
enabled state of touchscreen to |touch_screen_enabled|.
remove this comment; it's obvious from the code

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller_unittest.cc
(right):

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller_unittest.cc:365: }
can you also add tests for the login state stuff too?

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:247: const bool
touch_screen_status = IsTouchscreenEnabled();
mind just inlining this in the SetTouchscreensEnabled() call below?

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:258: bool
InputDeviceSettings::IsTouchscreenEnabled() const {
mind renaming this to IsTouchscreenEnabledInPrefs() so it clearer that it isn't
referring to the same state as SetTouchscreensEnabled()?

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.h (right):

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:190: // Toggles the
status of Touchpad on or off and updates the local prefs.
nit: s/Touchpad/touchpad/

[Qiang(Joe) Xu, 2016-12-02 08:04:31]

Patchset #5 (id:140001) has been deleted

[Qiang(Joe) Xu, 2016-12-02 08:11:52]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing IsTouchscreenEnabled/SetTouchscreenEnabled api in
input_device_settings.h, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called.

Please see the last comment in the bug link for more info.

BUG=655388
TEST=manual test on device, and add test coverage.
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible and isolated with toggle
touchscreen shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

[Qiang(Joe) Xu, 2016-12-02 08:13:05]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible and isolated with toggle
touchscreen shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

[Qiang(Joe) Xu, 2016-12-02 08:13:33]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

[Qiang(Joe) Xu, 2016-12-02 08:19:21]

warx@chromium.org changed reviewers:
+ afakhry@chromium.org, gab@chromium.org
- stevenjb@chromium.org

[Qiang(Joe) Xu, 2016-12-02 17:34:12]

Patchset #5 (id:160001) has been deleted

[Qiang(Joe) Xu, 2016-12-02 17:42:52]

Hi all, this change basically adds a new local pref for touchscreenenabled, and
separate SetPref and UpdateStatusOnHardware. Please take a look, thanks!

For reviewers:
derat@ for ash/, chrome/browser/chromeos, chrome/browser/ui/ash,
gab@ for prefs
achuith@ for author review on input_device_settings
FYI afakhry@ since this change is also strongly related to your change.

https://codereview.chromium.org/2533373002/diff/100001/ash/common/accelerator...
File ash/common/accelerators/debug_commands.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/common/accelerator...
ash/common/accelerators/debug_commands.cc:133:
delegate->SetTouchscreenEnabled(!touch_screen_status);
On 2016/11/30 22:58:07, Daniel Erat wrote:
> nit: i'd just inline this, e.g.
> 
>   delegate->SetTouchscreenEnabled(!delegate->IsTouchscreenEnabled());

Done.

https://codereview.chromium.org/2533373002/diff/100001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/common/shell_deleg...
ash/common/shell_delegate.h:140: virtual void SetTouchscreenEnabled(bool
enabled) {}
On 2016/11/30 22:58:07, Daniel Erat wrote:
> should this also be abstract? it feels a bit strange to have an abstract
> IsTouchscreenEnabled() method but this possibly-unimplemented
> SetTouchscreensEnabled() method. (i realize that this is probably due to the
old
> ToggleTouchscreen() method having an empty definition here.)

Done.

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:148:
WmShell::Get()->delegate()->SetTouchscreenEnabled(touch_screen_enabled);
On 2016/11/30 22:58:07, Daniel Erat wrote:
> is there a possibility of a race here? is it guaranteed that the profile's
state
> will be applied before this method is called and the forced-off state is
> applied?

done by removing this.

https://codereview.chromium.org/2533373002/diff/100001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:197: // Set the
enabled state of touchscreen to |touch_screen_enabled|.
On 2016/11/30 22:58:07, Daniel Erat wrote:
> remove this comment; it's obvious from the code

Done.

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:247: const bool
touch_screen_status = IsTouchscreenEnabled();
On 2016/11/30 22:58:07, Daniel Erat wrote:
> mind just inlining this in the SetTouchscreensEnabled() call below?

Done.

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:258: bool
InputDeviceSettings::IsTouchscreenEnabled() const {
On 2016/11/30 22:58:07, Daniel Erat wrote:
> mind renaming this to IsTouchscreenEnabledInPrefs() so it clearer that it
isn't
> referring to the same state as SetTouchscreensEnabled()?

Done.

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.h (right):

https://codereview.chromium.org/2533373002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:190: // Toggles the
status of Touchpad on or off and updates the local prefs.
On 2016/11/30 22:58:07, Daniel Erat wrote:
> nit: s/Touchpad/touchpad/

Done.

[gab, 2016-12-02 19:10:02]

prefs lgtm w/ nit

https://codereview.chromium.org/2533373002/diff/180001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1319: // *************** LOCAL STATE ***************
Local State prefs should be below this line (and yours in an OS_CHROME section).

[achuithb, 2016-12-02 19:41:42]

input_device_settings lgtm

[afakhry, 2016-12-02 20:04:28]

https://codereview.chromium.org/2533373002/diff/180001/ash/common/accelerator...
File ash/common/accelerators/debug_commands.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/ash/common/accelerator...
ash/common/accelerators/debug_commands.cc:133:
!delegate->IsTouchscreenEnabledInPrefs(false), false);
Nit: false /* is_local */.

https://codereview.chromium.org/2533373002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:296:
IsTouchscreenEnabledInPrefs(false));
I think you need to AND them together.

[Qiang(Joe) Xu, 2016-12-02 21:23:14]

https://codereview.chromium.org/2533373002/diff/180001/ash/common/accelerator...
File ash/common/accelerators/debug_commands.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/ash/common/accelerator...
ash/common/accelerators/debug_commands.cc:133:
!delegate->IsTouchscreenEnabledInPrefs(false), false);
On 2016/12/02 20:04:27, afakhry wrote:
> Nit: false /* is_local */.

Done.

https://codereview.chromium.org/2533373002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:296:
IsTouchscreenEnabledInPrefs(false));
On 2016/12/02 20:04:27, afakhry wrote:
> I think you need to AND them together.

I once thought it should do AND, but then find it should properly do XNOR?
Because the shortcut actually is doing ToggleTouchScreen, so it should flip the
final effect if it differs with the local pref, right?

I leave no change here for now. We can also discuss it per offline.

https://codereview.chromium.org/2533373002/diff/180001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/180001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1319: // *************** LOCAL STATE ***************
On 2016/12/02 19:10:02, gab wrote:
> Local State prefs should be below this line (and yours in an OS_CHROME
section).

Done.

[Daniel Erat, 2016-12-02 21:48:57]

https://codereview.chromium.org/2533373002/diff/200001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/common/shell_deleg...
ash/common/shell_delegate.h:138: virtual bool IsTouchscreenEnabledInPrefs(bool
is_local) const = 0;
is_local feels a bit vague. would you mind renaming this to something like
use_local_state throughout?

https://codereview.chromium.org/2533373002/diff/200001/ash/shell/shell_delega...
File ash/shell/shell_delegate_impl.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/shell/shell_delega...
ash/shell/shell_delegate_impl.cc:269: return true;
should these have NOTIMPLEMENTED() as well? i think we usually use
NOTIMPLEMENTED() for methods that aren't implemented yet but that should be in
the future (i believe the macro was introduced when porting chrome to
non-windows platforms). is that the case here, or will these never be
implemented?

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:194: bool
touch_screen_enabled = !forced_off;
nit: this is only used in one place, so it'd probably be cleaner to inline it in
the SetTouchscreenEnabledInPrefs call

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.h:75: // Set backlights
to |forced_off| and the corresponding enabled state of
nit: maybe this?

// Updates the power manager's backlights-forced-off state and
// enables or disables the touchscreen. No-op if 
// |backlights_forced_off_| already equals |forced_off|.

https://codereview.chromium.org/2533373002/diff/200001/ash/test/test_shell_de...
File ash/test/test_shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/test/test_shell_de...
ash/test/test_shell_delegate.h:82: bool touch_screen_enabled_in_local_pref_;
nit: s/touch_screen/touchscreen/ (since the method names use "Touchscreen"
rather than "TouchScreen"; it's just one word)

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:288: if (!user_prefs)
is this expected to happen or should a warning/error be logged?

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:295:
SetTouchscreensEnabled(IsTouchscreenEnabledInPrefs(true) ==
this is a bit tricky, so i'd probably write it out as:

  bool enabled_in_local_state = IsTouchscreenEnabledInPrefs(true);
  bool enabled_in_user_prefs = IsTouchscreenEnabledInPrefs(false);
  SetTouchscreensEnabled(enabled_in_local_state == enabled_in_user_prefs);

but i actually don't understand this statement. we enable the touchscreen if
it's enabled in both places but also if it's disabled in both places?

i see that there was earlier discussion abou tthis, but i think it'd be
worthwhile to figure it out now (and add a comment if this is correct, because
it looks strange just from reading the code).

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.h (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:177: // Registers local
pref names for touch screen status.
nit: s/touch screen/touchscreen/

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:191: // is set in local
pref, otherwise in user pref.
nit: s/local pref/local state/

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1887: const char kLocalTouchScreenEnabled[] =
"events.local_touch_screen.enabled";
it looks like this doesn't match the order from the header. can you move it so
it does?

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.h (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.h:298: extern const char kLocalTouchScreenEnabled[];
i know that you're matching the existing style, but it's unfortunate that these
are "TouchScreen" and "TouchPad" (i.e. two words) rather than the "Touchscreen"
and "Touchpad" (which i think are more common, and also used in other places).

[afakhry, 2016-12-02 22:33:20]

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:295:
SetTouchscreensEnabled(IsTouchscreenEnabledInPrefs(true) ==
On 2016/12/02 21:48:56, Daniel Erat wrote:
> this is a bit tricky, so i'd probably write it out as:
> 
>   bool enabled_in_local_state = IsTouchscreenEnabledInPrefs(true);
>   bool enabled_in_user_prefs = IsTouchscreenEnabledInPrefs(false);
>   SetTouchscreensEnabled(enabled_in_local_state == enabled_in_user_prefs);
> 
> but i actually don't understand this statement. we enable the touchscreen if
> it's enabled in both places but also if it's disabled in both places?
> 
> i see that there was earlier discussion abou tthis, but i think it'd be
> worthwhile to figure it out now (and add a comment if this is correct, because
> it looks strange just from reading the code).

I discussed with him offline. This should be an AND, since we're just updating
the status here from both local_state and prefs. Toggling the status as a result
of the shortcut is already done in debug_commands.cc.

[Qiang(Joe) Xu, 2016-12-02 23:55:11]

Hi, new patch based on derat@'s comments, thanks!

https://codereview.chromium.org/2533373002/diff/200001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/common/shell_deleg...
ash/common/shell_delegate.h:138: virtual bool IsTouchscreenEnabledInPrefs(bool
is_local) const = 0;
On 2016/12/02 21:48:56, Daniel Erat wrote:
> is_local feels a bit vague. would you mind renaming this to something like
> use_local_state throughout?

Done.

https://codereview.chromium.org/2533373002/diff/200001/ash/shell/shell_delega...
File ash/shell/shell_delegate_impl.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/shell/shell_delega...
ash/shell/shell_delegate_impl.cc:269: return true;
On 2016/12/02 21:48:56, Daniel Erat wrote:
> should these have NOTIMPLEMENTED() as well? i think we usually use
> NOTIMPLEMENTED() for methods that aren't implemented yet but that should be in
> the future (i believe the macro was introduced when porting chrome to
> non-windows platforms). is that the case here, or will these never be
> implemented?

I think they will never be implemented here as this method needs chrome/browser
dependency.

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.cc:194: bool
touch_screen_enabled = !forced_off;
On 2016/12/02 21:48:56, Daniel Erat wrote:
> nit: this is only used in one place, so it'd probably be cleaner to inline it
in
> the SetTouchscreenEnabledInPrefs call

Done.

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
File ash/system/chromeos/power/tablet_power_button_controller.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/system/chromeos/po...
ash/system/chromeos/power/tablet_power_button_controller.h:75: // Set backlights
to |forced_off| and the corresponding enabled state of
On 2016/12/02 21:48:56, Daniel Erat wrote:
> nit: maybe this?
> 
> // Updates the power manager's backlights-forced-off state and
> // enables or disables the touchscreen. No-op if 
> // |backlights_forced_off_| already equals |forced_off|.

Done.

https://codereview.chromium.org/2533373002/diff/200001/ash/test/test_shell_de...
File ash/test/test_shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/200001/ash/test/test_shell_de...
ash/test/test_shell_delegate.h:82: bool touch_screen_enabled_in_local_pref_;
On 2016/12/02 21:48:56, Daniel Erat wrote:
> nit: s/touch_screen/touchscreen/ (since the method names use "Touchscreen"
> rather than "TouchScreen"; it's just one word)

Done.

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:288: if (!user_prefs)
On 2016/12/02 21:48:56, Daniel Erat wrote:
> is this expected to happen or should a warning/error be logged?

I take a look at this:
https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager....

See Line 425 and line 443 (and line 448 has a CHECK). So only Line 425 could
introduce nullptr in the call chain I think. I don't know if we need a
LOG(WARNING) here, maybe in line 425 there?

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:295:
SetTouchscreensEnabled(IsTouchscreenEnabledInPrefs(true) ==
On 2016/12/02 21:48:56, Daniel Erat wrote:
> this is a bit tricky, so i'd probably write it out as:
> 
>   bool enabled_in_local_state = IsTouchscreenEnabledInPrefs(true);
>   bool enabled_in_user_prefs = IsTouchscreenEnabledInPrefs(false);
>   SetTouchscreensEnabled(enabled_in_local_state == enabled_in_user_prefs);
> 
> but i actually don't understand this statement. we enable the touchscreen if
> it's enabled in both places but also if it's disabled in both places?
> 
> i see that there was earlier discussion abou tthis, but i think it'd be
> worthwhile to figure it out now (and add a comment if this is correct, because
> it looks strange just from reading the code).

Done.

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.h (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:177: // Registers local
pref names for touch screen status.
On 2016/12/02 21:48:57, Daniel Erat wrote:
> nit: s/touch screen/touchscreen/

Done.

https://codereview.chromium.org/2533373002/diff/200001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:191: // is set in local
pref, otherwise in user pref.
On 2016/12/02 21:48:57, Daniel Erat wrote:
> nit: s/local pref/local state/

Done.

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1887: const char kLocalTouchScreenEnabled[] =
"events.local_touch_screen.enabled";
On 2016/12/02 21:48:57, Daniel Erat wrote:
> it looks like this doesn't match the order from the header. can you move it so
> it does?

Done.

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.h (right):

https://codereview.chromium.org/2533373002/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.h:298: extern const char kLocalTouchScreenEnabled[];
On 2016/12/02 21:48:57, Daniel Erat wrote:
> i know that you're matching the existing style, but it's unfortunate that
these
> are "TouchScreen" and "TouchPad" (i.e. two words) rather than the
"Touchscreen"
> and "Touchpad" (which i think are more common, and also used in other places).

Done.

[Daniel Erat, 2016-12-03 00:08:44]

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:137: // prefs, otherwise from user prefs.
nit: s/local prefs/local state/

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:141: // pref is set in local pref, otherwise in user
pref.
nit: s/local pref/local state/, s/user pref/user prefs/

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:145: // Updates the on/off status of the touchscreen
from the combination of local
nit: s/local/local state/

i think it'd probably be worthwhile to also document in this comment how the two
possible values interact (i think that both need to be set to true, right?). not
having this documented in the interface is a bit confusing, since one needs to
look at all of the implementations to figure out what the desired behavior is.

https://codereview.chromium.org/2533373002/diff/220001/ash/mus/shell_delegate...
File ash/mus/shell_delegate_mus.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/mus/shell_delegate...
ash/mus/shell_delegate_mus.cc:235: NOTIMPLEMENTED();
just to double-check, are these going to be implemented later? if so,
NOTIMPLEMENTED() seems correct.

https://codereview.chromium.org/2533373002/diff/220001/ash/shell/shell_delega...
File ash/shell/shell_delegate_impl.h (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/shell/shell_delega...
ash/shell/shell_delegate_impl.h:57: void SetTouchscreenEnabledInPrefs()
override;
this won't compile, will it? it looks like you're missing the arguments here.

(which i guess raises the question of where this code is used... :-P)

https://codereview.chromium.org/2533373002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:254: const bool
touch_pad_status =
nit: s/touch_pad/touchpad/

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
chrome/common/pref_names.cc:848: const char kTouchscreenEnabled[] =
"events.touchscreen.enabled";
thanks for updating the constant names! i don't think you want to update the
values, though, as that will make users lose their existing settings.

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1885: const char kLocalTouchscreenEnabled[] =
"events.local_touchscreen.enabled";
i don't know the usual pattern here. do we usually introduce new prefs with
"local_" in the name, or should the local state just use kTouchscreenEnabled as
well?

[Qiang(Joe) Xu, 2016-12-03 00:58:31]

Patchset #8 (id:240001) has been deleted

[Qiang(Joe) Xu, 2016-12-03 01:01:17]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[Qiang(Joe) Xu, 2016-12-03 01:02:08]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by this rule:
user pref on, local pref on -> touchscreen on
user pref off, local pref on -> touchscreen off
user pref on, local pref off -> touchscreen off
user pref off, local pref off -> touchscreen on
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by the rule:
enabled_in_local_state && enabled_in_user_pref
==========

[commit-bot: I haz the power, 2016-12-03 01:02:22]

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

[Qiang(Joe) Xu, 2016-12-03 01:02:44]

Hi, here is the reply and new patch, thanks!

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:137: // prefs, otherwise from user prefs.
On 2016/12/03 00:08:44, Daniel Erat wrote:
> nit: s/local prefs/local state/

Done.

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:141: // pref is set in local pref, otherwise in user
pref.
On 2016/12/03 00:08:44, Daniel Erat wrote:
> nit: s/local pref/local state/, s/user pref/user prefs/

Done.

https://codereview.chromium.org/2533373002/diff/220001/ash/common/shell_deleg...
ash/common/shell_delegate.h:145: // Updates the on/off status of the touchscreen
from the combination of local
On 2016/12/03 00:08:44, Daniel Erat wrote:
> nit: s/local/local state/
> 
> i think it'd probably be worthwhile to also document in this comment how the
two
> possible values interact (i think that both need to be set to true, right?).
not
> having this documented in the interface is a bit confusing, since one needs to
> look at all of the implementations to figure out what the desired behavior is.

Done.

https://codereview.chromium.org/2533373002/diff/220001/ash/mus/shell_delegate...
File ash/mus/shell_delegate_mus.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/mus/shell_delegate...
ash/mus/shell_delegate_mus.cc:235: NOTIMPLEMENTED();
On 2016/12/03 00:08:44, Daniel Erat wrote:
> just to double-check, are these going to be implemented later? if so,
> NOTIMPLEMENTED() seems correct.

I think yes. AFAIK, chromeos UI will move to mus-ash architecture. So this
should be implemented in the future.

https://codereview.chromium.org/2533373002/diff/220001/ash/shell/shell_delega...
File ash/shell/shell_delegate_impl.h (right):

https://codereview.chromium.org/2533373002/diff/220001/ash/shell/shell_delega...
ash/shell/shell_delegate_impl.h:57: void SetTouchscreenEnabledInPrefs()
override;
On 2016/12/03 00:08:44, Daniel Erat wrote:
> this won't compile, will it? it looks like you're missing the arguments here.
> 
> (which i guess raises the question of where this code is used... :-P)

oops, I missed here. Compiler for chromeos and linux target cannot find this. It
is initialized in shell_browser_main_parts only. There aren't many docs there. I
am still not sure of this...

https://codereview.chromium.org/2533373002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.cc:254: const bool
touch_pad_status =
On 2016/12/03 00:08:44, Daniel Erat wrote:
> nit: s/touch_pad/touchpad/

Done.

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
chrome/common/pref_names.cc:848: const char kTouchscreenEnabled[] =
"events.touchscreen.enabled";
On 2016/12/03 00:08:44, Daniel Erat wrote:
> thanks for updating the constant names! i don't think you want to update the
> values, though, as that will make users lose their existing settings.

Done.

https://codereview.chromium.org/2533373002/diff/220001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1885: const char kLocalTouchscreenEnabled[] =
"events.local_touchscreen.enabled";
On 2016/12/03 00:08:44, Daniel Erat wrote:
> i don't know the usual pattern here. do we usually introduce new prefs with
> "local_" in the name, or should the local state just use kTouchscreenEnabled
as
> well?

I think they can use the same value. gab@ may have an idea here.

[commit-bot: I haz the power, 2016-12-03 01:41:35]

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

[commit-bot: I haz the power, 2016-12-03 01:41:36]

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_...)

[Qiang(Joe) Xu, 2016-12-03 02:35:38]

The CQ bit was checked by warx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-03 02:36:03]

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

[Daniel Erat, 2016-12-03 03:11:55]

lgtm w/nits but i'd still prefer reusing the same pref constant for local state
if possible

https://codereview.chromium.org/2533373002/diff/280001/ash/common/shell_deleg...
File ash/common/shell_delegate.h (right):

https://codereview.chromium.org/2533373002/diff/280001/ash/common/shell_deleg...
ash/common/shell_delegate.h:149: // Toggles the status of the touchpad enabled
or disabled.
nit: "... status of the touchpad between enabled and disabled."?

https://codereview.chromium.org/2533373002/diff/280001/chrome/browser/chromeo...
File chrome/browser/chromeos/system/input_device_settings.h (right):

https://codereview.chromium.org/2533373002/diff/280001/chrome/browser/chromeo...
chrome/browser/chromeos/system/input_device_settings.h:199: // Toggles the
status of touchpad enabled or disabled and updates the user
nit: "... between enabled and disabled"

[commit-bot: I haz the power, 2016-12-03 03:50:53]

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

[commit-bot: I haz the power, 2016-12-03 03:50:54]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-12-03 21:52:16]

The CQ bit was checked by warx@chromium.org

[Qiang(Joe) Xu, 2016-12-03 21:52:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
achuith@chromium.org, gab@chromium.org, derat@chromium.org
Link to the patchset: https://codereview.chromium.org/2533373002/#ps300001
(title: "nits")

[commit-bot: I haz the power, 2016-12-03 21:52:29]

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-12-03 22:38:53]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1480801936196280,
"parent_rev": "529e0555536973dc7f220d157afdc44813655dd4", "commit_rev":
"ce45ef724984f43136a4e955314aa1bed65307e7"}

[commit-bot: I haz the power, 2016-12-03 22:39:18]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by the rule:
enabled_in_local_state && enabled_in_user_pref
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by the rule:
enabled_in_local_state && enabled_in_user_pref
==========

[commit-bot: I haz the power, 2016-12-03 22:39:19]

Committed patchset #10 (id:300001)

[commit-bot: I haz the power, 2016-12-03 22:40:59]

Description was changed from

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by the rule:
enabled_in_local_state && enabled_in_user_pref
==========

to

==========
Enabled/disable touch screen in TabletPowerButtonController

By exposing several apis in input_device_settings.h and add new local pref for
TouchScreenEnabled, we can disable touch screen events when backlights are
forced off so that no screen UI events can be triggered and enable touch screen
when stopping backlights forced off is called properly.

BUG=655388
BUG=633304
TEST=manual test on device, and add test coverage.
also test that this added behavior is compatible with toggle touchscreen
shortcut
The shortcut induced in https://codereview.chromium.org/2467023004 is per user
pref, backlights-forced-off related touch screen status is per device pref. The
final hardware touchscreen status is determined by the rule:
enabled_in_local_state && enabled_in_user_pref

Committed: https://crrev.com/4c8baedbf7a0b83ec7b5fd77dc1f5d5627fe6dd7
Cr-Commit-Position: refs/heads/master@{#436177}
==========

[commit-bot: I haz the power, 2016-12-03 22:41:00]

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