Add a method to change a frameLoader's process
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: nika, Assigned: qdot)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(3 files)
Currently when we want to change the process of a page loaded in a xul:browser, we have to go through frontend code to do so. This is currently done in updateBrowserRemoteness
, where we remove the <browser>
from the DOM, change the "remote"
and "remoteType"
attributes, and re-insert it.
We should add a new method to nsFrameLoader
which takes relevant properties (perhaps as a dictionary
, such that it is easy to add new options), and triggers an internal process swap without DOM mutations.
// Potential Signature
[ChromeOnly]
partial interface FrameLoader {
[Throws] void updateRemoteness(RemotenessOptions aDict);
}
dictionary RemotenessOptions {
DOMString? remoteType;
FrameLoader? sameProcessAsFrameLoader;
WindowProxy? opener;
}
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
We have mRemoteFrameChild which owns a RemoteFrameChild, but
mRemoteFrame is just a state. Change to mIsRemoteFrame to reflect
this.
Assignee | ||
Comment 2•6 years ago
|
||
Adds a method for to nsFrameLoaderOwner destroying and rebuilding a
FrameLoader in order to facilitate a process switch. Method works
without requiring that the work be done in the frontend.
Depends on D22789
Assignee | ||
Comment 3•6 years ago
|
||
Since we now have a method on nsFrameLoaderOwner/MozFrameLoaderOwner
that can update remoteness, we should no longer need to unbind and
rebind browser elements to the tree to change their remoteness
attributes. We can just call the method and have the Frameloaders
rebuilt in the backend.
Depends on D22790
Assignee | ||
Comment 4•6 years ago
|
||
Adding WIP patches for now, but still battling oranges on try. Latest try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf6c939539d40e234381c3243c84e95195e11761
All failures are now either browser-chrome or marionette tests.
For browser chrome, it looks like there may be something up with SessionStore or Navigation. Still trying to nail down exactly what it is.
For marionette (and also some bc tests), it seems that changing Frameloaders via the new method causes some pages not to render correctly. I can't tell if it's just bc pages (about:*, etc) or what yet, but there's a lot of failed simulated click events and what not.
Assignee | ||
Comment 5•6 years ago
|
||
Some of the oranges on try are also asserting on the same issues as bug 1301399, where we have session history but no mOSHE in docshell. Still trying to figure out how to work around that, or if it is what is triggering these problems. There's a chance that's the culprit for some of the navigation issues, since we can't move to mOSHE if there isn't one.
Assignee | ||
Comment 6•6 years ago
|
||
Minimal STR for at least one of the things I'm running into right now:
- Open new tab
- Load about:newtab
- Load example.org
Expected:
Navigating to example.org will show active back button, which goes back to about:newtab.
Actual:
Back button not active.
Extra fun:
If you try navigating
about:newtab -> example.org -> about:newtab -> example.org, you get a history chain of example.org -> example.org. So history entries are getting dropped during the process switch using the new method.
Assignee | ||
Comment 7•6 years ago
|
||
Just looked at a stack from a clean tree where the test timeout for browser/base/content/test/tabs/browser_new_tab_in_privileged_process_pref.js is happening.
It looks like, with the patches attached to this bug, we're missing a call to _callProgressListeners with an OnLocationChange update. This should happen on creation the new browser-custom-element, not sure why that's not happening.
Assignee | ||
Comment 9•6 years ago
|
||
Bugs mentioned in Comment 6 and Comment 7 fixed, was a problem with where browser element attributes were set in relation to FrameLoader rebuilding.
New try:
Still some failures:
- Python tests (Marionette, Firefox Functional Tests)
- browser/components/preferences/in-content/tests/[a bunch of tests] (Not sure why but updating the prefs dialog in tests is broken in quite a few ways)
- browser/base/content/test/general/browser_bug495058.js (Probably event or remote type based)
- browser/base/content/test/general/browser_contentSearchUI.js (already has an intermittent filed but I think this patch makes it permaorange?)
- Some devtools breakage with responsive mode
Assignee | ||
Comment 10•6 years ago
|
||
Added a patch with a pref to turn frameloader building on, defaulting to off, so we can at least land this and get it to other fission team members while I'm on PTO.
Seems to be passing try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5421b059cab3662db099eafd331c442ed0e90378
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 11•6 years ago
|
||
Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d63e984effad Rename mRemoteFrame to mIsRemoteFrame; r=nika https://hg.mozilla.org/integration/autoland/rev/bade3af814f6 Allow updating of Frameloader Remoteness via FrameLoaderOwner; r=nika https://hg.mozilla.org/integration/autoland/rev/0bc302ab1f25 Don't change node binding to tree when updating remoteness; r=nika
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d63e984effad
https://hg.mozilla.org/mozilla-central/rev/bade3af814f6
https://hg.mozilla.org/mozilla-central/rev/0bc302ab1f25
Description
•