Thanks Scott. Seems this is easy enough to break that we should get a test added around this. If no other testing path is obvious we could add a 10s media clip and force battery power and check that battery watch time is reported.

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The above CL landed in 59.0.3066.0

A friendly reminder that M59 Beta  launch is coming soon! Your bug is labelled as Release Block Beta. All fixes need to be merged into the release branch (3071) latest by tomorrow, 04/26 4:00 PM PT in order to make into the desktop Beta final build cut. Thank you!

I spoke with scottmg@ and we don't have to gate beta on this, stable is sufficient.  That said - we should fix this and merge it to the beta branch ASAP, as power regressions suck.

Adding benhenry@ as an FYI in case this is something we can monitor in the future.

Do we need this in M59? It landed right around the branch point, but is large so maybe a revert won't go cleanly.

I think until we know what's wrong it's hard to say if this is limited to just a power regression or if other important messages (as the bug title indicates) are being dropped.

I'll look at this.

This issue is already fixed on trunk, it turns out (although I don't know if the fix was made with awareness of the issue). The issue is that https://codereview.chromium.org/2795883002 moved services away from implementing OnConnect() but left several clients still relying on the relevant service implementing OnConnect() (the default impl is do-nothing). One of those clients was ChildThreadImpl, which connected to the Device Service via connector->Connect("services:device")->GetRemoteInterfaces() in order to then connect to the PowerMonitor. Since the Device Service no longer implemented OnConnect() as of the CL in question, the InterfaceProvider returned from that client-side invocation was empty.

This CL fixed the problem as part of eliminating OnConnect() entirely: https://codereview.chromium.org/2804373002. The earlier CL had left other similar problematic client-side invocations, but all of them were fixed in that followup CL as part of completely eliminating the client-side usage of Connector::Connect().

Totally agreed about an end-to-end test. Filed  crbug.com/715985 , which the team doing the Device Service will own.

On second thought, moving to fixed so that QA can verify that the problematic behavior described in the OP is no longer reproducible on trunk (I don't have a Windows laptop to build on -- I did as complete testing as I could on Linux to verify my reasoning, but couldn't do the full end-to-end verification there).

[Auto-generated comment by a script] We noticed that this issue is targeted for M-59; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-59 label, otherwise remove Merge-TBD label. Thanks.

Why do you think this should not be merged?

The CL with the fix is 464167, whereas 59 was cut at 464641.

Although that raises a different and interesting question. 464167 landed on April 12, whereas this bug report is from April 21. scottmg@, did you do the testing in the OP on a build that included 464167? If so, it would seem that my reasoning isn't actually sufficient (the testing that I was able to do on Linux and Android indicates that at least on those platforms, 464167 restored the connection that had been broken in https://codereview.chromium.org/2795883002).

blergh.

I did the continuous bisect on the small range in 59.0.3065.0..59.0.3067.0 to identify the individual CL, as we'd already narrowed it down to that range.

So I don't think there's anything to be merged as the breakage was 59.0.3066.0, the fix was 59.0.3070.0, and the branch was 59.0.3071.0.

But we should still get QA to confirm.