New issue
Advanced search Search tips

Issue 703362 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Participants' hotlists:
Fixing-touch


Sign in to add a comment

Add D-Bus method for querying lid and tablet-mode switch states

Project Member Reported by jonr...@chromium.org, Mar 20 2017

Issue description

Currently PowerManagerClient::InputEventReceived is notified of state changes.

Some signals such as power_manager::InputEvent_Type_TABLET_MODE_ON can now be set before Chrome has started.

This can lead to Chrome being out of sync with the state in powerd.

We need an ability to poll powerd at startup for state, so that the corresponding Chrome components can be updated.

Brought to light by b/36452425
 

Comment 1 by derat@chromium.org, Mar 20 2017

Components: OS>Kernel>Power
Summary: Add D-Bus method for querying lid and tablet-mode switch states (was: PowerManagerClient needs to be able to poll state at startup)

Comment 2 by derat@chromium.org, Mar 20 2017

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/47a097813e20193093ae5f2dfa8a4f9027129935

commit 47a097813e20193093ae5f2dfa8a4f9027129935
Author: Daniel Erat <derat@chromium.org>
Date: Tue Mar 21 02:30:15 2017

system_api: Add powerd GetSwitchStates method and proto.

Add a constant for a new GetSwitchStates powerd D-Bus method
and a corresponding SwitchStates protobuf definition to be
included in replies.

BUG= chromium:703362 
TEST=built it

Change-Id: I0cb2f6f72e62abaf64bb466b2fb3116c8487c3ce
Reviewed-on: https://chromium-review.googlesource.com/457261
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/47a097813e20193093ae5f2dfa8a4f9027129935/dbus/power_manager/dbus-constants.h
[modify] https://crrev.com/47a097813e20193093ae5f2dfa8a4f9027129935/system_api.gyp
[add] https://crrev.com/47a097813e20193093ae5f2dfa8a4f9027129935/dbus/power_manager/switch_states.proto

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2fbc25ae56c4fd8233dc2a8cbd471c88f6635777

commit 2fbc25ae56c4fd8233dc2a8cbd471c88f6635777
Author: Daniel Erat <derat@chromium.org>
Date: Tue Mar 21 06:12:03 2017

power: Add GetSwitchStates D-Bus method.

Add a GetSwitchStates D-Bus method that Chrome can call to
request the current tablet and lid switch states. Right now,
Chrome just emits InputEvent signals with the states at
startup, and Chrome doesn't have any way to determine the
initial states if misses them.

BUG= chromium:703362 
CQ-DEPEND=I0cb2f6f72e62abaf64bb466b2fb3116c8487c3ce
TEST=added unit test; also manually verified by running:
     dbus-send --system --dest=org.chromium.PowerManager \
       --print-reply --type=method_call \
       /org/chromium/PowerManager \
       org.chromium.PowerManager.GetSwitchStates
     on lumpy and seeing a response whose first field is "0"
     (LidState::OPEN) and second is "2"
     (TabletMode::UNSUPPORTED)

Change-Id: I2a40757615cdf8737cac4f34c986471d17a475f4
Reviewed-on: https://chromium-review.googlesource.com/457262
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/2fbc25ae56c4fd8233dc2a8cbd471c88f6635777/power_manager/powerd/daemon.cc
[modify] https://crrev.com/2fbc25ae56c4fd8233dc2a8cbd471c88f6635777/power_manager/powerd/daemon.h
[modify] https://crrev.com/2fbc25ae56c4fd8233dc2a8cbd471c88f6635777/power_manager/powerd/daemon_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 21 2017

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

commit 39242fdb96ac70d6530fd046e3b2abd98e7d66ad
Author: derat <derat@chromium.org>
Date: Tue Mar 21 16:27:33 2017

Update cros_system_api to 47a097813e201930.

Pull in a newer version of the Chrome OS system_api library
to get a new powerd constant and protocol buffer definition.

BUG= 703362 

Review-Url: https://codereview.chromium.org/2760333002
Cr-Commit-Position: refs/heads/master@{#458432}

[modify] https://crrev.com/39242fdb96ac70d6530fd046e3b2abd98e7d66ad/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 21 2017

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

commit 08e333dbc8949a69fc48129055683477017bf5d8
Author: derat <derat@chromium.org>
Date: Tue Mar 21 21:51:22 2017

chromeos: Add PowerManagerClient::GetSwitchStates().

Add a method to PowerManagerClient for asynchronously
requesting the current lid and tablet mode states from
powerd.

Also make a few PowerManagerClient::Observer methods receive
enums rather than bools and clean up FakePowerManagerClient
a bit.

BUG= 703362 

Review-Url: https://codereview.chromium.org/2768543002
Cr-Commit-Position: refs/heads/master@{#458566}

[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/ash/common/wm/maximize_mode/maximize_mode_controller.cc
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/ash/common/wm/maximize_mode/maximize_mode_controller.h
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/ash/wm/maximize_mode/maximize_mode_controller_unittest.cc
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chrome/browser/chromeos/login/lock/webui_screen_locker.cc
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chrome/browser/chromeos/login/lock/webui_screen_locker.h
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chromeos/BUILD.gn
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chromeos/dbus/fake_power_manager_client.cc
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chromeos/dbus/fake_power_manager_client.h
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chromeos/dbus/power_manager_client.cc
[modify] https://crrev.com/08e333dbc8949a69fc48129055683477017bf5d8/chromeos/dbus/power_manager_client.h

Comment 7 by derat@chromium.org, Mar 21 2017

Status: Fixed (was: Started)
Let me know if you run into any problems.

The PowerManagerClient interface is pretty awful and seems due for an overhaul. It might be nicer if it requested and cached things like this directly instead of making consumers request them themselves (although I've seen another line of reasoning that the client classes under //chromeos/dbus should just be extremely thin wrappers around D-Bus). I guess that that's work for another bug and another day, though...
Components: -UI>TouchView

Sign in to add a comment