1434737 - Move to ChromeUtils.import()

Status: RESOLVED FIXED

(MailNews Core :: Backend, enhancement)

Description

[Philipp Kewisch [:Fallen][📱📆]]

Cu.import is no more, long live ChromeUtils.import(). Changing this also fixes the eslint checks.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Calendar Part - v1

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ac6471bed232f88e010ccc418f58fc3cbfcf4a8a

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Seamonkey Part - v1

Totally untested. Also removes XPCOMUtils import if there was no further occurrence in that file. This might break if other files rely on it being imported implicitly, but I'll leave that to you to check.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: IM/Chat - v1

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Mail/Common/Ldap/Editor - v1

Comment 5

[Florian Quèze [:florian]]

Comment on attachment 8947261 [details] [diff] [review]
IM/Chat - v1

I have not tested this, but looks great by code inspection. Thanks!

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Philipp Kewisch [:Fallen]  from comment #0)
> Cu.import is no more, long live ChromeUtils.import().
Can you please clarify? When/where will this be withdrawn? The Mozmill part of your try is all orange, so something isn't right yet.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Not sure when it will be withdrawn, but I noticed due to the eslint rule. I'll look into the mozmill tests for calendar soon.

Note also the remaining patches are untested, and the try run is calendar only because I only later decided to go ahead with the other patches.

Esentially this is s/(Components.utils|Cu).import(/ChromeUtils.import(/g and s/XPCOMUtils.defineLazyModuleGetter/ChromeUtils.defineModuleGetter/g plus some indent changes and removing some imports. If you want to be on the safe side, we could keep the imports.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

I likely fixed the ChromeUtils in Mozmill part with this try run for the calendar patch: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=949a144da49bdeed43bd6d14ecf1e0a8ca8bb362

Comment 9

[Jorg K (CEST = GMT+2)]

Sorry, caught in bustage, now it's ready to go again.

> Note also the remaining patches are untested, and the try run is calendar only
> because I only later decided to go ahead with the other patches.
So can you please do a try for all patches.

Comment 10

[Frank-Rainer Grahl (:frg)]

Thanks saw the underlying change yesterday and thought we should follow. You beat me to it :)

> So can you please do a try for all patches.

SeaMonkey tests are busted. I will need to check the removed imports. Hope I manage this weekend.

Comment 11

[[:MakeMyDay]]

What's the status here? Comment 8 suggests that you will update the patch, right?

Comment 12

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8947252 [details] [diff] [review]
Seamonkey Part - v1

Need to be SeaMonkey instead of Seamonkey in comment but otherwise r+. Thanks

Let me check it in personally because it might need a rebase.

Next stop bug 767640 followup for c-c I assume :)

Comment 13

[[:MakeMyDay]]

Comment on attachment 8947242 [details] [diff] [review]
Calendar Part - v1

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

I just noticed that you only added two changes in mail in your last push, so codewise it looks good for calendar, r=me.

Comment 14

[Philipp Kewisch [:Fallen][📱📆]]

Here is a push that I believe will work, I forgot one more Sandbox change. I'm splitting the sandbox change out into a separate patch as well.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=86c0ef1d2d0f4f1eaa807bbaffa9e602380ef512

Comment 15

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Mozmill Sandbox Changes - v1

Feel free to delegate to e.g. Magnus if you don't want to review this.

Comment 16

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Calendar Part - v2

Comment 17

[Jorg K (CEST = GMT+2)]

Comment on attachment 8948278 [details] [diff] [review]
Mozmill Sandbox Changes - v1

Usually I refer all matters related to Mozmill to Aceman. I'm also happy to rs this as long as it works. I'll check the try later. Thanks for taking this bug!

Comment 18

[:aceman]

Comment on attachment 8948278 [details] [diff] [review]
Mozmill Sandbox Changes - v1

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

Thanks, the try run seems fine.
This only changes some sandbox calls. When do we migrate all normal Cu.import() calls throughout c-c?

Also, can we have the bug number from m-c where they decide on dropping Cu.import() ?

Comment 19

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #18)
> This only changes some sandbox calls. When do we migrate all normal
> Cu.import() calls throughout c-c?
That's done in all the other patches in the bug ;-)

Comment 20

[Jorg K (CEST = GMT+2)]

Comment on attachment 8947262 [details] [diff] [review]
Mail/Common/Ldap/Editor - v1

Thanks. I'll land the whole lot now before it rots.

Comment 21

[Frank-Rainer Grahl (:frg)]

> Thanks. I'll land the whole lot now before it rots.

Let me checkin the SeaMonkey part. Will do it tomorrow. Suite building is broken and waiting for an r+ wich will bitrot the fix or the patch whatever is checked in first.

Comment 22

[Jorg K (CEST = GMT+2)]

Attachment: Seamonkey Part - v1 - rebased

The rot set in already. This is the SM part rebased.

Philipp, the calendar part also doesn't apply. For example in one failed hunk I see:
this.mProperties = new cal.data.PropertyMap();
when the current code is:
this.mProperties = new cal.calPropertyBag();

That change happened a while ago, so I don't know what's going on there.

Comment 23

[Jorg K (CEST = GMT+2)]

Can you check in the parts separately? Why did Mozmill fail then?

Comment 24

[Philipp Kewisch [:Fallen][📱📆]]

Ah, sorry. This was on top of bug 1433802 which I haven't sent an updated patch for. I've done that now, then you should be able to land everything correctly.

Comment 25

[Jorg K (CEST = GMT+2)]

Yep, I've just worked that out myself ;-)

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ac48db6177e
Move to ChromeUtils.import() - Chat part. r=florian
https://hg.mozilla.org/comm-central/rev/932a8eee8f8c
Move to ChromeUtils.import() - Mail/Common/Ldap/Editor part. r=jorgk
https://hg.mozilla.org/comm-central/rev/f4ec71bab936
Move to ChromeUtils.import() - Mozmill Sandbox changes. r=aceman
https://hg.mozilla.org/comm-central/rev/0250e2262e3c
Move to ChromeUtils.import() - Calendar part. r=MakeMyDay

Comment 27

[Jorg K (CEST = GMT+2)]

The target should really be TB 60.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/87ca394913b1
Move to ChromeUtils.import() - SeaMonkey part. r=frg

Comment 29

[:aceman]

Fallen, can we change the Product and Component of the bug as this now covered all of c-c not just calendar?