The default bug view has changed. See this FAQ.

(MessageChannel) Implement HTML5's channel messaging API

RESOLVED FIXED in mozilla26

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: James Leigh, Assigned: baku)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

Trunk
mozilla26
dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(relnote-firefox -)

Details

Attachments

(7 attachments, 15 obsolete attachments)

12.64 KB, patch
Details | Diff | Splinter Review
9.85 KB, patch
Details | Diff | Splinter Review
16.15 KB, patch
Details | Diff | Splinter Review
16.42 KB, patch
Details | Diff | Splinter Review
9.55 KB, patch
Details | Diff | Splinter Review
3.96 KB, patch
Details | Diff | Splinter Review
21.56 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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.
Use postMessage?

Just curious,
do you have some real world use case for MessageChannel's ports?
OS: Linux → All
Hardware: x86 → All
Version: unspecified → Trunk
(Reporter)

Comment 2

6 years ago
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.
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

6 years ago
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).
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

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

Updated

5 years ago
Depends on: 741618

Comment 6

5 years ago
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

5 years ago
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

4 years ago
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.
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
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.
Keywords: dev-doc-needed
I hope to be able to get (part of) this feature implemented as a side effect of bug 876397.
baku will likely have time when he gets back from vacation
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.
Assignee: nobody → amarchesini
(Assignee)

Comment 14

4 years ago
(In reply to Johnny Stenback (:jst, jst@mozilla.com) 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.
Please make sure you look at bug 907060, we need to make MessagePort objects pretty flexible.
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.
(Assignee)

Comment 17

4 years ago
Created attachment 794588 [details] [diff] [review]
WIP 1 - Interfaces
(Assignee)

Comment 18

4 years ago
Created attachment 794589 [details] [diff] [review]
WIP 2 - Structured Clone Algorithm
Attachment #794589 - Flags: feedback?(bugs)
(Assignee)

Comment 19

4 years ago
Created attachment 794590 [details] [diff] [review]
WIP 3 - PostMessage
Attachment #794590 - Flags: feedback?(bugs)
(Assignee)

Comment 20

4 years ago
Created attachment 794591 [details] [diff] [review]
WIP 3 - PostMessage
Attachment #794590 - Attachment is obsolete: true
Attachment #794590 - Flags: feedback?(bugs)
Attachment #794591 - Flags: feedback?(bugs)
Attachment #794589 - Flags: feedback?(bugs) → feedback+
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.
Attachment #794591 - Flags: feedback?(bugs) → feedback-
(Assignee)

Comment 22

4 years ago
Created attachment 795513 [details] [diff] [review]
patch 1 - WebIDL interfaces
Attachment #794588 - Attachment is obsolete: true
Attachment #795513 - Flags: review?(bugs)
(Assignee)

Comment 23

4 years ago
Created attachment 795515 [details] [diff] [review]
patch 2 - structured clone algorithm

This second patch makes MessagePort transferable via window.postMessage.
Attachment #794589 - Attachment is obsolete: true
Attachment #795515 - Flags: review?(bugs)
(Assignee)

Comment 24

4 years ago
Created attachment 795516 [details] [diff] [review]
patch 3 - postMessage

Implementation of postMessage in MessagePort
Attachment #794591 - Attachment is obsolete: true
Attachment #795516 - Flags: review?(bugs)
(Assignee)

Comment 25

4 years ago
Created attachment 795518 [details] [diff] [review]
patch 4 - Start() method

This patch implements start() method and the queue of events.
Attachment #795518 - Flags: review?(bugs)
(Assignee)

Comment 26

4 years ago
Created attachment 795968 [details] [diff] [review]
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.
Attachment #795968 - Flags: review?(bugs)
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 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;
(Assignee)

Comment 29

4 years ago
Created attachment 795997 [details] [diff] [review]
patch 3 - postMessage
Attachment #795516 - Attachment is obsolete: true
Attachment #795516 - Flags: review?(bugs)
Attachment #795997 - Flags: review?(bugs)

Updated

4 years ago
No longer depends on: 741618
Duplicate of this bug: 741618
Note that you want to avid JSVAL_* no matter what.  Use JS::UndefinedValue() instead if you need that value.
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
Attachment #795513 - Flags: review?(bugs) → review+
Comment on attachment 795515 [details] [diff] [review]
patch 2 - structured clone algorithm

PostMessageRead/WriteStructuredClone will need some clean ups at some point
Attachment #795515 - Flags: review?(bugs) → review+
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 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.
Attachment #795968 - Flags: review?(bugs) → review-
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...
Attachment #795997 - Flags: review?(bugs) → review+
Comment on attachment 795518 [details] [diff] [review]
patch 4 - Start() method

># HG changeset patch
># Parent a4af4ae5eae7963cf16c769fca696688389947b3
># User Andrea Marchesini <amarchesini@mozilla.com>
>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.
Attachment #795518 - Flags: review?(bugs) → review-
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.
Yes, I was thinking that too yesterday, and I think we will need to have worker support too.
(Assignee)

Comment 40

4 years ago
> 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.
(Assignee)

Comment 41

4 years ago
Created attachment 796666 [details] [diff] [review]
patch 4 - Start() method

Cycle collection and timer are implemented in the following patch. This fixes just mPort and mEvent.
Attachment #795518 - Attachment is obsolete: true
Attachment #796666 - Flags: review?(bugs)
(Assignee)

Comment 42

4 years ago
Created attachment 796668 [details] [diff] [review]
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.
Attachment #796668 - Flags: review?(bugs)
(Assignee)

Comment 43

4 years ago
Created attachment 796669 [details] [diff] [review]
patch 6 - unshipped message port queue

Tests for unshipped message port queue.
Attachment #795968 - Attachment is obsolete: true
Attachment #796669 - Flags: review?(bugs)
Andrea: You rock! :)
Yes he does!

