1374282 - Switch to async/await from Task.jsm/yield - part 2

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, enhancement, P1)

Description

[Florian Quèze [:florian]]

Bug 1353542 converted browser/ and toolkit/, but there are plenty of uses of Task.jsm in the rest of the tree.

This bug will covert the rest of the tree except mobile/ and devtools/.

Comment 1

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2faf6953a8733c15e755b0e86dc5a21a96eabf19

Comment 2

[Florian Quèze [:florian]]

Attachment: task.jsm.js xpcshell script

This is the script I used to generate the main patch I'll attach here.
I changed the heuristic a bit to make better automated decisions about whether a generator is a task or not. It now takes into account the presence of ".next(" and "Task." in the file to help decide, so I can no longer produce separate patches for the replacements where the script is 100% confident and for where it's a bit speculative. However, given that there are few non-test files involved here, I don't think it'll be a problem for reviews.

Comment 3

[Florian Quèze [:florian]]

Attachment: script generated patch to remove Task.jsm calls

Here are the non-test files affected by this patch:
 addon-sdk/source/lib/sdk/system/child_process/subprocess.js                                            |   15 +-
 dom/browser-element/BrowserElementChildPreload.js                                                      |    7 +-
 dom/ipc/manifestMessages.js                                                                            |   13 +-
 dom/manifest/ManifestFinder.jsm                                                                        |    7 +-
 dom/manifest/ManifestObtainer.jsm                                                                      |   28 +-
 dom/push/PushRecord.jsm                                                                                |   12 +-
 uriloader/exthandler/nsHandlerService-json.js                                                          |    7 +-

Comment 4

[Florian Quèze [:florian]]

Attachment: hand cleanup for the script generated patch to remove Task.jsm calls

Comment 5

[Florian Quèze [:florian]]

Attachment: script generated patch to remove Promise.defer calls

Comment 6

[Florian Quèze [:florian]]

Attachment: script generated patch to remove useless bind calls

Comment 7

[Florian Quèze [:florian]]

Attachment: script generated patch to remove generators from add_task in browser/ and toolkit/

The script in bug 1353542 missed the generators in add_task that didn't contain any yield. Let's clean this up!

Comment 8

[Florian Quèze [:florian]]

Attachment: ban Task.jsm during startup

The few pieces of toolkit code that still need to support Task.jsm can be adapted to load it lazily only when really needed, and then we can ban the Task.jsm module during startup using the browser_startup.js test.

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8879284 [details] [diff] [review]
ban Task.jsm during startup

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

::: toolkit/modules/Sqlite.jsm
@@ +592,5 @@
>              // Keep Task.spawn here to preserve API compat; unfortunately
>              // func was a generator rather than a task here.
> +            let isGenerator = func =>
> +              Object.prototype.toString.call(func) == "[object Generator]";
> +            if (isGenerator(func)) {

paolo pointed out that I don't need this first test that should always be false. Next time I regenerate the patch I'll simplify this block, and I'll definitely do a full try run again before landing.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8879279 [details] [diff] [review]
script generated patch to remove Promise.defer calls

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

Can I see a diff -w of this?

Comment 11

[Florian Quèze [:florian]]

Attachment: Diff -w of attachment 8879279

(In reply to Dave Townsend [:mossop] from comment #10)
> Comment on attachment 8879279 [details] [diff] [review]
> script generated patch to remove Promise.defer calls
> 
> Review of attachment 8879279 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can I see a diff -w of this?

There you go! Thanks for the other reviews :-).

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/b0dd91a6a1fd
script generated patch to remove Task.jsm calls, r=Mossop.
https://hg.mozilla.org/mozilla-central/rev/9c7644b6de3c
hand cleanup for the script generated patch to remove Task.jsm calls, r=Mossop.
https://hg.mozilla.org/mozilla-central/rev/52e7befb507a
script generated patch to remove Promise.defer calls, r=Mossop.
https://hg.mozilla.org/mozilla-central/rev/a65201e69ebf
script generated patch to remove useless bind calls, r=Mossop.
https://hg.mozilla.org/mozilla-central/rev/05d686d3c53d
script generated patch to remove generators from add_task in browser/ and toolkit/, r=Mossop.
https://hg.mozilla.org/mozilla-central/rev/7a9536f89bc7
ban Task.jsm during startup, r=Mossop, a=sheriffduty

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b0dd91a6a1fd
https://hg.mozilla.org/mozilla-central/rev/9c7644b6de3c
https://hg.mozilla.org/mozilla-central/rev/52e7befb507a
https://hg.mozilla.org/mozilla-central/rev/a65201e69ebf
https://hg.mozilla.org/mozilla-central/rev/05d686d3c53d
https://hg.mozilla.org/mozilla-central/rev/7a9536f89bc7