677638 - (MessageChannel) Implement HTML5's channel messaging API

Status: RESOLVED FIXED

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

Description

[James Leigh]

User Agent: Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.30 (KHTML, like Gecko) Ubuntu/11.04 Chromium/12.0.742.112 Chrome/12.0.742.112 Safari/534.30

Steps to reproduce:

Channel messaging appears to be unimplemented.

The example code from the HTML5 specification produces an error. The line of code "var channel = new MessageChannel();" from the HTML5 spec below causes an exception in the error console.

http://dev.w3.org/html5/postmsg/#channel-messaging


Actual results:

"Error: MessageChannel is not defined" appears in the error console


Expected results:

Independent pieces of code (e.g. running in different browsing contexts) should be able to communicate directly using channel messaging.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Use postMessage?

Just curious,
do you have some real world use case for MessageChannel's ports?

Comment 2

[James Leigh]

When writing a reusable js library it is important not to clobber other messages that might be intended for other libraries within the page. By using a message channel all communication within that channel can be controlled.

By restricting all communication to previously authorized message channels, the chances of an malicious message received from an untrusted source is reduced.

It's in the HTML5 spec draft and two implementations already exist: WebKit and Opera.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

yeah, I can see use cases for ports, it is mainly the
MessageChannel I'm curious about. Since, IMO, MessageChannel is rather
awkward and it would be probably easier to have
var portToOtherWindow = otherWindow.createPort("name");
or some such.

Comment 4

[Ioana (away)]

This issue reproduces exactly as in the bug description on:
Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110920 Firefox/9.0a1 (20110920085517).

Comment 5

[Florian Bender]

I agree with :smaug. This will make it a lot easier to use MessageChannel. Plus, it should provide better security by design, IMO.

Comment 6

[Yehuda Katz]

This is also implemented in Internet Explorer[1][2].

[1] http://msdn.microsoft.com/en-us/library/ie/hh673525(v=vs.85).aspx
[2] http://msdn.microsoft.com/en-us/library/windows/apps/hh441303.aspx

Comment 7

[Yehuda Katz]

It is also implemented in Safari[1][2].

[1] http://trac.webkit.org/browser/trunk/WebCore/dom/MessageChannel.cpp?rev=44915
[2] https://www.evernote.com/shard/s2/sh/2ee0b6ed-45b2-4dcd-be7f-38920e9d1793/b1f0f712fa618c72dbb2f4079ec34266/res/fb04a128-214e-4095-82fd-f051e983f957/skitch.png

Comment 8

[Mike Williamson]

Hi - this bug has been dormant for awhile, I was wondering if there were any plans for it?

We're building a fairly large, HTML5 app, which uses multiple webworkers, and the only way we've found for one worker to communicate with another so far is to pass each worker a connected MessagePort.

I'm assuming this bug needs engineering resources?  It seems like implementing the entire channeling API would be fairly non-trivial, but we'd love to expand our platform base beyond just Chrome.

Comment 9

[Brendan Eich [:brendan]]

Cc'ing Johnny and Jonas - can we get this bug assigned? Looks like it is _de riguer_.

See also https://news.ycombinator.com/item?id=6154170.

/be

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Johnny, anyone on the DOM team that can help? I'm reluctant to pile more stuff on Khuey or Bent given that we already have a number of critical things depending on them.

Comment 11

[Mounir Lamouri (:mounir)]

I hope to be able to get (part of) this feature implemented as a side effect of bug 876397.

Comment 12

[Andrew Overholt [:overholt]]

baku will likely have time when he gets back from vacation

Comment 13

[Johnny Stenback (:jst)]

Over to baku per Andrew's comment. Andrea, if you don't have time once you're back, let me and Andrew know and we'll hunt down other resources to help here.

Comment 14

[Andrea Marchesini [:baku]]

