Closed Bug 1489458 Opened 2 years ago Closed 1 year ago

Stop console.log'ing the world in browser.js' startup code

Categories

(Firefox for Android :: General, defect, P2)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 69
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- fixed
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: Gijs, Assigned: brad.arant)

Details

(Whiteboard: [fennec68.1])

Attachments

(1 file, 3 obsolete files)

Creating console objects and logging to them isn't free (cf. bug 1486885 comment 10). And on mobile, 99.999999% of users will never see the messages. So doing it by default on mobile in a startup method seems like a waste of time.
OS: Unspecified → Android
Hardware: Unspecified → ARM
Version: unspecified → Trunk

Sounds like a possible performance benefit and not too difficult.

Priority: -- → P2
Assignee: nobody → brad.arant

This patch removes the obvious debug and extraneous 'informational' messages in the browser.js file. I left exception handling log entries as those will be needed.

Attachment #9070298 - Attachment is obsolete: true

Depends on D35213

Submitted patch to keep requested message and change to console.warn.

Attachment #9072594 - Attachment is obsolete: true
Attachment #9073603 - Attachment is obsolete: true

Please check-in.

Keywords: checkin-needed

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b788a1f2f095
Remove extraneous console logging in browser.js.;r=VladBaicu

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Gijs, should we uplift this console spam fix to Fennec ESR 68? There will be no Fennec 69 release. Fennec users will be switched to the ESR 68 channel for the next 6-12 months, so the only way to release a fix to Fennec users will be to uplift to ESR.

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Chris Peterson [:cpeterson] from comment #12)

Gijs, should we uplift this console spam fix to Fennec ESR 68? There will be no Fennec 69 release. Fennec users will be switched to the ESR 68 channel for the next 6-12 months, so the only way to release a fix to Fennec users will be to uplift to ESR.

I guess so, doing so is risk-free, as all we're doing is removing some extraneous logging. Probably best if the patch author or reviewer does the uplift request though.

Flags: needinfo?(vlad.baicu)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(brad.arant)

(In reply to :Gijs (he/him) from comment #13)

I guess so, doing so is risk-free, as all we're doing is removing some extraneous logging. Probably best if the patch author or reviewer does the uplift request though.

Sounds good. "Risk-free" is nice. :)

Brad or Vlad, can you please request uplift to Fennec ESR 68 when you have a chance?

Comment on attachment 9074687 [details]
Bug 1489458 - Remove extraneous console logging in browser.js.;r?VladBaicu

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Low impact on final release.
  • User impact if declined: Extra logging message that may slow performance.
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Only impact to logging. No logic changes.
  • String or UUID changes made by this patch:
Flags: needinfo?(brad.arant)
Attachment #9074687 - Flags: approval-mozilla-esr68?
Flags: needinfo?(vlad.baicu)

Comment on attachment 9074687 [details]
Bug 1489458 - Remove extraneous console logging in browser.js.;r?VladBaicu

Removes some extraneous logging which can cause perf issues. Approved for Fennec 68.1b1.

Attachment #9074687 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Whiteboard: [fennec68.1]
You need to log in before you can comment on or make changes to this bug.