Closed
Bug 1204775
Opened 7 years ago
Closed 7 years ago
SharedWorker.port should be a 'real' MessagePort.
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(1 file, 1 obsolete file)
81.91 KB,
patch
|
Details | Diff | 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+
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b663b3a5bd2c
Attachment #8661056 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 3•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccfddcbccdac
Keywords: checkin-needed
I had to back this out for various bustages: https://hg.mozilla.org/integration/mozilla-inbound/rev/27cdeffa4ae1 https://treeherder.mozilla.org/logviewer.html#?job_id=14146579&repo=mozilla-inbound https://treeherder.mozilla.org/logviewer.html#?job_id=14146100&repo=mozilla-inbound
Flags: needinfo?(amarchesini)
(In reply to Wes Kocher (:KWierso) from comment #4) > I had to back this out for various bustages: > https://hg.mozilla.org/integration/mozilla-inbound/rev/27cdeffa4ae1 > > https://treeherder.mozilla.org/logviewer.html#?job_id=14146579&repo=mozilla- > inbound > https://treeherder.mozilla.org/logviewer.html#?job_id=14146100&repo=mozilla- > inbound Oops, this was supposed to go to bug 1203463.
Flags: needinfo?(amarchesini)
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)
Assignee | ||
Comment 7•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/faf94ffc0c5a
Flags: needinfo?(amarchesini)
https://hg.mozilla.org/mozilla-central/rev/faf94ffc0c5a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
I think this is responsible for the spike in both bug 1189886 and bug 1168411.
Assignee | ||
Comment 10•7 years ago
|
||
Yes, it is and I'm actively working on it.
Flags: needinfo?(amarchesini)
Updated•3 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•