1382955 - Remove MozPowerManager and related things

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Nicholas Nethercote [inactive]]

In bug 1382099 gsvelto noticed that the MozPowerManager is no longer necessary.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove MozPowerManager and related things

gsvelto, I've taken a crack at this. I don't really know what I'm doing, I just
started trimming and didn't stop until it seemed complete. Is modifying the
.webidl files ok?

Comment 2

[Gabriele Svelto [:gsvelto]]

(In reply to Nicholas Nethercote [:njn] from comment #1)
> Created attachment 8888681 [details] [diff] [review]
> Remove MozPowerManager and related things
> 
> gsvelto, I've taken a crack at this. I don't really know what I'm doing, I
> just
> started trimming and didn't stop until it seemed complete. Is modifying the
> .webidl files ok?

Just had a quick look since I'm still on paternal leave, I'll dig deeper early next month when I'm back full time. I think more stuff can be trimmed from HAL but I'll have to check it. Also I need to check that Fennec isn't using wakelocks because IIRC it was at some point so we might have to keep those around. BTW see also bug 1369194 for something somewhat related.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Remove a bunch of unused HAL stuff

kanru, please review the sync-messages.ini changes, which require an IPC peer.

gsvelto, please review the rest.

Comment 4

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8889288 [details] [diff] [review]
Remove a bunch of unused HAL stuff

Review of attachment 8889288 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for the IPC change.

This patch is not complete. I think you will also remove all the users of the removed API?

Comment 5

[Nicholas Nethercote [inactive]]

> This patch is not complete. I think you will also remove all the users of
> the removed API?

I'm not sure if I understand your comment. There are no users of these APIs, which is why I was able to remove them. AS far as I can tell the patch is complete. (It builds on all platforms on try.)

Comment 6

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Nicholas Nethercote [:njn] from comment #5)
> > This patch is not complete. I think you will also remove all the users of
> > the removed API?
> 
> I'm not sure if I understand your comment. There are no users of these APIs,
> which is why I was able to remove them. AS far as I can tell the patch is
> complete. (It builds on all platforms on try.)

You're right. I missed your first patch :)

Comment 7

[Gabriele Svelto [:gsvelto]]

Back from parental leave... I'll review this tomorrow.

Comment 8

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8888681 [details] [diff] [review]
Remove MozPowerManager and related things

Almost there but not quite; after these changes the power manager service has three methods that are now unused: restart(), reboot() and powerOff(), see [1]. The latter are implemented via hal::Reboot() and hal::PowerOff() which can also be removed and the former was a "special" restart function only ever used in gonk so it can also go.

[1] https://dxr.mozilla.org/mozilla-central/rev/36f95aeb4c77f7cf3b3366583008cd6e4b6b1dba/dom/power/nsIPowerManagerService.idl#25

Comment 9

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8889288 [details] [diff] [review]
Remove a bunch of unused HAL stuff

Clearing the review flag as more changes are required as per the comment above.

Comment 10

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove MozPowerManager and related things

This is the same as the previous version, except for the commit log. I put the
nsIPowerManagerService changes in a separate patch.

Comment 11

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove nsIPowerManagerService::{powerOff,reboot,restart} and related things

Comment 12

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Remove a bunch of unused HAL stuff

Comment 13

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8892376 [details] [diff] [review]
(part 2) - Remove nsIPowerManagerService::{powerOff,reboot,restart} and related things

Review of attachment 8892376 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM save for the comment below. From what I can tell this should be all of it.

::: dom/ipc/ContentParent.cpp
@@ -680,5 @@
> -
> -  bool done = false;
> -  Monitor monitor("mozilla.dom.ContentParent.JoinAllSubprocesses");
> -  XRE_GetIOMessageLoop()->PostTask(NewRunnableFunction(
> -                                     &ContentParent::JoinProcessesIOThread,

ContentParent::JoinProcessesIOThread() seems unused now, let's remove that too.

Comment 14

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8892377 [details] [diff] [review]
(part 3) - Remove a bunch of unused HAL stuff

Review of attachment 8892377 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 15

[Gabriele Svelto [:gsvelto]]

I hadn't noticed but there's a build failure on Android with the latest patch set, it should be trivial to fix though:

https://treeherder.mozilla.org/logviewer.html#?job_id=119958778&repo=try&lineNumber=17115

Comment 16

[Nicholas Nethercote [inactive]]

Comment on attachment 8892375 [details] [diff] [review]
(part 1) - Remove MozPowerManager and related things

Review of attachment 8892375 [details] [diff] [review]:
-----------------------------------------------------------------

bz, the two .webidl files need review from a DOM peer. This is all b2g-only power stuff that is no longer needed.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8892375 [details] [diff] [review]
(part 1) - Remove MozPowerManager and related things

r=me

Comment 18

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/615e7773cd68625b1d8ce9c22df7186c7ae921ca
Bug 1382955 (part 1) - Remove MozPowerManager and related things. r=bz,gsvelto.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2345ddc27945fb93f71de800f0cf79184c3c1550
Bug 1382955 (part 2) - Remove nsIPowerManagerService::{powerOff,reboot,restart} and related things. r=gsvelto.

https://hg.mozilla.org/integration/mozilla-inbound/rev/df3915693fa3bec56bacc17a4f2588f99ab635b4
Bug 1382955 (part 3) - Remove a bunch of unused HAL stuff. r=gsvelto,kanru.

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/615e7773cd68
https://hg.mozilla.org/mozilla-central/rev/2345ddc27945
https://hg.mozilla.org/mozilla-central/rev/df3915693fa3