1368456 - [meta] Remove uses of Promise.jsm from Firefox

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, enhancement, P1)

Description

[:Paolo Amadini]

Now that bug 939636 is resolved, most imports of the Promise.jsm module can already be removed. Because some callers still use Promise.defer(), bug 1034599 should be fixed before all instances can be removed.

Comment 1

[Florian Quèze [:florian]]

In case anybody needs to be convinced that this is related to performance, see this profile captured at a random time on a slow netbook: https://perf-html.io/public/4799687e95491a7258e702308f81cfb01b3c18cd/calltree/?implementation=js&invertCallstack&thread=0
See how much time is being spent within Promise-backend.js.

Comment 2

[Florian Quèze [:florian]]

Attachment: script generated patch

This excludes files in a /test/ or /tests/ folder, or files from testing/.

Comment 3

[Florian Quèze [:florian]]

Attachment: promise.jsm.js xpcshell script

Comment 4

[Florian Quèze [:florian]]

Attachment: blacklist Promise.jsm before first paint

We can now blacklist Promise.jsm before first paint to avoid regressions.
We unfortunately can't blacklist it completely from startup because http://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/devtools/shared/builtin-modules.js#18 still imports it during delayed startup, and I don't feel like fighting that devtools code to make it lazy... especially when all of this is going to move to an add-on soon.

Comment 5

[Florian Quèze [:florian]]

Editing the summary to match what I'm doing here.

Try seems green-ish: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b5e36f64ce03327016be15f53c97035bdf3fadc

And Talos seems happy: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=16f96f3cda75b5d312c62e7627aed240c2b5ac96&newProject=try&newRevision=8b5e36f64ce03327016be15f53c97035bdf3fadc&framework=1

Comment 6

[Florian Quèze [:florian]]

Attachment: remove Promise.jsm imports in tests

This is an attempt to also cleanup tests where it's easy. If it's too hard to have a green run with this, it can be postponed to a follow-up.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=58c5d52d708102b3c54c7554865770dab7563c3f

Comment 7

[Florian Quèze [:florian]]

Attachment: remove Promise.jsm imports in tests v2

Fixed test failures from the previous try push, new try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=804660f53717455a0bbe9595af98545d038f6eea

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Feel free to set me as the reviewer if paolo is busy / overloaded.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8880187 [details] [diff] [review]
script generated patch

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

Looks fine - though I didn't check to see if this was exhaustive (nor that there were no remaining usages of Promise.jsm stuff in any of these files...)

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8880190 [details] [diff] [review]
blacklist Promise.jsm before first paint

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

\o/

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8880474 [details] [diff] [review]
remove Promise.jsm imports in tests v2

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

As before, I didn't actually check to see if there were any remaining uses of Promise.jsm in these folders with this patch, but the changes themselves look good.

You probably already had this in mind, but probably it'd be a good idea to send mail to firefox-dev / dev-platform and let them know that Promise.jsm is no longer to be used.

Great job!

::: toolkit/components/osfile/tests/xpcshell/test_creationDate.js
@@ -9,5 @@
>   * Test to ensure that deprecation warning is issued on use
>   * of creationDate.
>   */
>  add_task(async function test_deprecatedCreationDate () {
> -  let deferred = Promise.defer();

So, I guess nothing was using deferred.promise? Weird. I wonder if this will fix some orange!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c7e4d4547b7
Remove imports of Promise.jsm from Firefox, r=mconley.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1b1c441ed4c
blacklist Promise.jsm before first paint, r=mconley.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d121d4c2033
remove Promise.jsm imports in tests, r=mconley.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9c7e4d4547b7
https://hg.mozilla.org/mozilla-central/rev/e1b1c441ed4c
https://hg.mozilla.org/mozilla-central/rev/6d121d4c2033