1388403 - Remove uses of Promise.jsm from Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

According to bug 1378173 comment #9 we can just remove Promise.jsm imports, so all of these in C-C:
https://dxr.mozilla.org/comm-central/search?q=regexp%3Aimport.*promise.jsm&redirect=false

Let's to this before M-C remove Promise.jsm

We should do a try run after that.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: removePromise.patch

Removed all uses of promise.jsm in C-C. The most in calendar, then in mailnews. One was in suite in a test. Also removed it.

Comment 2

[Richard Marti (:Paenglab)]

Try build: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=17e47ccf07990762b9ec6b69948381ba97be9439

Comment 3

[Jorg K (CEST = GMT+2)]

You should have run the Xpcshell tests as well.

Comment 4

[Richard Marti (:Paenglab)]

Try with xpcshell: https://hg.mozilla.org/try-comm-central/rev/1f0a5bf3abc22e5ecb98f95ae2368e86a350fadc

Comment 5

[Jorg K (CEST = GMT+2)]

No need to repeat Mozmill :-(, but thanks anyway.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8896242 [details] [diff] [review]
removePromise.patch

Thanks, this is what I asked for. We need to land this before M-C remove Promise.jsm.

Comment 7

[Jorg K (CEST = GMT+2)]

More precisely: We need to land this before M-C remove Promise.jsm in bug 1378173.

Comment 8

[Florian Quèze [:florian]]

Comment on attachment 8896242 [details] [diff] [review]
removePromise.patch

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

::: suite/common/places/tests/head_common.js
@@ -29,5 @@
>                                    "resource://gre/modules/FileUtils.jsm");
>  XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
>                                    "resource://gre/modules/NetUtil.jsm");
> -XPCOMUtils.defineLazyModuleGetter(this, "Promise",
> -                                  "resource://gre/modules/Promise.jsm");

I would revert this change, as some suite tests do seem to use Promise.defer: http://searchfox.org/comm-central/search?q=Promise.defer&case=true&path=suite

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: removePromise.patch (v2).

Removed suite/ hunk, thanks Florian!

Comment 11

[:aceman]

(In reply to Florian Quèze [:florian] [:flo] from comment #8)
> ::: suite/common/places/tests/head_common.js
> @@ -29,5 @@
> >                                    "resource://gre/modules/FileUtils.jsm");
> >  XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
> >                                    "resource://gre/modules/NetUtil.jsm");
> > -XPCOMUtils.defineLazyModuleGetter(this, "Promise",
> > -                                  "resource://gre/modules/Promise.jsm");
> 
> I would revert this change, as some suite tests do seem to use
> Promise.defer:
> http://searchfox.org/comm-central/search?q=Promise.defer&case=true&path=suite

How can you keep loading Promise.jsm if the file is going away (unless I misunderstand the bug) ?
In bug 1378173 there is mention that you can use PromiseUtils.defer() instead of Promise.defer() so I'd think this line should be changed to loading PromiseUtils.jsm instead of Promise.jsm and adapt the calls.

Comment 12

[Jorg K (CEST = GMT+2)]

If we land the patch now with the hunk for SM, we will break SM. If Promise.jsm goes away and we then land the patch, it doesn't matter, since SM will be broken either way. Maybe best not touch SM in this case so that they can use the appropriate replacement.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/421dc4935bc3
Remove uses of Promise.jsm. r=philipp,jorgk

Comment 14

[Jorg K (CEST = GMT+2)]

FRG, you need to take some action here for SM. Replace Promise.jsm with PromiseUtils.jsm and check the usage of .defer.

Comment 15

[Frank-Rainer Grahl (:frg)]

Tests in SeaMonkey are broken for a long time. ewong is now trying to get them going again. Seems to be only one occurrence in tests so I will just let it stay in for now. Fixing all the broken tests later will be fun... not :)