New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 759846 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Remove TouchScreenEnabled local state

Project Member Reported by warx@chromium.org, Aug 28 2017

Issue description

Induced from: https://codereview.chromium.org/2533373002/

In that cl:

We use a local state for TouchScreenEnabled used by tablet power button. It is persistent for each reboot, and gets initialized/updated within TabletPowerButtonController. This dependency is proved to be fragile now.

Let us make this change:
(1) remove this local state
(2) make touchscreen state dependent on powerd's OnGetBacklightsForcedOff, instead of TabletPowerButtonController's OnGetBacklightsForcedOff.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50c6f8cbfa7274552b55759683145e284197cd64

commit 50c6f8cbfa7274552b55759683145e284197cd64
Author: Qiang Xu <warx@chromium.org>
Date: Thu Aug 31 20:55:24 2017

cros: remove TouchscreenEnabled local state

changes:
- Remove TouchscreenEnabledLocal
- Add ash::TouchscreenEnabledSource to replace bool parameter to
identify each enabled/disabled source.
- Instead, it becomes a member in chromeos::system::InputDeviceSettings
and gets initialized from powerd's backlights forced off state.
- Move instance of ash::PowerButtonDisplayController from
ash::TabletPowerButtonController to ash::PowerButtonController, so that
it is always constructed. Thus it removes the touchscreen enabled
status initialization dependency on ash::TabletPowerButtonController.
- modified test coverage to represent the change.

Test: covered by tests.
Bug:  759846 
Change-Id: I9d29afff10b916893410a213ae76bcc42caaddfb
Reviewed-on: https://chromium-review.googlesource.com/640551
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: Dan Erat (OOO Fri-Mon) <derat@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498992}
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/accelerators/debug_commands.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/public/cpp/BUILD.gn
[add] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/public/cpp/touchscreen_enabled_source.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/shell_delegate.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/shell_test_api.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/shell_test_api.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/system/power/power_button_display_controller.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/system/power/power_button_display_controller.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/system/power/tablet_power_button_controller.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/system/power/tablet_power_button_controller.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/system/power/tablet_power_button_controller_unittest.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/test_shell_delegate.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/test_shell_delegate.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/wm/power_button_controller.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/ash/wm/power_button_controller.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/chromeos/system/input_device_settings.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/chromeos/system/input_device_settings.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/common/pref_names.cc
[modify] https://crrev.com/50c6f8cbfa7274552b55759683145e284197cd64/chrome/common/pref_names.h

Comment 2 by warx@chromium.org, Aug 31 2017

Status: Fixed (was: Assigned)

Comment 3 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 4 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment