Open Bug 1494103 Opened 11 months ago Updated 2 months ago

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

Categories

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

task

Tracking

()

People

(Reporter: bgrins, Unassigned)

References

Details

Attachments

(1 file)

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.
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.
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.
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → WONTFIX
Assignee: nobody → bgrinstead
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Status: REOPENED → ASSIGNED
Type: defect → task
Priority: -- → P3

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).

Assignee: bgrinstead → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.