-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WakeLock and only require activation once #139
Comments
This is related to #138. |
My assumption would be that any kind of wake lock ( |
I agree (with maybe a bit of thought needed about "screen"), but the specs don't say that. |
I am assuming Page Visibility is a sub topic of Page Lifecycle. One thing which spec should clarify is what would Wakelock's 'active' attribute reflect when page is not visible/backgrounded. Should there be another attribute like 'frozen'. For example in case of screen wake when tab is not visible active would be false but frozen will be true whereas in case of system wake lock frozen would be true but active would also be true. This way that tab or app using wake lock will be aware of Wakelock's real status. |
While implementing against the newest API, noticed that in order to maintain 'screen' wake lock across page visibility changes, wake lock must be reacquired every time the document becomes visible during 'visibilitychange' events. I think it's at least worth calling out that anyone who wants to use 'screen' wake lock will likely have to do the same thing. It isn't a lot of extra code or state to manage, but might be worth putting this into the API if it is common enough. |
@cterefinko I just re-read the updated spec and stumbled upon this note (green box at the end of the algorithm): "The additional visibility requirement for screen wake lock is to prevent keeping the screen on when the page is not visible, such as when the browser is minimized. As this condition is transient and not under control of the web page, the specification chooses to automatically acquire and release the lock when visibility changes rather than having the page to deal with it, e.g[.] by re-requesting the lock each time the page becomes visible again." |
I've done some digging, and I think that note was part of a previous version of the spec that no longer the rest of the text: it was added in #88 back in 2016, but #153 made it so that when a page becomes hidden all screen locks are automatically released and not re-acquired when visibility changes. The question is whether we want to revert to the previous behavior. |
One could probably argue for reverting to the previous behavior. I played with the new implementation earlier today, and it is a little annoying that one has to re-acquire the lock upon each visibility change. The only (unfortunately pretty dangerous) foot gun I see is people forgetting about the screen lock in absence of a visual indicator. So I think for now at least we are on the safe side when we require people to re-acquire the lock every time. |
We could make it an option instead, like autoReacquire: true or similar |
Thanks for the investigations! I understand the concerns about forgetting about wake lock so making it optional seems reasonable. In practice, not sure what the use-case is for not wanting re-acquiring, but it isn't too hard to work around if we want to keep playing it safe. |
As it's easy to work around, and as we acknowledge the foot gun, we can definitely start the OT without re-acquiring, and then see what the OT feedback is. @kenchris' proposal is also future proof, we could add the option later and not break any code (which with an OT is not a concern, but still nice to have). |
This note was added back in commit 16cd4c7 ("Spec update"), but commit 34119d2 ("Handle document visibility") changed the spec in a way that contradicts the note: we no longer handle visibility changes automatically, and leave it up to script authors to do that instead. Whether this is a good idea or at least be made optional is still being discussed; in the meantime, remove the note since it does not reflect reality. Related to w3c#139
This note was added back in commit 16cd4c7 ("Spec update"), but commit 34119d2 ("Handle document visibility") changed the spec in a way that contradicts the note: we no longer handle visibility changes automatically, and leave it up to script authors to do that instead. Whether this is a good idea or at least be made optional is still being discussed; in the meantime, remove the note since it does not reflect reality. Related to #139
Discussed at TPAC 2019 F2F. Decided to keep the internal state of the API simple, tab change does release the lock. The necessary code for sites is minimal. Leaving this issue open to track adding informative prose to the spec with example of how to reacquire a lock on page life cycle transitions. |
Would this be the same as the document becoming non-fully active? I think the "handling document loss of full activity" might be wrong at the moment... paraphrasing the spec:
Shouldn't step 3 wait for the document to become fully active again before firing the event? (also, fire the event should probably be queued... we might want to separate "release the lock" from the actual "release" event being fired). CC @rakuco. |
It's not entirely clear to be, to be honest. The definition of "fully active document" in HTML isn't very easy to understand, and the Page Lifecycle spec does not seem to explain how well it interacts with that concept, but adds a few other ones ("frozen" and "discarded") that seem to precede loss of full activity. OTOH, there doesn't seem to be an explicit hook in HTML for "run these algorithms when the document is no longer fully active", so in Chromium we just run it in In addition to WICG/page-lifecycle#31, WICG/page-lifecycle#7 also mentions Wake Lock integration, as it's not entirely clear to me how page visibility, page lifecycle and wake locks are supposed to interact (and the order that's supposed to happen).
Is it possible for a document to be come fully active again (emphasis on again)? My understanding is that the document is no longer fully active when the browsing context is destroyed, in which case there'd be no state to go back to in the first place. There's also the bfcache case which is kind of related to the page visibility + page lifecycle + wake lock integration I mentioned above. I remember there was a discussion in Chromium a while ago about whether pages holding wake locks would be eligible for bfcache, but in the end the locks are always supposed to be released when the page becomes hidden and before the bfcache checks anyway.
Well, this was what prompted the long discussions in #293 and #299 in the first place :-) The conclusion back then was that there was no benefit in queuing rather than firing the event directly. |
If a document holding a becomes non-fully active and then becomes fully active then I think a |
Could you provide an example of how this usually happens, or a way to trigger this in code? Testing non-fully active documents in WPT hasn't been very easy in my experience, and it's not clear to me how to get the same document back to an active state. |
I believe the existing logic for handling page visibility would apply, so it's likely we don't need to do anything. But @rakuco and I will recheck and a test case if possible. Let's find a time to chat. |
@rakuco, I think we can close this, right? |
From @jyasskin:
The text was updated successfully, but these errors were encountered: