879088 - Use defineLazyModuleGetter for lazy modules

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

In a bunch of places we use XPCOMUtils.defineLazyGetter to load a module. We should use defineLazyModuleGetter, since it's designed for that.

To fix this, we should look at all the places we use defineLazyGetter:
http://mxr.mozilla.org/mozilla-central/search?find=/mobile/android/&string=definelazygetter

And if it's used to load a module, e.g.:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#24

We should replace it with defineLazyModuleGetter, e.g.:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#34

Comment 1

[Nicolas Carlo [:nickecarlo]]

I can work on this bug.

Comment 2

[Nicolas Carlo [:nickecarlo]]

Margaret,

I'm almost done with this bug but I can't figure out how I should approach the following two instances:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#43

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#60

Everything else was straight forward but the above two declare a variable in the lamda function which is then returned. Couldn't quite figure out from the documentation how to approach this situation. For now, I've left these two alone.

Also, I've only changed those instances of defineLazyGetter to defineLazyModuleGetter that were used to load modules. There were others that I left untouched. So its only browser.js, AddonUpdateService.js, and SessionStore.js that had relevant pieces of code that needed changing.

Once you advise me on how to tackle the above two situations then I can submit the patch.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nicolas Carlo from comment #2)
> Margaret,
> 
> I'm almost done with this bug but I can't figure out how I should approach
> the following two instances:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#43
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#60
> 
> Everything else was straight forward but the above two declare a variable in
> the lamda function which is then returned. Couldn't quite figure out from
> the documentation how to approach this situation. For now, I've left these
> two alone.

Actually, you can safely change those two as well. Look at the definition of defineLazyModuleGetter:
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/loader/XPCOMUtils.jsm#220

It uses the same pattern almost exactly.

Comment 4

[Nicolas Carlo [:nickecarlo]]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> Actually, you can safely change those two as well. Look at the definition of
> defineLazyModuleGetter:
> http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/loader/XPCOMUtils.
> jsm#220
> 
> It uses the same pattern almost exactly.

Thanks a lot. Should have checked there first myself. I still feel a little lost around here.

Comment 5

[Nicolas Carlo [:nickecarlo]]

Attachment: Replace XPCOMUtils.defineLazyGetter with defineLazyModuleGetter

Changes made in files browser.js (thanks to Mark Finkle, I changed the remaining two instances as well), AddonUpdateService.js, and SessionStore.js.

Comment 6

[:Margaret Leibovic]

Comment on attachment 758916 [details] [diff] [review]
Replace XPCOMUtils.defineLazyGetter with defineLazyModuleGetter

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

Looks good.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9be31da8e36a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9be31da8e36a