(In reply to Johnny Stenback (:jst, [e-mail]) from comment #13)
> Over to baku per Andrew's comment. Andrea, if you don't have time once
> you're back, let me and Andrew know and we'll hunt down other resources to
> help here.

I start working on it tomorrow. Sounds fun.

Comment 15

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Please make sure you look at bug 907060, we need to make MessagePort objects pretty flexible.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

There are also some challenges here when it comes to worker lifetimes. Bent should be able to explain the problem as well as a potential solution.

Comment 17

[Andrea Marchesini [:baku]]

Attachment: WIP 1 - Interfaces

Comment 18

[Andrea Marchesini [:baku]]

Attachment: WIP 2 - Structured Clone Algorithm

Comment 19

[Andrea Marchesini [:baku]]

Attachment: WIP 3 - PostMessage

Comment 20

[Andrea Marchesini [:baku]]

Attachment: WIP 3 - PostMessage

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 794591 [details] [diff] [review]
WIP 3 - PostMessage


>+
>+  // We can't simply call dispatchEvent on the window because doing so ends
>+  // up flipping the trusted bit on the event, and we don't want that to
>+  // happen because then untrusted content can call postMessage on a chrome
>+  // window if it can get a reference to it.
content sure shouldn't get access to chrome window.

>+
>+  nsIPresShell *shell = doc->GetShell();
>+  nsRefPtr<nsPresContext> presContext;
>+  if (shell) {
>+    presContext = shell->GetPresContext();
>+  }


>+
>+  message->SetTrusted(mTrusted);
These events should be always trusted.

>+
>+  nsEventStatus status = nsEventStatus_eIgnore;
>+  nsEventDispatcher::Dispatch(mPort,
>+                              presContext,
>+                              message->GetInternalNSEvent(),
>+                              message,
>+                              &status);
Just use the normal DispatchEvent on mPort


But I see this is existing code in nsGlobalWindow and that code is on crack, and yes, apparently I did review it.

Comment 22

[Andrea Marchesini [:baku]]

Attachment: patch 1 - WebIDL interfaces

Comment 23

[Andrea Marchesini [:baku]]

Attachment: patch 2 - structured clone algorithm

This second patch makes MessagePort transferable via window.postMessage.

Comment 24

[Andrea Marchesini [:baku]]

Attachment: patch 3 - postMessage

Implementation of postMessage in MessagePort

Comment 25

[Andrea Marchesini [:baku]]

Attachment: patch 4 - Start() method

This patch implements start() method and the queue of events.

Comment 26

[Andrea Marchesini [:baku]]

Attachment: patch 5 - unshipped message port queue

If this test covers all the unshipped message port queue usage, then we don't have to implement anything more than what we already have.

Comment 27

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 795513 [details] [diff] [review]
patch 1 - WebIDL interfaces

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

::: dom/base/MessagePort.h
@@ +28,5 @@
> +
> +  nsPIDOMWindow*
> +  GetParentObject() const
> +  {
> +     return GetOwner();

We don't usually do 3-space indentation

Comment 28

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 795516 [details] [diff] [review]
patch 3 - postMessage

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

::: dom/base/MessagePort.cpp
@@ +216,5 @@
> +  }
> +
> +  nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(doc);
> +  nsCOMPtr<nsIDOMEvent> event;
> +  domDoc->CreateEvent(NS_LITERAL_STRING("MessageEvent"), getter_AddRefs(event));

Call the version on nsIDocument

@@ +218,5 @@
> +  nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(doc);
> +  nsCOMPtr<nsIDOMEvent> event;
> +  domDoc->CreateEvent(NS_LITERAL_STRING("MessageEvent"), getter_AddRefs(event));
> +  if (!event)
> +    return NS_OK;

{}

@@ +286,5 @@
> +  scInfo.event = event;
> +  scInfo.port = this;
> +
> +  JS::Rooted<JS::Value> transferable(aCx, aTransfer.WasPassed()
> +                                            ? aTransfer.Value() : JSVAL_VOID);

Why not

  JS::Handle<JS::Value> transferable = aTransfer.WasPassed()
                                       ? aTransfer.Value()
                                       : JS::UndefinedHandleValue;

Comment 29

[Andrea Marchesini [:baku]]

Attachment: patch 3 - postMessage

Comment 31

[Boris Zbarsky [:bzbarsky]]

Note that you want to avid JSVAL_* no matter what.  Use JS::UndefinedValue() instead if you need that value.

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795513 [details] [diff] [review]
patch 1 - WebIDL interfaces

>+class MessagePort MOZ_FINAL : public nsDOMEventTargetHelper
>+{
>+public:
>+  NS_DECL_ISUPPORTS_INHERITED
>+  NS_REALLY_FORWARD_NSIDOMEVENTTARGET(nsDOMEventTargetHelper)
>+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(MessagePort,
>+                                           nsDOMEventTargetHelper)
>+
>+  MessagePort(nsPIDOMWindow* aWindow);
>+  ~MessagePort();
>+
>+  nsPIDOMWindow*
>+  GetParentObject() const
>+  {
>+     return GetOwner();
>+  }
nsDOMEventTargetHelper has already GetParentObject

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795515 [details] [diff] [review]
patch 2 - structured clone algorithm

PostMessageRead/WriteStructuredClone will need some clean ups at some point

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795513 [details] [diff] [review]
patch 1 - WebIDL interfaces


>+class MessageChannel MOZ_FINAL : public nsISupports
>+                               , public nsWrapperCache
MessageChannel wouldn't have to inherit nsISupports, but
doesn't matter much.

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795968 [details] [diff] [review]
patch 5 - unshipped message port queue

Please add also a test for the case when one of the ports isn't started yet, so
that messages to it don't go to the unshipped port queue.

And don't just post messages in a loop, but add some timeouts and such.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795997 [details] [diff] [review]
patch 3 - postMessage

>+struct StructuredCloneInfo {
{ to the next line.

>+  PostMessageRunnable* event;
>+  MessagePort* port;
mEvent, mPort
(I know, we're not super consistent when naming things in structs.)


> MessagePort::PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
>-                         const Optional<JS::Handle<JS::Value> >& aTransfer)
>+                         const Optional<JS::Handle<JS::Value> >& aTransfer,
>+                         ErrorResult& aRv)
> {
This doesn't handle the queue, but I think it will be added in some other patch...

Comment 37

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 795518 [details] [diff] [review]
patch 4 - Start() method

># HG changeset patch
># Parent a4af4ae5eae7963cf16c769fca696688389947b3
># User Andrea Marchesini <[e-mail]>
>Bug 677638 - Start() method
>
>diff --git a/dom/base/MessagePort.cpp b/dom/base/MessagePort.cpp
>--- a/dom/base/MessagePort.cpp
>+++ b/dom/base/MessagePort.cpp
>@@ -21,40 +21,49 @@
> namespace mozilla {
> namespace dom {
> 
> class PostMessageRunnable : public nsRunnable
> {
>   public:
>     NS_DECL_NSIRUNNABLE
> 
>-    PostMessageRunnable(MessagePort* aPort)
>-      : mPort(aPort)
>-      , mMessage(nullptr)
>+    PostMessageRunnable()
>+      : mMessage(nullptr)
>       , mMessageLen(0)
>     {
>     }
> 
>     ~PostMessageRunnable()
>     {
>-      NS_ASSERTION(!mMessage, "Message should have been deserialized!");
>+      // Ensure that the buffer is freed
>+      if (mMessage) {
>+        JSAutoStructuredCloneBuffer buffer;
>+        buffer.adopt(mMessage, mMessageLen);
>+      }
>     }
> 
>     void SetJSData(JSAutoStructuredCloneBuffer& aBuffer)
>     {
>       NS_ASSERTION(!mMessage && mMessageLen == 0, "Don't call twice!");
>       aBuffer.steal(&mMessage, &mMessageLen);
>     }
> 
>     bool StoreISupports(nsISupports* aSupports)
>     {
>       mSupportsArray.AppendElement(aSupports);
>       return true;
>     }
> 
>+    void Dispatch(MessagePort* aPort)
>+    {
>+      mPort = aPort;
>+      NS_DispatchToCurrentThread(this);
>+    }
>+
>   private:
>     nsRefPtr<MessagePort> mPort;
>     uint64_t* mMessage;
>     size_t mMessageLen;
> 
>     nsTArray<nsCOMPtr<nsISupports> > mSupportsArray;
> };
> 
>@@ -176,16 +185,18 @@ JSStructuredCloneCallbacks kPostMessageC
>   nullptr
> };
> 
> } // anonymous namespace
> 
> NS_IMETHODIMP
> PostMessageRunnable::Run()
> {
>+  MOZ_ASSERT(mPort);
>+
>   // Ensure that the buffer is freed even if we fail to post the message
>   JSAutoStructuredCloneBuffer buffer;
>   buffer.adopt(mMessage, mMessageLen);
>   mMessage = nullptr;
>   mMessageLen = 0;
> 
>   // Get the JSContext for the target window
>   nsCOMPtr<nsIScriptGlobalObject> sgo = do_QueryInterface(mPort->GetOwner());
>@@ -235,27 +246,28 @@ PostMessageRunnable::Run()
> 
>   message->SetTrusted(true);
> 
>   bool status;
>   mPort->DispatchEvent(event, &status);
>   return status ? NS_OK : NS_ERROR_FAILURE;
> }
> 
>-NS_IMPL_CYCLE_COLLECTION_INHERITED_1(MessagePort, nsDOMEventTargetHelper,
>-                                     mEntangledPort)
>+NS_IMPL_CYCLE_COLLECTION_INHERITED_2(MessagePort, nsDOMEventTargetHelper,
>+                                     mEntangledPort, mMessageQueue)
So the queue contains PostMessageRunnable objects, which aren't cycle collectable objects.
Those runnables may keep some nsISupports objects alive, so they should be cycle collected.
Perhaps you could just manually traverse/unlink the members of the PostMessageRunnable objects.
(nsRunnable has threadsafe refcounting so we may not want make PostMessageRunnable itself cycle collectable)


> MessagePort::Start()
> {
>-  // TODO
>+  if (mMessageQueueEnabled) {
>+    return;
>+  }
>+
>+  mMessageQueueEnabled = true;
>+
>+  while (!mMessageQueue.IsEmpty()) {
>+    nsRefPtr<PostMessageRunnable> event = mMessageQueue.ElementAt(0);
>+    mMessageQueue.RemoveElementAt(0);
>+
>+    event->Dispatch(this);
>+  }
Is this quite right? What should happen if some message event listener transfer the port to another location?

"4. Move all the events in the port message queue of original port to the port message queue of new port, if any, leaving the new port's port message queue in its initial disabled state."



Could you also post a patch which contains all the changes.

Comment 38

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Andrea: I'm worried about landing support for MessageChannel without also supporting passing ports to Workers. If we do that it becomes a lot more complicated for authors to test if MessageChannel-to-workers is supported or not since they can't simply do

if ("MessageChannel" in window) {
  ...
}

Do you have plans for implementing MessageChannel support for workers as well? It would be super awesome if you did.

Comment 39

[Olli Pettay [:smaug][[e-mail]]]

Yes, I was thinking that too yesterday, and I think we will need to have worker support too.

Comment 40

[Andrea Marchesini [:baku]]

> Do you have plans for implementing MessageChannel support for workers as
> well? It would be super awesome if you did.

Yes. It's part of the plan. I just wrote the first part about window-to-window but I'm going to implement MessagePort/MessageChannel for workers as well. My plan is to land main-thread and workers patches all together.

Comment 41

[Andrea Marchesini [:baku]]

Attachment: patch 4 - Start() method

Cycle collection and timer are implemented in the following patch. This fixes just mPort and mEvent.

Comment 42

[Andrea Marchesini [:baku]]

Attachment: patch 5 - cycle collection and timer

Here, we have cycle collection and timer, plus a test for the scenario you described:
an event listener that transfers the port.

Comment 43

[Andrea Marchesini [:baku]]

Attachment: patch 6 - unshipped message port queue

Tests for unshipped message port queue.

Comment 44

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Andrea: You rock! :)

Comment 45

[Olli Pettay [:smaug][[e-mail]]]

Yes he does!

(And I'll review the new patches tomorrow.)

Comment 46

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 796668 [details] [diff] [review]
patch 5 - cycle collection and timer


>+MessagePort::Dispatch()
>+{
>+  if (!mMessageQueueEnabled || mMessageQueue.IsEmpty() || mTimer) {
>+    return;
>+  }
> 
>-    event->Dispatch(this);
>+  nsRefPtr<PostMessageRunnable> event = mMessageQueue.ElementAt(0);
>+  mMessageQueue.RemoveElementAt(0);
>+
>+  event->Dispatch(this);
>+
>+  mTimer = do_CreateInstance("@mozilla.org/timer;1");
>+  if (mTimer) {
>+    mTimer->InitWithFuncCallback(TimerCallback, this, 0,
>+                                 nsITimer::TYPE_ONE_SHOT);
>   }
> }
Don't understand the use of timer. Also, this is unsafe use because timer isn't cancelled
when 'this' is deleted. TimerCallback can be called after 'this' is deleted and 
TimerCallback ends up accessing deleted object.
Use runnable if possible

Comment 47

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 796669 [details] [diff] [review]
patch 6 - unshipped message port queue


>+      if (expectedNumber > 102) {
>+        runTests();
>+      }
number 102 could use some comment

>+    }
>+
>+    a.port1.onmessage = function(evt) {
>+      testEvent(evt.data, 2);
>+    };
>+
>+    a.port2.onmessage = function(evt) {
>+      testEvent(evt.data, 1);
>+    };
>+
>+    b.port2.onmessage = function(evt) {
>+      testEvent(evt.data, 3);
>+    };
Could you perhaps do b.port1.addEventListener("message", function() { ok(false, "shouldn't be called")})

Comment 48

[Andrea Marchesini [:baku]]

Attachment: patch 5 - cycle collection and runnable

Is this what you mean with nsRunnable? What about the cycle collection and the runnable...?

Comment 49

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 797201 [details] [diff] [review]
patch 5 - cycle collection and runnable

>+    nsresult
>+    Run()
NS_IMETHOD Run()
                       mEntangledPort, mMessageQueue)
>+NS_IMPL_CYCLE_COLLECTION_CLASS(MessagePort)
>+
>+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(MessagePort,
>+                                                nsDOMEventTargetHelper)
>+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mEntangledPort)
>+
>+  // Custom unlink loop because this array contains nsRunnable objects
>+  // which are not cycle colleactable.
>+  while (!tmp->mMessageQueue.IsEmpty()) {
>+    ImplCycleCollectionUnlink(tmp->mMessageQueue[0]->mPort);
>+    ImplCycleCollectionUnlink(tmp->mMessageQueue[0]->mSupportsArray);
>+    tmp->mMessageQueue.RemoveElementAt(0);
>+  }
>+
>+  if (tmp->mDispatchRunnable) {
>+    ImplCycleCollectionUnlink(tmp->mDispatchRunnable->mPort);
>+  }
Could you just use NS_IMPL_CYCLE_COLLECTION_UNLINK macro for everything.
NS_IMPL_CYCLE_COLLECTION_UNLINK(mMessageQueue[0]->mPort)
and so on


>+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEntangledPort)
>+
>+  // Custom unlink loop because this array contains nsRunnable objects
>+  // which are not cycle colleactable.
>+  for (uint32_t i = 0, len = tmp->mMessageQueue.Length(); i < len; ++i) {
>+    ImplCycleCollectionTraverse(cb, tmp->mMessageQueue[i]->mPort,
>+                                "tmp->mMessageQueue[i]->mPort", 0);
>+    ImplCycleCollectionTraverse(cb, tmp->mMessageQueue[i]->mSupportsArray,
>+                                "tmp->mMessageQueue[i]->mSupportsArray", 0);
>+  }
>+
>+  if (tmp->mDispatchRunnable) {
>+    ImplCycleCollectionTraverse(cb, tmp->mDispatchRunnable->mPort,
>+                                "tmp->mDispatchRunnable-->port", 0);
s/--/-/
>+  }
Use macros for traverse?

Comment 50

[Andrea Marchesini [:baku]]

Attachment: patch 1 - WebIDL interfaces

rebased.

Comment 51

[Andrea Marchesini [:baku]]

Attachment: patch 2 - structured clone algorithm

rebased

Comment 52

[Andrea Marchesini [:baku]]

Attachment: patch 3 - postMessage

rebased + comments

Comment 53

[Andrea Marchesini [:baku]]

Attachment: patch 4 - Start() method

rebased + comments

Comment 54

[Andrea Marchesini [:baku]]

Attachment: patch 5 - cycle collection and runnable

Comment 55

[Andrea Marchesini [:baku]]

Attachment: patch 6 - unshipped message port queue

Comment 56

[Andrea Marchesini [:baku]]

Attachment: patch 7 - pref

Comment 57

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 798789 [details] [diff] [review]
patch 7 - pref

Could you use  Preferences::AddBoolVarCache for the pref
and then just return the current value in MessageChannel::PrefEnabled().
Calling GetBool all the time might show up in some odd profiles.

test_messageChannel_pref.html, please call SimpleTest.waitForExplicitFinish();
before running any tests.

I believe no need to add anything to all.js. Default for the pref should be false

Comment 58

[Andrea Marchesini [:baku]]

Attachment: patch 7 - pref

Comment 59

[Andrea Marchesini [:baku]]

After a chat with smaug, we decided to land what we have here, disabled by pref.
The worker patch of MessagePort will be a follow up.

The reason why we decided this is because: here are already have 7 patches and it's hard to keep them up-to-date.
Plus, the worker world is changing a lot lately and MessagePort/MessageChannel uses code that is still under development.

The pref will be removed only when worker and main-thread MessagePort/Channel are fully implemented.

Comment 60

[Andrea Marchesini [:baku]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3f261800748f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/00c47683de8d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/fdefdd5262d0
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/af0ceeb42bf2
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/731e32ded82e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/56da783d3638
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7c90e8e1b481

Comment 61

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3f261800748f
https://hg.mozilla.org/mozilla-central/rev/00c47683de8d
https://hg.mozilla.org/mozilla-central/rev/fdefdd5262d0
https://hg.mozilla.org/mozilla-central/rev/af0ceeb42bf2
https://hg.mozilla.org/mozilla-central/rev/731e32ded82e
https://hg.mozilla.org/mozilla-central/rev/56da783d3638
https://hg.mozilla.org/mozilla-central/rev/7c90e8e1b481

Comment 62

[SUN Haitao]

It seems that the behavior of the current implementation is not compatible with the specification (and all other engines). 

The following code snippet works fine in all other engines, but causes a DataCloneError in Gecko:
    targetWindow.postMessage({}, '*', [channel.port2]);

And the following code which works fine in Gecko, causes a DataCloneError in all other engines:
    targetWindow.postMessage({ports:[channel.port2]}, '*');

Comment 63

[Olli Pettay [:smaug][[e-mail]]]

File a followup please. This stuff isn't finalized yet (nor enabled).

Comment 64

[Andrea Marchesini [:baku]]

Yep, this is a follow up, but the spec says this:
http://www.whatwg.org/specs/web-apps/current-work/multipage/web-messaging.html#posting-messages

We should check what the other engines do and maybe file a bug to the spec... ?

Comment 65

[Olli Pettay [:smaug][[e-mail]]]

SUN Haitao, thanks for catching that bug!

Comment 66

[SUN Haitao]

I've filed a separate bug for the behavior difference. See Bug 912456.

Comment 67

[SUN Haitao]

(In reply to Olli Pettay [:smaug] from comment #65)
> SUN Haitao, thanks for catching that bug!

I'm working on a cross-engine inter-app communication framework (see https://github.com/sunhaitao/hail-them) which quite needs window-to-window message channel. Currently almost a half of all codes are used to make a hackish support for Gecko.

Comment 68

[Asa Dotzler [:asa]]

Release note could look something like "Firefox now supports HTML5's channel messaging API."  Feel free to suggest better wording if you have it. Thanks.

Comment 69

[Olli Pettay [:smaug][[e-mail]]]

No. This is all disabled by default, see comment 59.

Comment 70

[Lukas Blakk [:lsblakk] use ?needinfo]

Minusing as per comment 69, please nominate when this is getting preffed on by default.

Comment 71

[Cyril]

We have developed a near-polyfill to support MessageChannel in Firefox and IE

https://github.com/tildeio/MessageChannel.js

It is used in a library that allows to communicate with embedded untrusted content

https://github.com/tildeio/oasis.js

(In reply to SUN Haitao from comment #67)
> (In reply to Olli Pettay [:smaug] from comment #65)
> > SUN Haitao, thanks for catching that bug!
> 
> I'm working on a cross-engine inter-app communication framework (see
> https://github.com/sunhaitao/hail-them) which quite needs window-to-window
> message channel. Currently almost a half of all codes are used to make a
> hackish support for Gecko.

Comment 72

[SUN Haitao]

Even (In reply to Cyril from comment #71)
> We have developed a near-polyfill to support MessageChannel in Firefox and IE
> 
> https://github.com/tildeio/MessageChannel.js

Thanks. And I've already included a fallback solution that only use cross-document messaging. But an real implementation that works right should be much better.

> It is used in a library that allows to communicate with embedded untrusted
> content
> 
> https://github.com/tildeio/oasis.js

It seems that we are developing very similar solutions for almost the same problem: To use 'documents' as APIs in ANY modern browser engine. So anyone can introduce (many kinds of) new APIs that compatible with any engine without time consuming standardization. I believe it's important to make the Web first-class.

Comment 73

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Since this is resolved, is there any other bug that will be resolved when feature becomes pref-ed on by default ?

Comment 74

[Andrew Overholt [:overholt]]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #73)
> Since this is resolved, is there any other bug that will be resolved when
> feature becomes pref-ed on by default ?

Andrea, do we have such a bug?  I don't see one, either.

Comment 75

[Andrea Marchesini [:baku]]

This bug fixes just a part of the MessageChannel/MessagePort bug.
We still need to fix bug 912456, bug 911972 and bug 917254.