Closed Bug 1633726 Opened 5 years ago Closed 4 years ago

Remove EnigmailLazy and use XPCOMUtils.defineLazyGetter instead

Categories

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

Product:
MailNews Core
Component:
Security: OpenPGP
Type:
task

Tracking

(thunderbird_esr78 wontfix)

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: KaiE, Assigned: lasana)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

bug1633726v3.patch
(deleted), patch
mkmelin
: review+
Details | Diff | Splinter Review

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) (deleted) — — 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) (deleted) — — 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)
Attached patch bug1633726v3.patch (deleted) — — Splinter Review

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+
status-thunderbird_esr78: --- → wontfix
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: 4 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: