Closed Bug 1369194 Opened 2 years ago Closed 2 years ago

Remove navigator.requestWakeLock()

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: gsvelto, Assigned: agashlin)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

+++ 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.
No longer depends on: 1306447
Thanks Kris, sorry for the bug spam.
Keywords: meta
Priority: -- → P3
Adding ddn, just so I can check the docs for this.
Keywords: dev-doc-needed
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.
(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.
Sorry, I got this mixed up with the `navigator.getWakeLock` API from https://w3c.github.io/wake-lock/. Thanks for clearing things up, cheers!
Depends on: 1416078
Blocks: 1416078
No longer depends on: 1416078
Assignee: nobody → agashlin
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.
Attachment #8930219 - Flags: review?(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...).
Attachment #8930219 - Flags: review?(gsvelto) → review+
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.
Attachment #8930308 - Flags: review?(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.
Attachment #8930308 - Flags: review?(gsvelto) → review?(amarchesini)
Attachment #8930308 - Flags: review?(amarchesini) → review+
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/951c0a271074
https://hg.mozilla.org/mozilla-central/rev/7b021ff1814f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.