Closed
Bug 1497057
Opened 6 years ago
Closed 6 years ago
Remove ability to restart replaying children
Categories
(Core Graveyard :: Web Replay, enhancement)
Core Graveyard
Web Replay
Tracking
(firefox64 fixed)
RESOLVED
FIXED
mozilla64
| Tracking | Status | |
|---|---|---|
| firefox64 | --- | fixed |
People
(Reporter: bhackett1024, Assigned: bhackett1024)
Details
Attachments
(1 file)
|
6.58 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
Middleman processes can restart replaying children that have hanged or crashed. This feature was added to improve the user experience (being impacted by fewer crashes) but it seems to have turned out to have the opposite effect: many times replaying processes crash over and over in the same place, so instead of getting an immediate crash the tab hangs unresponsively for a while while the middleman keeps starting new processes and sending them to their doom, before finally giving up. This feature is also questionable from a software design standpoint, as replaying processes operate in a restricted environment (e.g. no flaky graphics drivers to interact with) and it should be possible to diagnose and fix all their crashes. The attached patch removes the ability to restart replaying children, and associated logic. Maybe we'll want this again sometime, but if so there isn't that much code involved to add back in (the bulk of the code for recovering child processes is still needed to switch back and forth between active recording and replaying children).
Attachment #9015124 -
Flags: review?(continuation)
|
||
Comment 1•6 years ago
|
||
Comment on attachment 9015124 [details] [diff] [review] patch Review of attachment 9015124 [details] [diff] [review]: ----------------------------------------------------------------- That makes sense. You can hit a similar problem with e10s child processes, but there the user decides to reload the page each time, so they can just decide to not do it if it keeps happening. ::: toolkit/recordreplay/ipc/ChildProcess.cpp @@ +522,2 @@ > void > +ChildProcessInfo::OnCrash(const char* aWhy) Maybe throw a comment here about why we don't try to restart the child?
Attachment #9015124 -
Flags: review?(continuation) → review+
Pushed by bhackett@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/31c68d4d5e8b Remove ability to restart replaying children, r=mccr8.
|
||
Comment 3•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/31c68d4d5e8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
|
||
Updated•4 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•