1494103 - Dynamically create the "sidebar" <browser> when it's needed

Status: NEW

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

Description

[Brian Grinstead [:bgrins]]

Right now we put the sidebar browser in browser.xul and then are careful to not eagerly access it from JS to avoid attaching XBL.

- https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/browser/base/content/browser.xul#1285
- https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/browser/base/content/browser-sidebar.js#43

To make this more compatible with Custom Elements (which get constructed regardless of element visibility or js reference), we could change this around to dynamically make the browser element when needed. I'm hoping this won't be too difficult, since we're already lazily accessing it.

Comment 1

[Brian Grinstead [:bgrins]]

Oddly, this seems to cause perf regressions https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=0e59d66dd39316efafdaefab76a16160f9d92130&newProject=try&newRevision=4f27edf059510d900af9610d3af2724eabbc4c9d&framework=1&showOnlyImportant=1. My first guess is that having it in the markup is causing browser.xml to be loaded eagerly, or something. Even though the binding isn't attached until the sidebar is opened, which I assume talos tests don't do.

Comment 2

[Brian Grinstead [:bgrins]]

Actually, it looks like the straightforward CE migration isn't causing any talos regression. Given that plus Comment 1, I'm going to close this. We may ultimately want to lazify this anyway, but it shouldn't block migration.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1494103 - Lazily create the "sidebar" <browser> only when it's needed;r=Gijs

Comment 4

[Brian Grinstead [:bgrins]]

I don't have time to work on this right now, so marking as unassigned. I still think this should help with performance since we won't be running the browser constructor at startup, but I'm seeing some test failures (at least some of which should be test fixes due to some tests expecting the node to be there before the sidebar is opened).