(And I'll review the new patches tomorrow.)
Attachment #796666 - Flags: review?(bugs) → review+
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
Attachment #796668 - Flags: review?(bugs) → review-
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")})
Attachment #796669 - Flags: review?(bugs) → review+
(Assignee)

Comment 48

4 years ago
Created attachment 797201 [details] [diff] [review]
patch 5 - cycle collection and runnable

Is this what you mean with nsRunnable? What about the cycle collection and the runnable...?
Attachment #796668 - Attachment is obsolete: true
Attachment #797201 - Flags: review?(bugs)
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?
Attachment #797201 - Flags: review?(bugs) → review+
(Assignee)

Comment 50

4 years ago
Created attachment 797544 [details] [diff] [review]
patch 1 - WebIDL interfaces

rebased.
Attachment #795513 - Attachment is obsolete: true
(Assignee)

Comment 51

4 years ago
Created attachment 797546 [details] [diff] [review]
patch 2 - structured clone algorithm

rebased
Attachment #795515 - Attachment is obsolete: true
(Assignee)

Comment 52

4 years ago
Created attachment 797547 [details] [diff] [review]
patch 3 - postMessage

rebased + comments
Attachment #795997 - Attachment is obsolete: true
(Assignee)

Comment 53

4 years ago
Created attachment 797548 [details] [diff] [review]
patch 4 - Start() method

rebased + comments
Attachment #796666 - Attachment is obsolete: true
(Assignee)

Comment 54

4 years ago
Created attachment 797550 [details] [diff] [review]
patch 5 - cycle collection and runnable
Attachment #797201 - Attachment is obsolete: true
(Assignee)

Comment 55

4 years ago
Created attachment 797551 [details] [diff] [review]
patch 6 - unshipped message port queue
Attachment #796669 - Attachment is obsolete: true
(Assignee)

Comment 56

4 years ago
Created attachment 798789 [details] [diff] [review]
patch 7 - pref
Attachment #798789 - Flags: review?(bugs)
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
Attachment #798789 - Flags: review?(bugs) → review+
(Assignee)

Comment 58

4 years ago
Created attachment 798794 [details] [diff] [review]
patch 7 - pref
Attachment #798789 - Attachment is obsolete: true
(Assignee)

Comment 59

4 years ago
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.
(Assignee)

Updated

4 years ago
Blocks: 911972
(Assignee)

Comment 60

4 years ago
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
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
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26

Comment 62

4 years ago
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]}, '*');
File a followup please. This stuff isn't finalized yet (nor enabled).
(Assignee)

Comment 64

4 years ago
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... ?
SUN Haitao, thanks for catching that bug!

Comment 66

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

Comment 67

4 years ago
(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.
Blocks: 914430

Updated

4 years ago
Blocks: 914974

Comment 68

4 years ago
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.
relnote-firefox: --- → ?
No. This is all disabled by default, see comment 59.
Minusing as per comment 69, please nominate when this is getting preffed on by default.
relnote-firefox: ? → -

Comment 71

4 years ago
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

4 years ago
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.
Since this is resolved, is there any other bug that will be resolved when feature becomes pref-ed on by default ?
(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.
Flags: needinfo?(amarchesini)
(Assignee)

Updated

3 years ago
Blocks: 952139
(Assignee)

Comment 75

3 years ago
This bug fixes just a part of the MessageChannel/MessagePort bug.
We still need to fix bug 912456, bug 911972 and bug 917254.
Flags: needinfo?(amarchesini)
Depends on: 983928
Blocks: 983928
No longer depends on: 983928
Depends on: 984990
Depends on: 984991
Depends on: 1140776
You need to log in before you can comment on or make changes to this bug.