Closed Bug 1575624 Opened 5 years ago Closed 4 years ago

Replace RemoteWebProgressManager with a C++ Component

Categories

(Core :: DOM: Content Processes, enhancement, P3)

70 Branch
enhancement

Tracking

()

RESOLVED INVALID
Tracking Status
firefox70 --- affected

People

(Reporter: barret, Assigned: barret)

References

Details

This is a continuation of the work bug 1510569 to remove nsIWebProgress implementations in JavaScript and rewrite their IPC to use IPDL. Specifically, RemoteWebProgressManager can be replaced with a C++ component after the ListenerArray in nsDocLoader is refactored out into its own component.

Assignee: nobody → brennie

Barret, are you still working on this bug to rewrite RemoteWebProgressManager?

If so, you should talk to mwoodrow. He thinks we should instead synthesize web progress events in parent process.

Flags: needinfo?(brennie)
Priority: -- → P3

I have some patches laying around from 6 months ago that worked on this, but if :mwoodrow has a better idea about we should handle this, I'm A-OK with closing this bug.

Flags: needinfo?(brennie)

(In reply to Barret Rennie [:barret] (he/him) from comment #2)

I have some patches laying around from 6 months ago that worked on this, but if :mwoodrow has a better idea about we should handle this, I'm A-OK with closing this bug.

I don't think Matt intended to rewrite the web progress event code any time soon, so feel free to proceed with your patches, if you like. I don't know what the priority of this work is.

Matt, is that correct? Do you have feedback for Barret if he does rewrite RemoteWebProgressManager in C++?

Flags: needinfo?(matt.woodrow)

The current code is assuming that the progress events for a docshell are meaningful for tracking the progress of a BrowsingContext navigation, and we're just directly forwarding the docshell events from the content process to the parent process.

With process switching (which happens now, but will happen much more often with fission), this really isn't true, as navigations can start in one docshell, and then swapped to a new docshell/process to finish. Bug 1602318 will add support for navigations that start without a docshell entirely (but will still finish with one).

The current solution for this is to filter the forwarded events (in BrowserParent::RecvOnStateChange mainly), so that the process switch isn't visible. That is, when we process switch, we hide the STOP from the old process and the START from the new one, so that the parent only sees a single START/STOP pair. That sort of works, but it's pretty ugly, and bug 1602318 will make it worse.

I think the ideal solution is for CanonicalBrowsingContext to just synthesize web progress events for the parent process, and to abandon trying to forward things from the content process.

Flags: needinfo?(matt.woodrow)

RemoteWebProgressManager was removed.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.