chromeos: Add PowerManagerClient::GetSwitchStates().

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}
Committed: https://chromium.googlesource.com/chromium/src/+/08e333dbc8949a69fc48129055683477017bf5d8

[Daniel Erat, 2017-03-21 16:14:16]

derat@chromium.org changed reviewers:
+ jonross@chromium.org

[Daniel Erat, 2017-03-21 16:14:17]

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:565: return;
i'm following this class's existing practice of apparently dropping callbacks on
the floor when d-bus calls to powerd fail, but i'm not sure if this will cause
any problems for you. in practice, it can probably only happen at startup if
chrome somehow manages to start faster than powerd.

[jonross, 2017-03-21 16:58:49]

On 2017/03/21 16:14:17, Daniel Erat wrote:
>
https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
> File chromeos/dbus/power_manager_client.cc (right):
> 
>
https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
> chromeos/dbus/power_manager_client.cc:565: return;
> i'm following this class's existing practice of apparently dropping callbacks
on
> the floor when d-bus calls to powerd fail, but i'm not sure if this will cause
> any problems for you. in practice, it can probably only happen at startup if
> chrome somehow manages to start faster than powerd.

LGTM, but consider adding a small doc (see comment)

[jonross, 2017-03-21 17:01:57]

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:302: void GetSwitchStates(const
GetSwitchStatesCallback& callback) override {
And I can just call this to poll. Sweet thanks!

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:565: return;
On 2017/03/21 16:14:16, Daniel Erat wrote:
> i'm following this class's existing practice of apparently dropping callbacks
on
> the floor when d-bus calls to powerd fail, but i'm not sure if this will cause
> any problems for you. in practice, it can probably only happen at startup if
> chrome somehow manages to start faster than powerd.

If this is only expected when chrome is faster than powerd, this is fine. As
powerd setting up after will notify us through InputEventReceived.

In my particular case the devices always have powerd running first, so this is
unlikely to come up. But good to know about. Maybe put a small doc about this?

[Daniel Erat, 2017-03-21 17:15:02]

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:565: return;
On 2017/03/21 17:01:57, jonross wrote:
> On 2017/03/21 16:14:16, Daniel Erat wrote:
> > i'm following this class's existing practice of apparently dropping
callbacks
> on
> > the floor when d-bus calls to powerd fail, but i'm not sure if this will
cause
> > any problems for you. in practice, it can probably only happen at startup if
> > chrome somehow manages to start faster than powerd.
> 
> If this is only expected when chrome is faster than powerd, this is fine. As
> powerd setting up after will notify us through InputEventReceived.
> 
> In my particular case the devices always have powerd running first, so this is
> unlikely to come up. But good to know about. Maybe put a small doc about this?

sure, makes sense. i've added comments in the header for all of the methods that
i think exhibit this pattern.

[Daniel Erat, 2017-03-21 17:15:14]

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

[jonross, 2017-03-21 17:16:25]

On 2017/03/21 17:15:02, Daniel Erat wrote:
>
https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
> File chromeos/dbus/power_manager_client.cc (right):
> 
>
https://codereview.chromium.org/2768543002/diff/1/chromeos/dbus/power_manager...
> chromeos/dbus/power_manager_client.cc:565: return;
> On 2017/03/21 17:01:57, jonross wrote:
> > On 2017/03/21 16:14:16, Daniel Erat wrote:
> > > i'm following this class's existing practice of apparently dropping
> callbacks
> > on
> > > the floor when d-bus calls to powerd fail, but i'm not sure if this will
> cause
> > > any problems for you. in practice, it can probably only happen at startup
if
> > > chrome somehow manages to start faster than powerd.
> > 
> > If this is only expected when chrome is faster than powerd, this is fine. As
> > powerd setting up after will notify us through InputEventReceived.
> > 
> > In my particular case the devices always have powerd running first, so this
is
> > unlikely to come up. But good to know about. Maybe put a small doc about
this?
> 
> sure, makes sense. i've added comments in the header for all of the methods
that
> i think exhibit this pattern.

Awesome thanks!

[commit-bot: I haz the power, 2017-03-21 17:16:25]

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, 2017-03-21 18:46:42]

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

[commit-bot: I haz the power, 2017-03-21 18:46:43]

Dry run: This issue passed the CQ dry run.

[Daniel Erat, 2017-03-21 21:30:56]

i just hacked up a local build of chrome to call this on startup and verified
that it works end-to-end on a device.

[Daniel Erat, 2017-03-21 21:31:03]

The CQ bit was checked by derat@chromium.org

[Daniel Erat, 2017-03-21 21:31:03]

The patchset sent to the CQ was uploaded after l-g-t-m from jonross@chromium.org
Link to the patchset: https://codereview.chromium.org/2768543002/#ps20001
(title: "document that callbacks aren't run on error")

[commit-bot: I haz the power, 2017-03-21 21:31:34]

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, 2017-03-21 21:51:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490131863432220,
"parent_rev": "64fc03ab5fd993cc6573fa24266d177aaf8b1679", "commit_rev":
"08e333dbc8949a69fc48129055683477017bf5d8"}

[commit-bot: I haz the power, 2017-03-21 21:52:09]

Description was changed from

==========
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
==========

to

==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/08e333dbc8949a69fc4812905568...
==========

[commit-bot: I haz the power, 2017-03-21 21:52:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/08e333dbc8949a69fc4812905568...