Closed Bug 1483248 Opened 2 years ago Closed 2 years ago

Allow recording child processes to handle TabChild deletion

Categories

(Core :: Web Replay, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached patch patchSplinter Review
Currently, middleman processes are responsible for sending delete messages for PBrowserChild actors to the UI process. This gives consistent behavior between middlemen that have recording children and ones that don't, but causes a lot of problems for PBrowserChild teardown: deleting a PBrowserChild also deletes all children of that actor, and can implicitly delete other state like any PContentPermissionRequest actors associated with the browser child's tab. The middleman tries to emulate this and make sure that its recording child process (if any) does not send messages to actors that have been implicitly deleted in this way, but this emulation is complicated, fragile, and incomplete (e.g. for the PContentPermissionRequest thing).

The attached patch greatly simplifies this situation by performing the PBrowserChild deletion in the recording child instead of the middleman, ensuring that the right teardown messages for associated actors are sent at the right time.  Middleman processes with only replaying children still need to do the deletes themselves, adding a little instrumentation.
Attachment #8999966 - Flags: review?(continuation)
Attachment #8999966 - Flags: review?(continuation) → review+
This needs a followup to work properly when closing a tab that has a recording child but is replaying old content.  In such cases we normally don't forward IPDL messages to the recording child (so that e.g. mouse clicks and other events that happen while replaying old state have no effect on the recording), but need to make an exception for PBrowser::Destroy.
Attachment #9002527 - Flags: review?(continuation)
Attachment #9002527 - Flags: review?(continuation) → review+
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/929bae71da4c
Part 1 - Allow recording child processes to handle TabChild deletion, r=mccr8.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8d0b1165402
Part 2 - Always forward destroy messages to recording TabChild, r=mccr8.
https://hg.mozilla.org/mozilla-central/rev/929bae71da4c
https://hg.mozilla.org/mozilla-central/rev/b8d0b1165402
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.