887781 - Need event for when message manager is disconnected

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Alexandre Poirot [:ochameau]]

For implementing devtools on b2g, we do have to stop using app iframe's message manager when the app is closed/crashed.

There is various observer notification being sent:
ipc:browser-destroyed, child-process-shutdown or ipc:content-shutdown

But none of them are sent for non-OOP message managers.
I haven't found any message, event or notification that would allow me to know, when I should stop listening for message and stop sending messages for a given message manager.

We should have an event for that that would fire no matter if the frame is remote or not.

After some prototyping and discussion with smaug, we end up suggesting to send a `message-manager-disconnected` nsIObserverService notification, from 
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsFrameMessageManager.cpp#808
  nsFrameMessageManager::Disconnect(bool aRemoveFromParent)

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Send message-manager-disconnect notification event when any message manager get disconnected.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Send message-manager-disconnect notification event when any message manager get disconnected.

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 768390 [details] [diff] [review]
Send message-manager-disconnect notification event when any message manager get disconnected.

Have I transcribed correctly what you explained to me on irc?

Comment 4

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

Comment on attachment 768390 [details] [diff] [review]
Send message-manager-disconnect notification event when any message manager get disconnected.

># HG changeset patch
># User Alexandre Poirot <[e-mail]>
>
>Bug 887781 - Send message-manager-disconnect notification event when any message manager get disconnected.
>
>diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp
>index 10549d9..d9e0bc9 100644
>--- a/content/base/src/nsFrameMessageManager.cpp
>+++ b/content/base/src/nsFrameMessageManager.cpp
>@@ -1,15 +1,16 @@
> /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> 
> #include "base/basictypes.h"
> 
>+
Please don't add this empty line

> function expectFrameProcessShutdown(iframe, frameMM, processMM, callback) {
>   let msgCount = 0;
>   function countMessage() {
>     msgCount += 1;
>-    if (msgCount == 3) {
>+    if (msgCount == 4) {
>       ok(true, "Observed all three expected events.");
s/three/four/

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Send message-manager-disconnect notification event when any message manager get disconnected.

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 768422 [details] [diff] [review]
Send message-manager-disconnect notification event when any message manager get disconnected.

Carry over the r+ after having addressed review comments.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba46beafbb79

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ba46beafbb79

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: b2g18 - Send message-manager-disconnect notification event when any message manager get disconnected.

Conflicts:

	content/base/test/test_child_process_shutdown_message.html

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 771317 [details] [diff] [review]
b2g18 - Send message-manager-disconnect notification event when any message manager get disconnected.

Patch rebased for b2g18

Comment 11

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

Should we perhaps take this to b2g18 so that fixing Bug 899354
would become easier (I assume one could observe the notification and oop-frameloader-crashed notification and remove the event listener if either one happens).

Comment 12

[Will Bamberg [:wbamberg]]

I've had a go at some docs for this: https://developer.mozilla.org/en-US/docs/Observer_Notifications#Message_manager

Alex/Wladimir, does this look all right? In particular, is the example sensible? If so, I'll add a link here from the main message manager docs.

Comment 13

[Wladimir Palant]

Yes, the documentation looks good to me - more extensive than what I expected.

Comment 14

[Alexandre Poirot [:ochameau]]

Thanks Will, that looks great!
But I would keep Observer_notifications page simple and just list all events, so that I would move this example in a message manager related page. I imagine we already have some ?

Comment 15

[Will Bamberg [:wbamberg]]

I've moved the example into the main message manager article (https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/The_message_manager) - I'm thinking of reorganizing that article a bit, but that's another story. Thanks for the reviews. I'm marking this ddc, but please don't hesitate to let me know if you see anything else.