1354341 - Avoid loading Console.jsm from LoginManagerContent's onPageShow()

Status: RESOLVED WONTFIX

(Toolkit :: Password Manager, enhancement)

Description

[Andrew McCreight [:mccr8]]

LoginManagerContent's onPageShow() calls _detectInsecureFormLikes(topWindow), which in turn has this:
  log("_detectInsecureFormLikes", topWindow.location.href);

This in turn ends up loading Console.jsm, even though it is not actually logging anything (unless the pref signon.debug is set). This appears to be the only reason we load Console.jsm at startup. It would be nice if we could avoid that.

One way to do that would be to make LoginHelper.jsm create a little shim console, but various places in the password manager use debug, warn and error in addition to log, so to really do this you'd have to duplicate a lot of the Console logic here which is not ideal.

Comment 1

[Andrew McCreight [:mccr8]]

On my machine, Console.jsm uses about 67kb to 74kb of memory per content process.

Comment 2

[Andrew McCreight [:mccr8]]

try: 2fe7fd31feca076a14844d4a0c7c6135e0fba3bd

Comment 3

[Andrew McCreight [:mccr8]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fe7fd31feca076a14844d4a0c7c6135e0fba3bd

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1354341 - Don't load ConsoleAPI.jsm for passwordmgr logging unless we need it.

Review commit: https://reviewboard.mozilla.org/r/128506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128506/

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8856547 [details]
Bug 1354341 - Don't load ConsoleAPI.jsm for passwordmgr logging unless we need it.

https://reviewboard.mozilla.org/r/128506/#review131720

Stealing this review from Dolske since I have opinions here and am the one who switched to console.jsm…

Console.jsm isn't passwordmgr-specific, so adding 60 lines of code to each consumer that wants to use it doesn't seem reasonable especially when tomorrow another consumer can come along and start using it and now those new lines are for nothing but added complexity. In fact, if you flip on formautofill with browser.formautofill.experimental and browser.formautofill.enabled prefs (which will soon be enabled by default on Nightly and should ship to release between Fx56 and Fx57) then you will find it also imports console.jsm on many pages.

Look at all the other consumers: https://dxr.mozilla.org/mozilla-central/search?q=new+ConsoleAPI%22+-path%3Atest+-path%3A%2Fsdk%2F&redirect=false
If none of those consumers happened to run during startup then that was just pure luck since logging is something one would want to use for debugging in all phases of the application.

Console.jsm is superior to Log.jsm in many ways and I'd love if everyone switched to Console.jsm and then we can remove Log.jsm but I suspect that's not going to happen anytime soon so I don't really see a good solution here. I'm willing to discuss alternatives or rationale but as-is I don't think this patch is a sustainable solution.

Comment 6

[Andrew McCreight [:mccr8]]

Fair enough.