Closed Bug 1204775 Opened 5 years ago Closed 5 years ago

SharedWorker.port should be a 'real' MessagePort.

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch mp.patch (obsolete) — Splinter Review
No description provided.
Attachment #8661056 - Flags: review?(khuey)
Comment on attachment 8661056 [details] [diff] [review]
mp.patch

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

::: dom/base/StructuredCloneHelper.h
@@ +190,5 @@
>  
>    // This must be called if the transferring has ports generated by Read().
>    // MessagePorts are not thread-safe and they must be retrieved in the thread
>    // where they are created.
> +  void TakeTransferredPorts(nsTArray<nsRefPtr<MessagePort>>& aPorts)

While we're here, make this take a move reference.

@@ +195,4 @@
>    {
>      MOZ_ASSERT(mSupportsTransferring);
>      MOZ_ASSERT(aPorts.IsEmpty());
>      aPorts.SwapElements(mTransferredPorts);

And then just assign here.

::: dom/messagechannel/MessagePort.h
@@ +60,5 @@
>  
>    virtual void
>    PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
>                const Optional<Sequence<JS::Value>>& aTransferable,
> +              ErrorResult& aRv);

Why are any of these functions still virtual?

::: dom/workers/ServiceWorkerRegistrar.cpp
@@ +10,5 @@
>  #include "nsIEventTarget.h"
>  #include "nsIInputStream.h"
>  #include "nsILineInputStream.h"
>  #include "nsIObserverService.h"
> +#include "nsIOutputStream.h"

...?

::: dom/workers/WorkerPrivate.cpp
@@ +1503,5 @@
> +                      MessagePort* aPort)
> +  : WorkerRunnable(aWorkerPrivate, WorkerThreadModifyBusyCount)
> +  {
> +    MOZ_ASSERT(aPort);
> +    // In order to move the port from 1 thread to the another one, we have to

s/1/one/ and then "the another" should become "the other" or "another"
Attachment #8661056 - Flags: review?(khuey) → review+
Keywords: checkin-needed
Except that mulet m(4) IS from this patch, so it's backed out too:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ad7f8d8c042
Flags: needinfo?(amarchesini)
Blocks: 1193414
https://hg.mozilla.org/mozilla-central/rev/faf94ffc0c5a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
I think this is responsible for the spike in both bug 1189886 and bug 1168411.
Blocks: 1189886, 1168411
Flags: needinfo?(amarchesini)
Yes, it is and I'm actively working on it.
Flags: needinfo?(amarchesini)
Duplicate of this bug: 667501
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.