Closed
Bug 1066619
Opened 11 years ago
Closed 11 years ago
PromiseWorker should accept a Promise as a message
Categories
(Toolkit :: Async Tooling, defect)
Toolkit
Async Tooling
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(1 file, 2 obsolete files)
6.50 KB,
patch
|
Yoric
:
review+
|
Details | Diff | Splinter Review |
In some cases (e.g. bug 1009799), it would be much more convenient to be able to pass the arguments to `Scheduler.post` as a Promise.
Assignee | ||
Comment 1•11 years ago
|
||
Plus as a bonus, tests for PromiseWorker.
Assignee: nobody → dteller
Attachment #8488672 -
Flags: review?(nfroyd)
Assignee | ||
Comment 2•11 years ago
|
||
Better handling of corner cases.
Attachment #8488672 -
Attachment is obsolete: true
Attachment #8488672 -
Flags: review?(nfroyd)
Attachment #8489315 -
Flags: review?(nfroyd)
![]() |
||
Comment 3•11 years ago
|
||
Comment on attachment 8489315 [details] [diff] [review]
PromiseWorker.post accepts Promise as arguments, v2
Review of attachment 8489315 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/components/promiseworker/PromiseWorker.jsm
@@ +249,4 @@
> * - {number|null} outExecutionDuration A parameter to be filled with the
> * duration of the off main thread execution for this call.
> * @param {*=} closure An object holding references that should not be
> * garbage-collected before the message treatment is complete.
Maybe specify that this can be a promise, or an array containing promises?
::: toolkit/components/promiseworker/tests/xpcshell/test_Promise.js
@@ +11,5 @@
> +// Worker must be loaded from a chrome:// uri, not a file://
> +// uri, so we first need to load it.
> +
> +let WORKER_SOURCE_URI = "chrome://promiseworker/content/worker.js";
> +do_load_manifest("data/chrome.manifest");
I'm going to assume that the relative URL here just works with the manifest declaration.
Attachment #8489315 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Nathan Froyd (:froydnj) from comment #3)
> I'm going to assume that the relative URL here just works with the manifest
> declaration.
Yes, it does.
Assignee | ||
Comment 5•11 years ago
|
||
Trivial changes.
Try: https://tbpl.mozilla.org/?tree=Try&rev=fe83724433a6
Attachment #8489315 -
Attachment is obsolete: true
Attachment #8490647 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•