1369194 - Remove navigator.requestWakeLock()

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

+++ This bug was initially created as a clone of Bug #1306391 +++

navigator.requestWakeLock() was introduced in B2G to allow system code to grab various Android wakelocks. Since it's not used anymore it should be removed together with related code in the wakelocks implementation.

Comment 1

[Gabriele Svelto [:gsvelto]]

Thanks Kris, sorry for the bug spam.

Comment 2

[Chris Mills [:cmills]]

Adding ddn, just so I can check the docs for this.

Comment 3

[Beth Rennie [:beth] (she/her)]

Is this a proposal to remove the `navigator.requestWakeLock` API that is provided by the pref `dom.wakelock.enabled` in Fx? Because there is another bug open (https://bugzilla.mozilla.org/show_bug.cgi?id=1054113) to enable it for Firefox for Android. I don't know enough about how much code is shared between B2G and Fx.

Comment 4

[Gabriele Svelto [:gsvelto]]

(In reply to Barret Rennie [:brennie] from comment #3)
> Is this a proposal to remove the `navigator.requestWakeLock` API that is
> provided by the pref `dom.wakelock.enabled` in Fx? Because there is another
> bug open (https://bugzilla.mozilla.org/show_bug.cgi?id=1054113) to enable it
> for Firefox for Android. I don't know enough about how much code is shared
> between B2G and Fx.

IIRC this API is non-standard so it shouldn't be enabled, I think it's safe if we close that bug as WONTFIX.

Comment 5

[Beth Rennie [:beth] (she/her)]

Sorry, I got this mixed up with the `navigator.getWakeLock` API from https://w3c.github.io/wake-lock/. Thanks for clearing things up, cheers!

Comment 6

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 1. remove navigator.requestWaitLock()

This is a straightforward removal of the interface. I don't touch MozWakeLock in this patch because I'm not sure if I can safely remove it without disturbing the other users of WakeLock.

Comment 7

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8930219 [details] [diff] [review]
1. remove navigator.requestWaitLock()

LGTM. There seems to be no more to remove since the remaining bits of implementation are still in use in the various widgets (e.g. to keep the screen on if the user is watching a video, etc...).

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 2. remove MozWaitLock

The only place where MozWaitLock was used was the return value of navigator.requestWaitLock, so this patch removes it. I also removed WaitLock from the cycle collector since it holds nothing that needs to be collected now that it doesn't need to be wrapped for JS.

I'm unfamiliar with these mechanisms so hopefully I've done it right, wake lock functionality still seems to behave correctly.

Comment 9

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8930308 [details] [diff] [review]
2. remove MozWaitLock

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

I don't feel confident in reviewing changes that touch cycle-collected stuff myself since I've had only little exposure to it. I'm redirecting the review to a DOM peer.

Comment 10

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Try looks good to me, requesting checkin.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=580b4a4541a3634831103c92b3cf5461e54a064c

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/951c0a271074
Remove navigator.requestWakeLock(). r=gsvelto, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b021ff1814f
Remove MozWakeLock. r=baku

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/951c0a271074
https://hg.mozilla.org/mozilla-central/rev/7b021ff1814f