1919633 - JS actors in dead ContentParents can receive messages after their protocol is unregistered

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Andrew McCreight [:mccr8]]

While I don't think this happens in regular browser usage, tests can unregister protocols.

One of these is browser_html_warning_messages.js, which has a subtest testUnsignedDisabled that disables SpecialPowers, which ends up running this onShutdown method that unregisters both the SpecialPowers and AppTestDelegate actors.

As part of bug 1885221 (typing JS window actor messages) I have added a special type testOnly. If a message is testOnly, we don't check the message, but instead check that the actor's current module URI matches an allow list of testing URIs. Anyways, when running the part of browser_html_warning_messages.js I mentioned above, this checking intermittently fails for the load message of the actor AppTestDelegate with the error (from my type checking patch): "couldn't get module URI for an actor with a testOnly type: no actor protocol found". This means we're inside JSActorManager::ReceiveRawMessage, and the GetActor call has succeeded, but our protocol is not actually currently registered, which seems bad.

I debugged it a bit and it looks like we never call JSActorManager::JSActorUnregister on the actor manager that has the relevant instance of AppTestDelegate. In the occasional instance where we don't hit the type error, it looks like JSActorDidDestroy runs before JSActorManager::ReceiveRawMessage, so we avoid getting the message that way.

With some further debugging, it appears that we skip over this actor manager here in JSActorService::UnregisterWindowActor because the relevant ContentParent is dead. It seems to fix the problem if we iterate over all ContentParents and not just live ones, and simply skip the SendUnregisterJSWindowActor call if the ContentParent is not alive, but maybe there's some bigger problem here and we should check that our ContentParent is alive in JSActorManager::ReceiveRawMessage? That being said, I added logging in WindowGlobalParent::RecvRawMessage for when the ContentParent is not alive, and it is happening quite a bit so maybe we can't do that easily.

It looks like UnregisterProcessActor has the same issue as UnregisterWindowActor.

Comment 1

[Andrew McCreight [:mccr8]]

Nika, any ideas? Thanks.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1919633 - Unregister JS actors even if the ContentParent is not alive.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

I think it's normal for a JSActor to potentially receive messages from "dead" ContentParent objects, because being "dead" actually just means that the process is in the process of dying, and can no longer be re-used (yes, it's an unfortunate naming). The process when being marked dead doesn't even necessarily have to have been sent a shutdown message yet - it might not be sent until a tab which is in the process of closing finishes exiting.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3227b078a10c
Unregister JS actors even if the ContentParent is not alive. r=nika

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/3227b078a10c