Skip to content

On Web, map bfcache load/unload to suspend/resume - #2912

Merged
daxpedda merged 1 commit into
rust-windowing:masterfrom
fornwall:web-avoid-destroying-event-loop
Jun 28, 2023
Merged

daxpedda merged 1 commit into
rust-windowing:masterfrom
fornwall:web-avoid-destroying-event-loop

Conversation

@fornwall

@fornwall fornwall commented Jun 26, 2023 •

Copy link
Copy Markdown
Contributor
  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

In #2261, the web backend was changed to use the pagehide event instead of unload, to allow the bfcache to be used.

See https://web.dev/bfcache/ for more information about the bfcache (back/forward cache), and https://developer.chrome.com/docs/devtools/application/back-forward-cache/ for how to test it in chrome. Summary from the first resource:

bfcache is an in-memory cache that stores a complete snapshot of a page (including the JavaScript heap) as the user is navigating away

The problem with winit and the bfcache is that the event loop is destroyed in pagehide - which happens before the snapshot is put in the bfcache. So when a user navigates back to the page running a winit program it's no longer responding.

This can be seen on e.g. the web example program (cargo run-wasm --example web) - after restoring the page from the bfcache the event log is no longer updated.

This PR changes pagehide handling to only destroy the event loop if persisted is false. If persisted is true, we now emit a Suspended event.

It also adds a corresponding listener for pageshow with persisted being true, to emit a Resumed event in that case.

@fornwall
fornwall requested a review from daxpedda as a code owner June 26, 2023 22:56
fornwall added a commit to fornwall/wgpu-game-of-life that referenced this pull request Jun 26, 2023

@daxpedda daxpedda left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what we should do here is look at PageTransitionEvent.persisted.

If it's true we should send Suspended and also hook up the pageshow event with a Resumed event.
If it's false we can resume to destroy the event loop.

Thank you for looking into this!

Comment thread CHANGELOG.md Resolved
@daxpedda daxpedda self-assigned this Jun 27, 2023
@daxpedda daxpedda added DS - web Affects the Web backend (WebAssembly/WASM) C - waiting on author Waiting for a response or another PR labels Jun 27, 2023
@fornwall
fornwall force-pushed the web-avoid-destroying-event-loop branch from f9f42fb to 1b8d46f Compare June 28, 2023 10:54
@fornwall fornwall changed the title On Web, do not destroy the event loop on pagehide events On Web, map bfcache load/unload to suspend/resume Jun 28, 2023
@fornwall

fornwall commented Jun 28, 2023 •

Copy link
Copy Markdown
Contributor Author

I think what we should do here is look at PageTransitionEvent.persisted.

If it's true we should send Suspended and also hook up the pageshow event with a Resumed event. If it's false we can resume to destroy the event loop.

Thanks @daxpedda for the helpful feedback! I updated the PR according to this, could you have a look?

Specifically, is it correct to just send_event(Event::Suspended) and send_event(Event::Resumed), or is something else required for proper lifecycle management?

@fornwall
fornwall force-pushed the web-avoid-destroying-event-loop branch 2 times, most recently from 1b8d46f to e5ed4d1 Compare June 28, 2023 11:03
@fornwall
fornwall requested a review from daxpedda June 28, 2023 11:05

@daxpedda daxpedda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff, thank you!
Couple of nits.

Lifecycle management seems fine. Though I'm trying to figure out if it's necessary for us to actively ignore a request_redraw() after a Suspend event.

Comment thread src/event.rs Resolved
@fornwall
fornwall force-pushed the web-avoid-destroying-event-loop branch from e5ed4d1 to 4c141af Compare June 28, 2023 11:47
@fornwall

Copy link
Copy Markdown
Contributor Author

@daxpedda Thanks again for the quick review!

I applied your suggested code change, and answered your comments about pageshow events happening initially with persisted == false. Let me know what you think!

@fornwall
fornwall requested a review from daxpedda June 28, 2023 11:55
@daxpedda
daxpedda merged commit 924f332 into rust-windowing:master Jun 28, 2023
@fornwall
fornwall deleted the web-avoid-destroying-event-loop branch September 25, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C - waiting on author Waiting for a response or another PR DS - web Affects the Web backend (WebAssembly/WASM)

Development

Successfully merging this pull request may close these issues.

2 participants