Reland "Replace Chrome IPC with Mojo IPC for querying BatteryStatus service"

Reland  "Replace Chrome IPC with Mojo IPC for querying BatteryStatus service"

The original CL https://codereview.chromium.org/592153002 was reverted as it broke the battery status layout tests.

BUG=420623
Committed: https://crrev.com/2438a3e2acb1318a4e59c78764b532bac3649161
Cr-Commit-Position: refs/heads/master@{#301719}

[ppi, 2014-10-28 15:59:41]

ppi@chromium.org changed reviewers:
+ timvolodine@chromium.org

[ppi, 2014-10-28 16:09:44]

Hi Tim,

ps1 is what was originally reviewed and landed.

ps2 restores plumbing for layout tests - does this approach look good to you?

Ptal.

[timvolodine, 2014-10-28 18:59:28]

I understand the only change is in
content/renderer/renderer_blink_platform_impl.cc. lgtm, a couple of comments
below:

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:156:
blink::WebBatteryStatusListener* g_test_battery_status_listener = nullptr;
i think this is approximately how it used to be implemented before
https://codereview.chromium.org/446603002/.

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:1087: if (type ==
blink::WebPlatformEventBattery) {
this doesn't look like it will generalize very well, but for now I think it's
fine because other API are not using mojo (yet).

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:1192: *static_cast<const
blink::WebBatteryStatus*>(&status));
do you really need a static_cast here?

[ppi, 2014-10-28 19:16:03]

ppi@chromium.org changed reviewers:
+ jam@chromium.org, rockot@chromium.org, tsepez@chromium.org

[ppi, 2014-10-28 19:24:33]

ppi@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[ppi, 2014-10-28 19:24:34]

Thanks!

Adding the other reviewers. This is mostly the same as the original patch
(uploaded as patch set 1). Please take a look or stamp based on Tom's review.

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:156:
blink::WebBatteryStatusListener* g_test_battery_status_listener = nullptr;
On 2014/10/28 18:59:28, timvolodine wrote:
> i think this is approximately how it used to be implemented before
> https://codereview.chromium.org/446603002/.

Acknowledged.

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:1087: if (type ==
blink::WebPlatformEventBattery) {
On 2014/10/28 18:59:28, timvolodine wrote:
> this doesn't look like it will generalize very well, but for now I think it's
> fine because other API are not using mojo (yet).

Ack. Let's figure something more general out once we need to add a second case
like this one.

https://codereview.chromium.org/685703002/diff/20001/content/renderer/rendere...
content/renderer/renderer_blink_platform_impl.cc:1192: *static_cast<const
blink::WebBatteryStatus*>(&status));
On 2014/10/28 18:59:28, timvolodine wrote:
> do you really need a static_cast here?

Done.

[Tom Sepez, 2014-10-28 19:30:12]

Rubberstamp LGTM from previous patch.

[darin (slow to review), 2014-10-28 20:11:17]

darin@chromium.org changed reviewers:
+ darin@chromium.org

[darin (slow to review), 2014-10-28 20:11:18]

Rubber stamp LGTM

[Ken Rockot(use gerrit already), 2014-10-28 20:25:40]

lgtm

[ppi, 2014-10-28 21:04:31]

The CQ bit was checked by ppi@chromium.org

[commit-bot: I haz the power, 2014-10-28 21:06:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/685703002/60001

[commit-bot: I haz the power, 2014-10-28 21:55:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-28 21:56:06]

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