Closed Bug 1522713 Opened 6 years ago Closed 6 years ago

Add a method to change a frameLoader's process

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla67
Fission Milestone M2
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.

https://searchfox.org/mozilla-central/rev/c035ee7d3a5cd6913e7143e1bce549ffb4a566ff/browser/base/content/tabbrowser.js#1657-1695

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;
}
Fission Milestone: --- → M2
Status: NEW → ASSIGNED

We have mRemoteFrameChild which owns a RemoteFrameChild, but
mRemoteFrame is just a state. Change to mIsRemoteFrame to reflect
this.

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

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

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.

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.

Minimal STR for at least one of the things I'm running into right now:

  1. Open new tab
  2. Load about:newtab
  3. 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.

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.

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:

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=232736513&revision=fc9eaf4eb8ceeebb953a5374c2626fc2caa88453

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

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

Attachment #9049651 - Attachment description: Bug 1522713 - Rename mRemoteFrame to mIsRemoteFrame → Bug 1522713 - Rename mRemoteFrame to mIsRemoteFrame;
Attachment #9049652 - Attachment description: Bug 1522713 - Allow updating of Frameloader Remoteness via FrameLoaderOwner → Bug 1522713 - Allow updating of Frameloader Remoteness via FrameLoaderOwner;
Attachment #9049653 - Attachment description: Bug 1522713 - Don't change node binding to tree when updating remoteness → Bug 1522713 - Don't change node binding to tree when updating remoteness;
Component: DOM → DOM: Core & HTML
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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: