1681138 - Avoid delaying bookmarks toolbar initialization for new windows post-startup

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[:Gijs (he/him)]

After bug 1667237 we always delay showing bookmarks until after the window is ready.

This makes sense for startup, but doesn't make as much sense once for windows that are opened subsequently - the places DB will have already loaded, and we should be able to get the bookmarks without more IO (though whether that is really true right now, ie whether we don't do the IO N times for N windows, I'm less sure about...).

Comment 1

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

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

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1681138 - avoid delaying loading the bookmarks toolbar in new windows post-startup, r?mak

Comment 3

[Marco Bonardo [:mak]]

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

we should be able to get the bookmarks without more IO (though whether that is really true right now, ie whether we don't do the IO N times for N windows, I'm less sure about...).

We'd do N times for N windows, each view has a result, the result is who runs the query, causing the IO. Results are not shared.
In the future that may change if we decide to keep the bookmarks tree in memory, though that may also not be great for memory usage if the user has thousands (but we could also just keep a tree of empty nodes and fill their infos on-demand).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e8e8cc89f61
avoid delaying loading the bookmarks toolbar in new windows post-startup, r=mak

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (bug 1680216, bug 1681138) for browser_984455_bookmarks_items_reparenting.js failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=D9O_VLW_Q0S6QCWOOkdi6Q.0&fromchange=67e673da78c5d876a49e28c31d068ba3e1d08e5d&searchStr=windows%2C10%2Cx64%2Cwebrender%2Cdebug%2Cmochitests%2Ctest-windows10-64-qr%2Fdebug-mochitest-browser-chrome-e10s%2Cbc1&tochange=85d196d4bbd3406d0f06baf1010b07a9525d30aa

Backout link: https://hg.mozilla.org/integration/autoland/rev/85d196d4bbd3406d0f06baf1010b07a9525d30aa

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

...
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - Entering test bound testOverflowingBookmarksItemsContextMenu
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - Ensuring panel is ready.
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - Buffered messages logged at 01:48:31
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - Waiting for context menu on personal-bookmarks
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - Buffered messages finished
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js | uncaught exception - TypeError: can't access property "buildContextMenu", menupopup._view is null at placesContextShowing@resource:///modules/PlacesUIUtils.jsm:1350:5
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - onpopupshowing@chrome://browser/content/browser.xhtml:1:22
[task 2021-01-22T01:48:31.200Z] 01:48:31     INFO - synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:594:32
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:526:10
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - synthesizeMouseAtCenter@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:701:10
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - checkPlacesContextMenu/<@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:47:16
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - checkPlacesContextMenu@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:60:5
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - testOverflowingBookmarksItemsContextMenu@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:244:9
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - 
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - Stack trace:
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:simpletestOnerror:2112
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtPoint:594
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouse:526
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtCenter:701
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:checkPlacesContextMenu/<:47
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:checkPlacesContextMenu:60
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:testOverflowingBookmarksItemsContextMenu:244
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - GECKO(9900) | JavaScript error: resource:///modules/PlacesUIUtils.jsm, line 1350: TypeError: can't access property "buildContextMenu", menupopup._view is null
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - Console message: [JavaScript Error: "TypeError: can't access property "buildContextMenu", menupopup._view is null" {file: "resource:///modules/PlacesUIUtils.jsm" line: 1350}]
[task 2021-01-22T01:48:31.201Z] 01:48:31     INFO - placesContextShowing@resource:///modules/PlacesUIUtils.jsm:1350:5
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - onpopupshowing@chrome://browser/content/browser.xhtml:1:22
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:594:32
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:526:10
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - synthesizeMouseAtCenter@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:701:10
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - checkPlacesContextMenu/<@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:47:16
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - checkPlacesContextMenu@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:60:5
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - testOverflowingBookmarksItemsContextMenu@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js:244:9
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - 
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - TEST-PASS | browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js | New bookmark item shouldn't be disabled - 
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - Closing context menu
[task 2021-01-22T01:48:31.202Z] 01:48:31     INFO - Waiting for overflow
[task 2021-01-22T01:48:31.220Z] 01:48:31     INFO - GECKO(9900) | [Child 2772, Main Thread] WARNING: Scrolled rect smaller than scrollport?: file /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:6984
[task 2021-01-22T01:48:31.220Z] 01:48:31     INFO - GECKO(9900) | [Child 2772, Main Thread] WARNING: Scrolled rect smaller than scrollport?: file /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:6984
[task 2021-01-22T01:48:31.220Z] 01:48:31     INFO - GECKO(9900) | [Child 2772, Main Thread] WARNING: Scrolled rect smaller than scrollport?: file /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:6984
[task 2021-01-22T01:48:31.305Z] 01:48:31     INFO - TEST-PASS | browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js | Item with ID personal-bookmarks should no longer be in the gNavBar - 
[task 2021-01-22T01:48:31.305Z] 01:48:31     INFO - TEST-PASS | browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js | Item with ID personal-bookmarks should be overflowing - 
[task 2021-01-22T01:48:31.305Z] 01:48:31     INFO - TEST-PASS | browser/components/customizableui/test/browser_984455_bookmarks_items_reparenting.js | Item with ID personal-bookmarks should have overflowedItem attribute - 
[task 2021-01-22T01:48:31.309Z] 01:48:31     INFO - Waiting until we stop overflowing
...

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9432550128b
avoid delaying loading the bookmarks toolbar in new windows post-startup, r=mak

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c9432550128b