Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation

Committed: https://crrev.com/084d8f4d4b85f99634b2a05cbfe81282b7b15c29
Cr-Commit-Position: refs/heads/master@{#425182}

[Qiang(Joe) Xu, 2016-10-13 05:26:44]

Description was changed from

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation
==========

to

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation
==========

[Qiang(Joe) Xu, 2016-10-13 05:26:45]

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

[Qiang(Joe) Xu, 2016-10-13 16:01:41]

Hi Daniel, could you PTAL, thanks!

[Daniel Erat, 2016-10-13 17:25:19]

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/fake_power_ma...
File chromeos/dbus/fake_power_manager_client.cc (right):

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/fake_power_ma...
chromeos/dbus/fake_power_manager_client.cc:140: const
GetBacklightsForcedOffStateCallback& callback) {}
mind stubbing the implementation of this to make it easier to write tests for
the UI code that will call it? i think you just need to add a
backlights_forced_off_ bool member and then update it in SetBacklightsForcedOff
and do something like this here:

  base::ThreadTaskRunnerHandle::Get()->PostTask(
      FROM_HERE, base::Bind(callback, backlights_forced_off_));

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

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:253: writer.AppendBool(is_off);
nit: this can be shortened to:

  dbus::MessageWriter(&method_call).AppendBool(forced_off);

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:510: if (!reader.PopBool(&state))
nit: multi-line if bodies need curly brackets

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

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.h:191: virtual void
SetBacklightsForcedOff(bool is_off) = 0;
nit: "forced_off" might be a tiny bit clearer than "is_off"

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.h:194: virtual void
GetBacklightsForcedOffState(
any reason not to call this (and its callback) just "GetBacklightsForcedOff" to
match the d-bus method?

[Qiang(Joe) Xu, 2016-10-13 21:19:41]

Hi Daniel, updates uploaded. PTAL. Thanks!

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/fake_power_ma...
File chromeos/dbus/fake_power_manager_client.cc (right):

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/fake_power_ma...
chromeos/dbus/fake_power_manager_client.cc:140: const
GetBacklightsForcedOffStateCallback& callback) {}
On 2016/10/13 17:25:19, Daniel Erat wrote:
> mind stubbing the implementation of this to make it easier to write tests for
> the UI code that will call it? i think you just need to add a
> backlights_forced_off_ bool member and then update it in
SetBacklightsForcedOff
> and do something like this here:
> 
>   base::ThreadTaskRunnerHandle::Get()->PostTask(
>       FROM_HERE, base::Bind(callback, backlights_forced_off_));

Done.

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

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:253: writer.AppendBool(is_off);
On 2016/10/13 17:25:19, Daniel Erat wrote:
> nit: this can be shortened to:
> 
>   dbus::MessageWriter(&method_call).AppendBool(forced_off);

Done.

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.cc:510: if (!reader.PopBool(&state))
On 2016/10/13 17:25:19, Daniel Erat wrote:
> nit: multi-line if bodies need curly brackets

Done.

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

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.h:191: virtual void
SetBacklightsForcedOff(bool is_off) = 0;
On 2016/10/13 17:25:19, Daniel Erat wrote:
> nit: "forced_off" might be a tiny bit clearer than "is_off"

Done.

https://codereview.chromium.org/2409983006/diff/1/chromeos/dbus/power_manager...
chromeos/dbus/power_manager_client.h:194: virtual void
GetBacklightsForcedOffState(
On 2016/10/13 17:25:19, Daniel Erat wrote:
> any reason not to call this (and its callback) just "GetBacklightsForcedOff"
to
> match the d-bus method?

Changed to match dbus method.

[Daniel Erat, 2016-10-13 21:37:35]

lgtm

https://codereview.chromium.org/2409983006/diff/20001/chromeos/dbus/power_man...
File chromeos/dbus/power_manager_client.cc (right):

https://codereview.chromium.org/2409983006/diff/20001/chromeos/dbus/power_man...
chromeos/dbus/power_manager_client.cc:493: if (!reader.PopDouble(&percent)) {
ah, thanks for fixing this too :-)

[Qiang(Joe) Xu, 2016-10-13 21:38:40]

The CQ bit was checked by warx@chromium.org

[commit-bot: I haz the power, 2016-10-13 21:39:08]

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-10-13 22:12:26]

Description was changed from

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation
==========

to

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation
==========

[commit-bot: I haz the power, 2016-10-13 22:12:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-13 22:14:11]

Description was changed from

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation
==========

to

==========
Make [S,G]getBacklightsForcedOff dbus API integrated to power_manager_client

BUG=633304
TEST=test the API which is called in the following implementation

Committed: https://crrev.com/084d8f4d4b85f99634b2a05cbfe81282b7b15c29
Cr-Commit-Position: refs/heads/master@{#425182}
==========

[commit-bot: I haz the power, 2016-10-13 22:14:11]

Patchset 2 (id:??) landed as
https://crrev.com/084d8f4d4b85f99634b2a05cbfe81282b7b15c29
Cr-Commit-Position: refs/heads/master@{#425182}