657581 - add-on installation dialog takes between 4 and 7 seconds to appear

Status: RESOLVED INACTIVE

(Toolkit :: Add-ons Manager, defect)

Description

[Dietrich Ayala (:dietrich)]

I test a lot of my add-ons by opening from the local filesystem via cmd/ctrl+o.

I noticed that it takes a while for the install dialog to open. Mentally I grouped it with the intentional 2 second delay (bug 416605). However, over time I've noticed that the delay that flips my anger flag is *before* that dialog becomes visible.

I just tried to open an xpi from the filesystem a few times in quick succession and the times in seconds were 7, 7, 4, 6.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Interesting. I thought that I have had already filed that bug. But can't find it now. So yes, it really takes a long time and it is even longer when you select multiple extensions. AFAIK we copy all XPI files to the profile first which is similar to the download progress from websites (bug 652896). So both bugs could be related.

Comment 2

[Dave Townsend [:mossop]]

We don't copy the files to the profile first, however we do ping AMO to get updated metadata (plus if necessary the update check) before showing the install dialog I think. Since local file installs are not the common case I'm inclined to just wait and see what comes out of the install changes that Boriss is working on before thinking much about this.

Comment 3

[Dietrich Ayala (:dietrich)]

That's crazy talk. We can't just have zero response from the application while waiting on background network activity that the user isn't notified about, so doesn't know is happening. That's gotta be *far* slower than anything happening locally on the disk, and is possibly the cause of these 7 second waits I'm talking about.

What are the install changes you're referring to? Bug number?

I think that we should show the install dialog immediately, with a progress bar and textual updates: "Pinging AMO for metadata...", "Doing update check...", "Getting a sponge bath...". And once all of that stuff is complete, we do the security delay.

EVEN BETTER... if all that crap takes longer than the security delay (sounds like that's happening a lot if that stuff comprises any of the 7 seconds here), we skip the delay at that point, since the user has already had to sit there watching their beard grow as we do our pre-maybe-they-might-want-to-install housecleaning.

Comment 4

[Dave Townsend [:mossop]]

(In reply to comment #3)
> What are the install changes you're referring to? Bug number?

https://wiki.mozilla.org/Extension_Manager:Projects:Improve_Add-on_Installation

Comment 5

[Dietrich Ayala (:dietrich)]

13:25 <@dietrich> like showing the UI, but with buttons disabled or something like that.
13:30 <@Mossop> dietrich: Your question confuses me
13:31 <@Mossop> Hmm, don't we show the doorhanger in that case?
13:31 <@dietrich> Mossop: you had said that the delay in the security UI showing up was due to AMO ping, iiuc
13:32 <@dietrich> er, install dialog
13:32 <@Mossop> That is one delay yes, maybe we don't show the doorhanger because it is already downloaded
13:32 <@Mossop> But that seems like an easy option, show the doorhanger with an indeterminate progress bar while it does the ping
13:33 <@Mossop> Anyway the code that handles doing all that up to showing the install UI is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/amWebInstallListener.js
13:34 <@Mossop> Specifically styarting at http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/amWebInstallListener.js#335

Comment 6

[Dietrich Ayala (:dietrich)]

subsequent idea from mossop - remove this early return, so that the doorhanger stays open with the download progress meter running, until the point at which the install dialog pops up.

that way there'd never be a point with zero UI and zero feedback during the entire process of add-on install.

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#746

Comment 7

[Dietrich Ayala (:dietrich)]

Attachment: wip approach

this removes the already-downloaded return, and also switches the progress meter to indeterminate mode once the download is complete.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 570104 [details] [diff] [review]
wip approach

Review of attachment 570104 [details] [diff] [review]:
-----------------------------------------------------------------

I'm having difficulty figuring out where this notification eventually gets dismissed now but I haven't looked at this code in a while. Any ideas?

Comment 9

[Dietrich Ayala (:dietrich)]

This looks like where it should get dismissed once downloads finish:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#1149

And it kinda looks like that should still get called with my patch applied...

Comment 10

[Dave Townsend [:mossop]]

(In reply to Dietrich Ayala (:dietrich) from comment #9)
> This looks like where it should get dismissed once downloads finish:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> urlbarBindings.xml#1149
> 
> And it kinda looks like that should still get called with my patch applied...

That is where I'd expect it to get dismissed right now, but your patch removes the call to PopupNotifications.remove.

Comment 11

[Dietrich Ayala (:dietrich)]

hit diminishing returns on my time really quickly trying to figure out the UI bits. someone who's familiar with this code should take it from here.

also: XBL must die.

Comment 12

[Dietrich Ayala (:dietrich)]

snappy P1, so needs owner. assigning to dave to re-assign as needed.

Comment 13

[(dormant account)]

(In reply to Dietrich Ayala (:dietrich) from comment #12)
> snappy P1, so needs owner. assigning to dave to re-assign as needed.

After further discussion, it is best to lower priority to P2 since this is not primary UI.

Comment 14

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

A few minutes ago, AMO was down. Installing an add-on *from file* was impossible,
even an add-on with no updateURL; no UI at all, nothing. From the user's perspective,
that's an unresponsive command. That seems to me totally wrong.

Comment 15

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

AMO is back again and fully functional.

Not being based in the US, I guess my AMO experience is a bit
slower than the one MoCo people are used to.

But my last **local** installation of an add-on of mine having no
updateURL took 54 seconds (yes, fifty four seconds) between closing
the File Picker and the "Install add-on" dialog with the 5 seconds
delay appearing on screen.

This is just not usable.

Comment 16

[Dietrich Ayala (:dietrich)]

We shouldn't block UI on this, regardless of geographic location. And if we *have* to, we need a nice progress meter through the entire process.

And yeah we need a timeout on any network requests made. 54s is absurd.

Comment 17

[Dave Townsend [:mossop]]

Going to assign this to Blair with the full expectation that he won't have time to look at it until after DTC is done and dusted, and even then I am not his manager so he may have more important priorities.

Comment 18

[Dave Townsend [:mossop]]

Comment on attachment 570104 [details] [diff] [review]
wip approach

As far as I can tell this doesn't remove the notification properly when done, waiting on someone to explain to me why it doesn't or to fix it.

Comment 20

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.