Closed Bug 1633726 Opened 4 years ago Closed 3 years ago

Remove EnigmailLazy and use XPCOMUtils.defineLazyGetter instead

Categories

(MailNews Core :: Security: OpenPGP, task, P3)

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
86 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: KaiE, Assigned: lasana)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Magnus suggested we should remove the old EnigmailLazy code, and use XPCOMUtils.defineLazyGetter

Blocks: 1595318

Hmm. Looks like what it does is not really lazy loading though. https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/modules/lazy.jsm#11

A homegrown attempt that aims to only load the module once? But modules are always just loaded once. Maybe it was a workaround from the past? Or am I misreading?

(In reply to Magnus Melin [:mkmelin] from comment #1)

Hmm. Looks like what it does is not really lazy loading though. https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/modules/lazy.jsm#11

A homegrown attempt that aims to only load the module once? But modules are always just loaded once. Maybe it was a workaround from the past? Or am I misreading?

It returns a function, so that is lazy loading.

However, it would still be better to use defineLazyModuleGetters & friends (note: bug 1664697 is doing a lot of module loading rewrite, but is not addressing this specific issue).

Depends on: 1664697
Assignee: nobody → lasana
Priority: -- → P3
Status: NEW → ASSIGNED
Attached patch bug1633726.patch (obsolete) — Splinter Review

EnigmailLazy removed. I tested this using the existing openpgp tests and the ones coming in bug 1673652 (needs to be updated for this).

Attachment #9194935 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9194935 [details] [diff] [review]
bug1633726.patch

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

::: mail/extensions/openpgp/content/BondOpenPGP.jsm
@@ +24,2 @@
>    MailConstants: "resource:///modules/MailConstants.jsm",
>    Services: "resource://gre/modules/Services.jsm",

Let's keep Services, and later in this patch MailServices as normal imports.
They are going to be initialized always before this module anyway, so no need to lazy it.
Attachment #9194935 - Flags: review?(mkmelin+mozilla) → review+
Attached patch bug1633726v1.patch (obsolete) — Splinter Review

Requested updates made.

Attachment #9194935 - Attachment is obsolete: true
Attachment #9194945 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9194945 [details] [diff] [review]
bug1633726v1.patch

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

::: mail/extensions/openpgp/content/BondOpenPGP.jsm
@@ +24,2 @@
>    MailConstants: "resource:///modules/MailConstants.jsm",
>    Services: "resource://gre/modules/Services.jsm",

still here

::: mail/extensions/openpgp/content/modules/app.jsm
@@ +14,4 @@
>  
> +XPCOMUtils.defineLazyModuleGetters(this, {
> +  EnigmailLog: "chrome://openpgp/content/modules/log.jsm",
> +  Services: "resource://gre/modules/Services.jsm",

and here too
Attachment #9194945 - Flags: review?(mkmelin+mozilla)

Fixed the ones I missed.

Attachment #9194945 - Attachment is obsolete: true
Attachment #9194947 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9194947 [details] [diff] [review]
bug1633726v3.patch

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

Thx, r=mkmelin
Attachment #9194947 - Flags: review?(mkmelin+mozilla) → review+
Target Milestone: --- → 86 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/a4c5a82aedf8
Replace EnigmailLazy with XPCOM.defineLazyGetter(). r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: