Skip to content
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

Open
petele opened this issue Dec 13, 2018 · 18 comments
Open

WakeLock and only require activation once #139

petele opened this issue Dec 13, 2018 · 18 comments
Assignees

Comments

@petele
Copy link

petele commented Dec 13, 2018

From @jyasskin:

How does this interact with https://github.com/WICG/page-lifecycle, which pauses background pages even if the device as a whole is still running?

@jyasskin
Copy link
Member

This is related to #138.

@tomayac
Copy link
Contributor

tomayac commented Dec 14, 2018

My assumption would be that any kind of wake lock ("screen" or "system") would prevent the page from ever being forced to enter a frozen state. Wake locks should be "stronger" than system-initiated inventions from Page Lifecycle.

@jyasskin
Copy link
Member

jyasskin commented Dec 14, 2018

I agree (with maybe a bit of thought needed about "screen"), but the specs don't say that.

@mrunalk
Copy link

mrunalk commented Mar 21, 2019

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.

@cterefinko
Copy link

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.

@tomayac
Copy link
Contributor

tomayac commented Aug 6, 2019

@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."

@rakuco
Copy link
Member

rakuco commented Aug 6, 2019

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.

@tomayac
Copy link
Contributor

tomayac commented Aug 6, 2019

[B]ut #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.

@kenchris
Copy link
Contributor

kenchris commented Aug 6, 2019

We could make it an option instead, like autoReacquire: true or similar

@cterefinko
Copy link

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.

@tomayac
Copy link
Contributor

tomayac commented Aug 12, 2019

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).

rakuco added a commit to rakuco/wake-lock that referenced this issue Aug 22, 2019
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
kenchris pushed a commit that referenced this issue Aug 22, 2019
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
@reillyeon
Copy link
Member

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.

https://www.w3.org/2019/09/19-dap-minutes.html#x19

@marcoscaceres
Copy link
Member

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:

  1. if the document is no longer fully active, release the wake lock.
  2. Ask the OS to release the lock.
  3. 🧨 Fire an event named "release".

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.

@rakuco
Copy link
Member

rakuco commented Aug 20, 2021

Would this be the same as the document becoming non-fully active?

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 ContextDestroyed() which, AFAIU, runs when the browsing context is about to be destroyed.

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).

I think the "handling document loss of full activity" might be wrong at the moment... paraphrasing the spec:

  1. if the document is no longer fully active, release the wake lock.
  2. Ask the OS to release the lock.
  3. firecracker Fire an event named "release".

Shouldn't step 3 wait for the document to become fully active again before firing the event?

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.

(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.

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.

@reillyeon
Copy link
Member

If a document holding a becomes non-fully active and then becomes fully active then I think a release event should be fired. This pattern seems like something that will be common. For example, a WebSocket should fire a close event in the same situation. Maybe "queue a task" is the right way to express that, since task queues are paused while the document is not fully active.

@rakuco
Copy link
Member

rakuco commented Aug 20, 2021

If a document holding a becomes non-fully active and then becomes fully active then I think a release event should be fired.

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.

@marcoscaceres
Copy link
Member

marcoscaceres commented Oct 28, 2021

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.

@marcoscaceres
Copy link
Member

@rakuco, I think we can close this, right?

@marcoscaceres marcoscaceres changed the title WakeLock and Page Life Cycle WakeLock and only require activation once Apr 8, 2024
@w3c w3c deleted a comment May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants