Closed Bug 1606135 Opened 9 months ago Closed 2 months ago

make BackgroundPageThumbs fission compatible

Categories

(Firefox :: New Tab Page, task, P3)

task

Tracking

()

RESOLVED FIXED
81 Branch
Fission Milestone M6c
Tracking Status
firefox81 --- fixed

People

(Reporter: surkov, Assigned: enndeakin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

similar to bug bug 1576911, the logic should be switched to drawSnapshot API which is fission compatible (see bug 1475139). Beyond this, it seems we could do a number cleanup/code improvements here. Will file those as dependent bugs.

metadata for the file says bugs are tracked in New Tab Page so let's move this there.

Component: General → New Tab Page

Tracking for Fission Nightly (M6)

Fission Milestone: --- → M6
Priority: -- → P1

Can you point us to some documentation around what the best practice is for remediation here?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Gavin Suntop [:gvn] (he/they) from comment #3)

Can you point us to some documentation around what the best practice is for remediation here?

BackgroundPageThumbs uses a frame script ( backgroundPageThumbsContent.js ) right now. This is not compatible with fission. comment #0 suggests you could use the drawSnapshot API instead of having the frame script take a screenshot and then passing that back to the parent.

If more JS still needs to run in the content process for the background page thumbs code to work (which I'm not sure about) it would need to be converted to using a JSWindowActor. See e.g. https://phabricator.services.mozilla.com/D50584 for an example, https://firefox-source-docs.mozilla.org/dom/Fission.html#jswindowactor and https://firefox-source-docs.mozilla.org/dom/Fission.html#how-to-port-from-message-manager-and-framescripts-to-jswindowactors etc. for docs.

One issue I suspect may be problematic here is that AIUI there is currently no way to limit where we run JSWindowActors, except by URI. But in this case, we really want the actor to only run in the browser/docshell created for the background page thumbs. It'd be worth liaising directly with the fission team to figure that part out, if you end up needing to run JS.

Does that help?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(gsuntop)

Yes, thank you!

Flags: needinfo?(gsuntop)
Priority: P1 → P3
Assignee: surkov.alexander → enndeakin
Status: NEW → ASSIGNED

Tracking for Fission M6c Nightly milestone.

Fission Milestone: M6 → M6c
Depends on: 1644915

Now that windowless browser supports fission, I now have the background page thumbnailer creating new processes for each domain that it loads. However, this requires passing 'Ci.nsIWebBrowserChrome.CHROME_REMOTE_WINDOW | Ci.nsIWebBrowserChrome.CHROME_FISSION_WINDOW' to AppShell::CreateWindowlessBrowser when fission is enabled. I know we want to avoid using the fission.autostart preference, but it seems like this is a case where it can't be avoided. Does this sound right?

Flags: needinfo?(nika)

Yes, I believe background processes which need to create windows unrelated to any other existing window and need to pick whether those windows have Fission enabled will need to check the fission.autostart pref directly still.

Flags: needinfo?(nika)
Attachment #9138915 - Attachment description: Bug 1606135, switch background page thumbs service to use drawSnapshot and an actor instead of a framescript → Bug 1606135, switch background page thumbs service to use drawSnapshot and an actor instead of a framescript, r=adw
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f04d5ddcee0
switch background page thumbs service to use drawSnapshot and an actor instead of a framescript, r=adw
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Regressions: 1657036
You need to log in before you can comment on or make changes to this bug.