Closed Bug 1580766 Opened 1 year ago Closed 6 months ago

Expose a "session id" on CanonicalBrowsingContext, preserved when a tab's top level BC is replaced

Categories

(Core :: DOM: Content Processes, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla79
Fission Milestone M6a
Tracking Status
firefox79 --- fixed

People

(Reporter: zombie, Assigned: kashav)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

We need a stable ID that would represent a tab across BrowserContext getting replaced.

Wrong dependency.

Blocks: 1580764
No longer blocks: JSProcessActor

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → ?

@ Dave, are you working on this bug? kmag said you may have started working on this bug already.

Tracking this bug for Fission dogfooding (milestone M5) because kmag says people keep running into this problem with BrowsingContexts getting replaced.

Fission Milestone: ? → M5
Flags: needinfo?(dtownsend)
Priority: -- → P3
Duplicate of this bug: 1601104

Yes, didn't realise there was a bug already on file. Hopefully have a patch up today or tomorrow.

Assignee: nobody → dtownsend
Flags: needinfo?(dtownsend)

Adds a browserId property to all browsing contexts which the same for the
entire tree of contexts inside a frame element. If a new top-level context is
created for the frame then it is assigned the same value.

This allows identifying the frame element for a given browsing context.

Currently this is only done for XUL frame elements (browser/iframe). Not sure
if we want this for others.

Dave, are you still working on this browserId patch? This patch will unblock some Fission work. kmag provided some review feedback to be addressed.

Flags: needinfo?(dtownsend)

(In reply to Chris Peterson [:cpeterson] from comment #7)

Dave, are you still working on this browserId patch? This patch will unblock some Fission work. kmag provided some review feedback to be addressed.

I will probably have time to pick this back up sometime this week. If it needs to be finished sooner feel free to find someone else to pick it up.

Flags: needinfo?(dtownsend)

mossop's patch is waiting for r?kmag.

Zombie wants this feature.

Dave, what work is still needed for this session id bug? Phabricator says the review state is "Changes Planned".

Flags: needinfo?(dtownsend)

(In reply to Chris Peterson [:cpeterson] from comment #10)

Dave, what work is still needed for this session id bug? Phabricator says the review state is "Changes Planned".

The issue is that detailed at https://phabricator.services.mozilla.com/D56245#2191328. Unfortunately this bug is no longer a priority for me as it doesn't block anything I'm working on and so I'm having difficulty finding the time to finish it. If someone else wants to take it then that would be fine.

Flags: needinfo?(dtownsend)
Blocks: 1611142

(In reply to Dave Townsend [:mossop] (he/him) from comment #11)

If someone else wants to take it then that would be fine.

OK. I'll find another engineer to continue your work in progress.

Moving this bug to Fission Nightly M6a because Zombie says it doesn't need to block Fission dogfooding but recommends we finish it soon.

Assignee: dtownsend → nobody
Fission Milestone: M5 → M6a
Priority: P3 → P2

Assigning to Kashav because he volunteered to finish this bug.

Assignee: nobody → kmadan
See Also: → 1638007
Blocks: 1641638
Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30c061da89d8
Add a unique ID for the BrowsingContext tree inside a browser element. r=kmag

This is failing for the old rdm (which has been disabled in nightly, but is still the default for tests).

We want to swap the browser Ids in nsFrameLoader::SwapWithOtherRemoteLoader, but mOwnerContent is an HTMLIFrameElement for the rdm frame, and we can't get a XULFrameElement from that.

I'm not sure how long we plan to keep the old rdm around, so it might not be worth it to fix this. Maybe we can run the failing tests with the new pref? I'll get more information tomorrow.

Attachment #9153586 - Attachment description: Bug 1580766 - Handle swapping browser Ids for <iframe mozbrowser> frameloader swaps, r?kmag → Bug 1580766 - Move BrowserId to nsFrameLoaderOwner, r?kmag
Flags: needinfo?(kmadan)
Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3722901b6309
Add a unique ID for the BrowsingContext tree inside a browser element. r=kmag
https://hg.mozilla.org/integration/autoland/rev/6b9926a5ab88
Move BrowserId to nsFrameLoaderOwner, r=kmag

The assertions added in CreateBrowsingContext didn't account for content-initiated frame adoptions. We end up in a situation where we're creating a BrowsingContext for a frame that has an Id from its previous BrowsingContext. It might be possible to detect if we're adopting and make those assertions conditional, or just reset the browser Id when we start adopting, but the easiest solution is probably just to remove the assertions.

Flags: needinfo?(kmadan)
Attachment #9153586 - Attachment description: Bug 1580766 - Move BrowserId to nsFrameLoaderOwner, r?kmag → Bug 1580766 - Add a unique ID for the BrowsingContext tree inside a browser element. r=kmag
Attachment #9114258 - Attachment is obsolete: true
Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/585e0230b2d5
Add a unique ID for the BrowsingContext tree inside a browser element. r=kmag
Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b75b8bc4d1ac
Add a unique ID for the BrowsingContext tree inside a browser element. r=kmag
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
Blocks: 1646563
You need to log in before you can comment on or make changes to this bug.