8000 Fix images breaking on Firefox after a while (add heartbeat to prevent background page reap) by djahandarie · Pull Request #1852 · yomidevs/yomitan · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix images breaking on Firefox after a while (add heartbeat to prevent background page reap) #1852

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 2, 2025
Merged

Fix images breaking on Firefox after a while (add heartbeat to prevent background page reap) #1852

merged 4 commits into from
May 2, 2025

Conversation

djahandarie
Copy link
Collaborator
@djahandarie djahandarie commented Feb 28, 2025 •
edited
Loading

Since switching our image rendering to be over postMessage / MessageChannels, Firefox has had an issue where when its background page is reaped (which happens on MV3 extension on Firefox) after some time, and images will break on any existing tabs, while the rest of the extension will work (because sendMessage will cause the background page to come back alive, with a new worker, but the frontend will not know that it needs to reconnect to a new worker), and also new tabs will work because they will reconnect to the new backend worker.

I considered some different ideas (like ① putting a heartbeat directly in frontend worker to backend worker, and autoreparing, or ② having the backend signal all frontends that it's going byebye onSuspend), but in the end this idea was the simplest, to simply prevent the background page from dying.

The rest is as the comment says:

   /**
     * This is used to keep the background page alive on Firefox MV3, as it does not support offscreen.
     * The reason that backend persistency is required on FF is actually different from the reason it's required on Chromium --
     * on Chromium, persistency (which we achieve via the offscreen page, not via this heartbeat) is required because the load time
     * for the IndexedDB is incredibly long, which makes the first lookup after the extension sleeps take one minute+, which is
     * not acceptable. However, on Firefox, the database is backed by sqlite and starts very fast. Instead, the problem is that the
     * media-drawing-worker on the frontend holds a MessagePort to the database-worker on the backend, which closes when the extension
     * sleeps, because the database-worker is killed and currently there is no way to detect a closed port due to
     * https://github.com/whatwg/html/issues/1766 / https://github.com/whatwg/html/issues/10201
     *
     * So this is our only choice. We can remove this once there is a way to gracefully detect the closed MessagePort and rebuild it.
     * @returns {Promise<import('api').ApiReturn<'heartbeat'>>}
     */

@Kuuuube Kuuuube added kind/bug The issue or PR is regarding a bug area/performance The issue or PR is related to performance labels Mar 9, 2025
@IMakeBotsForYou
Copy link

Tested successfully on 2 devices; merge would be appreciated

@Kuuuube Kuuuube mentioned this pull request Mar 22, 2025
Closed
@Kuuuube Kuuuube mentioned this pull request Apr 6, 2025
Closed
@djahandarie djahandarie marked this pull request as ready for review May 2, 2025 23:39
@djahandarie djahandarie requested a review from a team as a code owner May 2, 2025 23:39
Kuuuube
Kuuuube approved these changes May 2, 2025
View reviewed changes
Copy link
Member
@Kuuuube Kuuuube left a comment

Choose a reason for hiding this comment

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

Looks good.

@djahandarie djahandarie added this pull request to the merge queue May 2, 2025
Merged via the queue into master with commit 88e9ddf May 2, 2025
26 checks passed
@djahandarie djahandarie deleted the extra-logging branch May 2, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kuuuube Kuuuube Kuuuube approved these changes

Assignees
No one assigned
Labels
area/performance The issue or PR is related to performance kind/bug The issue or PR is regarding a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@djahandarie @IMakeBotsForYou @Kuuuube
0