PromiseWorker should accept a Promise as a message

RESOLVED FIXED in mozilla35

Status

()

Toolkit
Async Tooling
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

(Blocks: 1 bug)

unspecified
mozilla35
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 8488672 [details] [diff] [review]
PromiseWorker.post accepts Promise as arguments

Plus as a bonus, tests for PromiseWorker.
Assignee: nobody → dteller
Attachment #8488672 - Flags: review?(nfroyd)
Created attachment 8489315 [details] [diff] [review]
PromiseWorker.post accepts Promise as arguments, v2

Better handling of corner cases.
Attachment #8488672 - Attachment is obsolete: true
Attachment #8488672 - Flags: review?(nfroyd)
Attachment #8489315 - Flags: review?(nfroyd)
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+
(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.
Created attachment 8490647 [details] [diff] [review]
PromiseWorker.post accepts Promise as arguments, v3

Trivial changes.
Try: https://tbpl.mozilla.org/?tree=Try&rev=fe83724433a6
Attachment #8489315 - Attachment is obsolete: true
Attachment #8490647 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/422d828c179e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/422d828c179e
Status: NEW → RESOLVED
Last Resolved: 4 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.