496727 - Clean up the code landed as part of bug 488706

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[(no longer active)]

Follow-up from bug 488706 comment 51.  I quote Mossop's comment there here for easier reference:

> FWIW I think we should avoid the (function() { })() style, I don't see a real
> need for it in this case and it is quite different to the general style in
> browser.js. I'd also question why we didn't use the migration path that already
> exists in nsBrowserGlue.js rather than rolling a new one. I guess the
> BrowserGlue is in the direct startup I don't think it should be causing enough
> of a hit to be concerned about.

He also mentioned on IRC that I should have used gPrefService, which is right.

mconnor, how would you like me to proceed here?  Do you want me to address all three items, or maybe some others as well?

Comment 1

[Dão Gottwald [:dao]]

Ehsan, ping?

Comment 2

[(no longer active)]

Attachment: Patch (v1)

Don't use the (function() {}() style and use gPrefService.  No other changes in this patch.  I don't think we should use the browser glue migration path because it's used for UI migration and it's absolutely needed before the UI appears, but this one is not, and I hate to add something to the browser startup cost when it's not needed there, as little as it may be.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 388457 [details] [diff] [review]
Patch (v1)

Should also use let instead of var here, and Cu instead of Components.utils.

Comment 4

[(no longer active)]

Attachment: For check-in

(In reply to comment #3)
> (From update of attachment 388457 [details] [diff] [review])
> Should also use let instead of var here, and Cu instead of Components.utils.

Done.

Comment 5

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/e042986c85f2