WebReplay mochitests broken by bug 1521189

RESOLVED FIXED in Firefox 67

Status

()

enhancement
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: Nika, Assigned: bhackett)

Tracking

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

These tests began failing after changes from bug 1521189 - it looks like WebReplay changes the shutdown lifecycle of PBrowser actors in a way which isn't compatible with this new actor's lifecycle.

Assignee

Comment 1

5 months ago
Posted patch patchSplinter Review

The problem here is that IPDL messages sent to the middleman are forwarded to their recording child process unless there is special handling for the message in ParentForwarding.cpp. The middleman has its own WindowGlobalChild actors, and if it sends a Destroy to the parent then the parent's delete response ends up being received in the recording process, which crashes.

The attached patch fixes this by dispatching parent->child PWindowGlobal messages to either the middleman or the recording process, depending on whether there is an actor with the right routing ID in the middleman.

Assignee: nobody → bhackett1024
Attachment #9039622 - Flags: review?(nika)
Comment on attachment 9039622 [details] [diff] [review]
patch

Review of attachment 9039622 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/recordreplay/ipc/ParentForwarding.cpp
@@ +140,5 @@
> +          found = true;
> +          break;
> +        }
> +      }
> +    }

Couldn't this entire check be simplified to:

```
IProtocol* actor = contentChild->Lookup(aMessage.routing_id());
if (actor && actor->GetProtocolTypeId() == PWindowGlobalStart) {
  // ...
}
```
Attachment #9039622 - Flags: review?(nika) → review+

Comment 3

5 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2220ce4355d5
Handle WindowGlobal messages within the middleman when appropriate, r=nika.

Comment 4

5 months ago
bugherder
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.