1055925 - Refactor promise to make it simpler to run async task on main and worker thread

Status: RESOLVED FIXED

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

Description

[Yuan Xulei [:yxl] (Leave from May 18)]

The promise should be resolved asynchronously and a few async tasks are created. 
With current implementation, each task needs two different classes， one for the promise running on the main thread and the other for that on the work thread.

For example, we have the pair of PromiseTask[1] and WorkerPromiseTask[2], and the pair of PromiseResolverTask[3] and WorkerPromiseResolverTask[4].

Each task pair classes do the same work except for thread specific code.

I want to strip out thread specific code from each task pair and merge each pair into one. So that we only need to create one class for each task and don't need to handle thread specific code individually for each task.

This change will faciliate bug 1035060, where we need to create new async task for abort callback.

[1] https://github.com/mozilla/gecko-dev/blob/28a6cdcb51ae87d94c054fa9d8414b919ac93afa/dom/promise/Promise.cpp#L1021
[2] https://github.com/mozilla/gecko-dev/blob/28a6cdcb51ae87d94c054fa9d8414b919ac93afa/dom/promise/Promise.cpp#L1026
[3] https://github.com/mozilla/gecko-dev/blob/28a6cdcb51ae87d94c054fa9d8414b919ac93afa/dom/promise/Promise.cpp#L1218
[4] https://github.com/mozilla/gecko-dev/blob/28a6cdcb51ae87d94c054fa9d8414b919ac93afa/dom/promise/Promise.cpp#L1224

Comment 1

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: refactor task

DispatchToMainOrWorkerThread is added to enable the task runnable of the main thread to run on the worker thread. So we don't need the worker runnable any more. 

This patch removes the worker task runnables, such as WorkerPromiseTask and WorkerPromiseResolverTask, and keeps only one runnable for each task.

Comment 2

[Yuan Xulei [:yxl] (Leave from May 18)]

Try was passed:

https://tbpl.mozilla.org/?tree=Try&rev=d17a80052515

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d17a80052515

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8475697 [details] [diff] [review]
refactor task

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

Nice patch!

::: dom/promise/Promise.cpp
@@ +939,5 @@
> +/* static */ void
> +Promise::DispatchToMainOrWorkerThread(nsIRunnable* aRunnable)
> +{
> +  MOZ_ASSERT(aRunnable);
> +  if (MOZ_LIKELY(NS_IsMainThread())) {

Could you remove MOZ_LIKELY unless there is a proven performance advantage?

Comment 4

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: refactor task

Nikhil, thanks.

MOZ_LIKELY is removed.

Try:

https://tbpl.mozilla.org/?tree=Try&rev=71abc83c2778
https://tbpl.mozilla.org/?tree=Try&rev=71abc83c2778

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7edb1ae2588f

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7edb1ae2588f