1374216 - Add-on icons should be in the navbar by first paint

Status: NEW

(Firefox :: Toolbars and Customization, defect, P3)

Description

[:Gijs (he/him)]

STR:

1. start nightly.


AR:
Additional button icons appear a second or so after the window comes up, creating distracting visual movement, not just for the icons but also for the location bar and search bar which resize based on the available space.

ER:
The window should be static after appearing. Icons for add-ons should appear in the toolbar immediately.


This is a regression because it used to work like this. My understanding is that we've delayed the startup of various bits to avoid some number of ms of startup costs, but it feels like the net result is now that startup looks messy, and essentially worse.

It's not really clear to me why we technically can't make this work. Seems like the simplest thing would be to require add-ons and their browser_actions to be loaded and ready by the time we first paint the window. 

If that is somehow impossible, a compromise could potentially be to have CustomizableUI *generate* the toolbarbuttons / widgets based on previous runtime, and hook up actual functionality when the add-on comes up. Assuming that at least the resource: URIs for the relevant icons are available immediately, that could be cheaper, and users would be none the wiser. My understanding is that in a webextension world, the toolbar buttons don't get notified for their instances being created nor do they have the possibility of manipulating the XUL button, so in principle this seems like it would be OK?

Andrew or Dave, do you have more context / thoughts here?

Comment 1

[:Gijs (he/him)]

Given that we don't ship with any of these (anymore), I guess we can live with this for 57 - certainly doesn't need to block. Plus we now reserve space in the toolbar for these and other icons, so it's only the actual icons, rather than any element resizing (ideally) happening.

I'm still confused about what broke this though...

Comment 2

[Florian Quèze [:florian]]

Gijs, can you still reproduce this? I think I fixed most (if not all) of this, and it's covered by flicker tests.

Comment 3

[Alex Thayer [:alexical] (she/her)]

For whatever reason it seems to be reproducible for me when running a different binary than the one most recently used. This would typically be artificial but it happens when restarting to update firefox, for instance.

Comment 4

[Florian Quèze [:florian]]

(In reply to Doug Thayer [:dthayer] from comment #3)
> For whatever reason it seems to be reproducible for me when running a
> different binary than the one most recently used. This would typically be
> artificial but it happens when restarting to update firefox, for instance.

Is this with real add-ons you installed yourself? (somehow I assumed this bug was mostly talking about system add-ons) Is the space for the icons already reserved and it's just the image that takes a few more ms to appear, or is the whole navbar content shifting to make space for the icons?

Comment 5

[Alex Thayer [:alexical] (she/her)]

Attachment: addonsnoise.flv

Comment 6

[Alex Thayer [:alexical] (she/her)]

It's with real add-ons, and only the other icons in that section (library and reader mode) shift. So overall it's not that bad, but it's still noisier than it could be of course. If you feel like it's a p3, I'm fine with that - I'm probably overly sensitive to this kind of noise.

Comment 7

[Florian Quèze [:florian]]

Thanks for the video! In it there's a frame where the last 2 icons shift and the urlbar gets resized to create space for the add-on icons. Then a couple frames later the actual icons appear. If we want to fix this, I think the next step would be capturing a startup profile, to see why these add-ons are not ready to have their icon in the navbar at first paint. I wouldn't be surprised if the add-on itself introduced some delay in its startup code to avoid slowing down the browser startup. I think I saw something like that in the gecko profiler add-on.

I read this bug too quickly when I wrote comment 2; it's in new windows that I got rid of the flicker, I don't think I fixed it for the first window.

I'm fine with p2, but I don't expect to take this bug myself in the near future. If you want to take it you are very welcome to do so :).

Comment 8

[:Gijs (he/him)]

My understanding is that some webextension stuff starts up "later", cf. bug 1378459 and dupes. I wouldn't be surprised if this races with other bits of browser startup leading to issues like this.

Prior to things going webextensions-only, there wasn't a lot we could do about this. Now, in theory, we could e.g. cache more state in CustomizableUI regarding add-ons, and thus display the buttons before the add-on properly started up, to throw out a crazy idea. :-)

That'd be a significant project, and we should investigate if there are quicker/easier solutions, but I believe that it would in theory be possible. Right now, CUI keeps an array of ids that occur in each area (toolbar/panel) and some ids can simply go missing when their add-ons (or builtin implementation) disappear / get disabled/uninstalled. So CUI has to deal with items appearing/disappearing willy-nilly, and putting them in the 'right' place, and that's what it does. But in a webextensions-only world, in theory it should know exactly which add-ons are and aren't actually present and enabled, and which buttons therefore are and aren't going to be really 'there', especially if that hasn't changed since last startup (which it shouldn't in 99% of cases). So perhaps it can now do better for cases like this...

Comment 9

[Andrew Swan [:aswan]]

Regarding WebExtensions, I think we're just talking about browser actions here?  Those are defined in an extension's manifest which we should have in the extensions startup cache (that's startupCache/webext.sc.lz4 in the profile).  That means we don't need to touch the actually extension .xpi to get to it, but we still load that file asychnronously.  Also we load individual WebExtensions APIs (including browser action which is the thing that turns those bits from the manifest into a corresponding call to CustomizableUI) lazily.

Bug 1378459 has become a general place to talk about extensions and startup, but I think of it more specifically as dealing with  code in extension background pages that wants to affect things that happen during startup.  I think that's mostly distinct from the issue in this bug about navbar icons, I hope to have an update and proposal on bug 1378459 soon...