1704868 - The notificationbar "stack" lazy getter is called when closing a tab

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Florian Quèze [:florian]]

I noticed in the profile of closing many tabs from bug 1379174 comment 6 that 2.2% of the time spent in removeTabs is spent in notificationbox.js: get stack https://share.firefox.dev/3g8aLPC

The code at https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/browser/base/content/tabbrowser.js#3811-3812 tries to avoid triggering the notification box lazy getter, but then doesn't check if the stack has already been initialized before accessing it.

Unfortunately, the getNotificationBox method is called at https://searchfox.org/mozilla-central/rev/3de2db87f3c9001ae478318d47a2ca3427574382/browser/base/content/browser.js#5859, so any tab that has already had a location change event will have a notification box.

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 1704868 - avoid triggering the notificationbar stack lazy getter for every tab close, r=mstriemer.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1682676

Comment 3

[Mark Striemer [:mstriemer]]

Attachment: Bug 1704868 - fix tests for lazy notificationbox creation r?florian

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc993666f052
avoid triggering the notificationbar stack lazy getter for every tab close, r=mstriemer.
https://hg.mozilla.org/integration/autoland/rev/0ac53cac5ca8
fix tests for lazy notificationbox creation r=florian

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (bug 1704868) for browser_app.js mochitest failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=os%2Cx%2C10.14%2Cwebrender%2Copt%2Cmochitests%2Ctest-macosx1014-64-qr%2Fopt-mochitest-browser-chrome-e10s%2Cbc1&selectedTaskRun=Nz4biu7_QkCTU3KPgFIzZw.0&fromchange=e40c2fa01489267d922ea6672a344be63e5fcf86&tochange=77b569d0233661e86c3db1b631482fec178e8618

Backout link: https://hg.mozilla.org/integration/autoland/rev/50da9f556ed0e4fe60a103a9991bac42fd3734ac

Failure log: https://treeherder.mozilla.org/logviewer?job_id=336821307&repo=autoland&lineNumber=1505

[task 2021-04-16T18:49:01.215Z] 18:49:01     INFO - TEST-START | accessible/tests/browser/mac/browser_app.js
[task 2021-04-16T18:49:02.136Z] 18:49:02     INFO - TEST-INFO | started process screencapture
[task 2021-04-16T18:49:02.236Z] 18:49:02     INFO - TEST-INFO | screencapture: exit 0
[task 2021-04-16T18:49:02.236Z] 18:49:02     INFO - Buffered messages logged at 18:49:01
[task 2021-04-16T18:49:02.236Z] 18:49:02     INFO - Entering test bound 
[task 2021-04-16T18:49:02.236Z] 18:49:02     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<title>Two</title>" line: 0}]
[task 2021-04-16T18:49:02.236Z] 18:49:02     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<title>Three</title>" line: 0}]
[task 2021-04-16T18:49:02.237Z] 18:49:02     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<title>Four</title>" line: 0}]
[task 2021-04-16T18:49:02.237Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | We now have 4 open tabs - 
[task 2021-04-16T18:49:02.237Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct role for tablist - 
[task 2021-04-16T18:49:02.237Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | 4 items in AXTabs - 
[task 2021-04-16T18:49:02.237Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | one selected tab - 
[task 2021-04-16T18:49:02.238Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct role for tab - 
[task 2021-04-16T18:49:02.238Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct subrole for tab - 
[task 2021-04-16T18:49:02.238Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct title for tab - 
[task 2021-04-16T18:49:02.238Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct title for tab - 
[task 2021-04-16T18:49:02.238Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | [u'AXShowMenu', u'AXScrollToVisible', u'AXPress'] - 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Has switch action - 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | one selected tab - 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - TEST-PASS | accessible/tests/browser/mac/browser_app.js | Correct title for tab - 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - Leaving test bound 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - Entering test bound 
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - Buffered messages finished
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/mac/browser_app.js | Root with no popups has 6 children - Got 5, expected 6
[task 2021-04-16T18:49:02.239Z] 18:49:02     INFO - Stack trace:
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - chrome://mochikit/content/browser-test.js:test_is:1362
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - chrome://mochitests/content/browser/accessible/tests/browser/mac/browser_app.js:null:133
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - Global property added while loading chrome://browser/content/nsContextMenu.js: screenshotsDisabled
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - Not taking screenshot here: see the one that was previously logged
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/mac/browser_app.js | Root has 1 more child - Got 6, expected 7
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - Stack trace:
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - chrome://mochikit/content/browser-test.js:test_is:1362
[task 2021-04-16T18:49:02.240Z] 18:49:02     INFO - chrome://mochitests/content/browser/accessible/tests/browser/mac/browser_app.js:null:169
[task 2021-04-16T18:49:02.251Z] 18:49:02     INFO - Not taking screenshot here: see the one that was previously logged
[task 2021-04-16T18:49:02.251Z] 18:49:02     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/mac/browser_app.js | Root has original child count - Got 5, expected 6
[task 2021-04-16T18:49:02.251Z] 18:49:02     INFO - Stack trace:
[task 2021-04-16T18:49:02.251Z] 18:49:02     INFO - chrome://mochikit/content/browser-test.js:test_is:1362
[task 2021-04-16T18:49:02.251Z] 18:49:02     INFO - chrome://mochitests/content/browser/accessible/tests/browser/mac/browser_app.js:null:179
[task 2021-04-16T18:49:02.541Z] 18:49:02     INFO - Not taking screenshot here: see the one that was previously logged
[task 2021-04-16T18:49:02.542Z] 18:49:02     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/mac/browser_app.js | Root has another child - Got 6, expected 7
[task 2021-04-16T18:49:02.542Z] 18:49:02     INFO - Stack trace:
[task 2021-04-16T18:49:02.542Z] 18:49:02     INFO - chrome://mochikit/content/browser-test.js:test_is:1362
[task 2021-04-16T18:49:02.542Z] 18:49:02     INFO - chrome://mochitests/content/browser/accessible/tests/browser/mac/browser_app.js:null:187
[task 2021-04-16T18:49:02.904Z] 18:49:02     INFO - Not taking screenshot here: see the one that was previously logged
[task 2021-04-16T18:49:02.904Z] 18:49:02     INFO - TEST-UNEXPECTED-FAIL | accessible/tests/browser/mac/browser_app.js | Root has the base child count - Got 5, expected 6
[task 2021-04-16T18:49:02.904Z] 18:49:02     INFO - Stack trace:
[task 2021-04-16T18:49:02.904Z] 18:49:02     INFO - chrome://mochikit/content/browser-test.js:test_is:1362
[task 2021-04-16T18:49:02.904Z] 18:49:02     INFO - chrome://mochitests/content/browser/accessible/tests/browser/mac/browser_app.js:null:194
[task 2021-04-16T18:49:02.930Z] 18:49:02     INFO - Leaving test bound 

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce3ebc6947d1
avoid triggering the notificationbar stack lazy getter for every tab close, r=mstriemer.
https://hg.mozilla.org/integration/autoland/rev/acfd4b75c08b
fix tests for lazy notificationbox creation r=florian

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ce3ebc6947d1
https://hg.mozilla.org/mozilla-central/rev/acfd4b75c08b