Delay exiting stub installer until after browser has started

VERIFIED FIXED in Firefox 57

Status

()

enhancement
P2
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: mhowell, Assigned: mhowell)

Tracking

unspecified
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 wontfix, firefox56 wontfix, firefox57 fixed)

Details

Attachments

(2 attachments)

Currently, the installers run the newly installed browser when installation is complete, and then immediately exit. This means there is some delay between the installer window going away and a browser window appearing, so the user has to wait with no indication that anything is happening. We could make browser startup feel faster if we delay exiting the installer until browser startup is substantially complete (which we could most simply define as being when a window has appeared).

We should only do this in the stub installer, because it isn't really conceptually appropriate for the full installer, and because the stub is still running a progress bar when it exits, but the full installer has a finish page.
Priority: P3 → P2
Matt, can you please confirm the proposed user experience here:
1 Will the progress bar remains at 100% for a little longer until the Firefox window opens?
2 Will the progress bar get to 100% only when the Firefox Window opens (would require progress bar adjustment tricks)

Michael would rather we do (2) but I'm unsure what's feasible.
We could do option 2, but keep in mind that the installer can't really track the progress of browser startup, so the the bar would have to either a) sit at not-quite-the-end until the browser window is up and then jump to 100% right before the installer window closes, or b) fill up the rest of the way on a timer (e.g. 1% every second or so), meaning we'll be guessing at how long the browser takes to start, and we would almost always get it wrong and end up jumping forward a bit anyway, or filling up the progress bar before the browser is open (as in option 1).
It sounds like a UX consideration, Michael do you have a preference between a) or b)?
Flags: needinfo?(mverdi)
(In reply to Romain Testard [:RT] from comment #3)
> It sounds like a UX consideration, Michael do you have a preference between
> a) or b)?

I mocked this up and ran it by Bryant Mao and Cindy Hsiang and we all agreed that option 2b is considerably better than the other options. 

Do we have any data to help us here? Do we know what the median time between the installer finishing and the Firefox window opening? 

My off the cuff guess is something like 8 seconds. So if we use that as an example, the progress bar could hit 92% width when the installation is done and then progress 1% of the width each of the next 8 seconds. Then if the gap on my computer falls at 4 seconds, then the progress bar jumps 4% at the end. Or if the gap on your computer is 12 seconds, then the progress bar sits at 100% for 4 seconds.
Flags: needinfo?(mverdi)
I *think* the histogram SIMPLE_MEASURES_CREATETOPLEVELWINDOW measures the time we're talking about; the graph at [0] shows its distribution as of release 54. I'm not entirely sure because I'm having a hard time finding where that gets set, but it sounds right.

The 75th percentile by that distribution is at ~ 5 seconds, so I think that's where we should set our guess. As in, do as Verdi described, but with 5 instead of 8. Does that look right to you guys?

We might have to reduce the value a bit once we've seen how 57 performs; the median seems to already be trending down since the middle of the nightly 55 cycle [1].

[0] https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-06-27&keys=__none__!__none__!__none__&max_channel_version=release%252F54&measure=SIMPLE_MEASURES_CREATETOPLEVELWINDOW&min_channel_version=null&os=Windows_NT&processType=parent&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-06-07&table=0&trim=1&use_submission_date=0
[1] https://telemetry.mozilla.org/new-pipeline/evo.html#!aggregates=median&cumulative=0&end_date=2017-08-01&keys=!__none__!__none__&max_channel_version=nightly%252F56&measure=SIMPLE_MEASURES_CREATETOPLEVELWINDOW&min_channel_version=nightly%252F53&os=Windows_NT&processType=parent&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-06-12&trim=1&use_submission_date=0
(In reply to Matt Howell [:mhowell] from comment #5)
> I *think* the histogram SIMPLE_MEASURES_CREATETOPLEVELWINDOW measures the
> time we're talking about; the graph at [0] shows its distribution as of
> release 54. I'm not entirely sure because I'm having a hard time finding
> where that gets set, but it sounds right.
> 
> The 75th percentile by that distribution is at ~ 5 seconds, so I think
> that's where we should set our guess. As in, do as Verdi described, but with
> 5 instead of 8. Does that look right to you guys?
This sounds good to me and yes we can probably adjust the value if we see big changes with FF57
I am currently working on this.
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Comment on attachment 8898371 [details]
Bug 1376597 Part 1 - Wait for the browser to start before exiting the stub installer.

https://reviewboard.mozilla.org/r/169738/#review175110

::: browser/installer/windows/nsis/stub.nsi:1699
(Diff revision 1)
> +  FindWindow $0 "${MainWindowClass}"
> +  ${If} $0 <> 0
> +    ${NSD_KillTimer} WaitForAppLaunch
> +    StrCpy $ProgressCompleted "$ProgressTotal"
> +    Call SetProgressBars
> +    Call SendPing

Return after the Call SendPing here and below?
Comment on attachment 8898371 [details]
Bug 1376597 Part 1 - Wait for the browser to start before exiting the stub installer.

https://reviewboard.mozilla.org/r/169738/#review175592

::: browser/installer/windows/nsis/stub.nsi:1694
(Diff revision 1)
>      Exec "$\"$INSTDIR\${FileMainEXE}$\""
>    ${EndIf}
>  FunctionEnd
>  
> +Function WaitForAppLaunch
> +  FindWindow $0 "${MainWindowClass}"

How will this interact with the Import Wizard?
Comment on attachment 8898371 [details]
Bug 1376597 Part 1 - Wait for the browser to start before exiting the stub installer.

https://reviewboard.mozilla.org/r/169738/#review175624

This mostly looks good, just the two issues I mentioned.
Attachment #8898371 - Flags: review?(agashlin) → review-
Comment on attachment 8898372 [details]
Bug 1376597 Part 2 - Simplify progress bar handling.

https://reviewboard.mozilla.org/r/169740/#review175582

::: browser/installer/windows/nsis/stub.nsi:50
(Diff revision 1)
>  
>  Var CanWriteToInstallDir
>  Var HasRequiredSpaceAvailable
>  Var IsDownloadFinished
>  Var DownloadSizeBytes
>  Var HalfOfDownload

You could get rid of HalfOfDownload now
Attachment #8898372 - Flags: review?(agashlin) → review+
Comment on attachment 8898371 [details]
Bug 1376597 Part 1 - Wait for the browser to start before exiting the stub installer.

https://reviewboard.mozilla.org/r/169738/#review175994

::: browser/installer/windows/nsis/stub.nsi:1694
(Diff revision 1)
>      Exec "$\"$INSTDIR\${FileMainEXE}$\""
>    ${EndIf}
>  FunctionEnd
>  
> +Function WaitForAppLaunch
> +  FindWindow $0 "${MainWindowClass}"

That is an excellent question. I will look into that.

::: browser/installer/windows/nsis/stub.nsi:1699
(Diff revision 1)
> +  FindWindow $0 "${MainWindowClass}"
> +  ${If} $0 <> 0
> +    ${NSD_KillTimer} WaitForAppLaunch
> +    StrCpy $ProgressCompleted "$ProgressTotal"
> +    Call SetProgressBars
> +    Call SendPing

I didn't do that because SendPing doesn't return, it always exits the installer, so those would be redundant. But that also means they don't hurt anything, so I don't mind adding them in for readability/consistency.
(In reply to Matt Howell [:mhowell] from comment #14)
> I didn't do that because SendPing doesn't return, it always exits the
> installer, so those would be redundant. But that also means they don't hurt
> anything, so I don't mind adding them in for readability/consistency.

Initially I thought that as well, but when actually sending the ping SendPing does return, the exit is done by OnPing after at least DownloadIntervalMS
(In reply to Adam Gashlin [:agashlin] from comment #17)
> (In reply to Matt Howell [:mhowell] from comment #14)
> > I didn't do that because SendPing doesn't return, it always exits the
> > installer, so those would be redundant. But that also means they don't hurt
> > anything, so I don't mind adding them in for readability/consistency.
> 
> Initially I thought that as well, but when actually sending the ping
> SendPing does return, the exit is done by OnPing after at least
> DownloadIntervalMS

Ah, good point, I'll fix it.
Comment on attachment 8898371 [details]
Bug 1376597 Part 1 - Wait for the browser to start before exiting the stub installer.

https://reviewboard.mozilla.org/r/169738/#review176316
Attachment #8898371 - Flags: review?(agashlin) → review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c170eca4275
Part 1 - Wait for the browser to start before exiting the stub installer. r=agashlin
https://hg.mozilla.org/integration/autoland/rev/c96e332ad836
Part 2 - Simplify progress bar handling. r=agashlin
(In reply to Adam Gashlin [:agashlin] from comment #11)
> How will this interact with the Import Wizard?

FWIW, beginning with 57 we won't show the import wizard on firstrun and instead will have a message about it on newtab.
https://hg.mozilla.org/mozilla-central/rev/7c170eca4275
https://hg.mozilla.org/mozilla-central/rev/c96e332ad836
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Depends on: 1394582
Depends on: 1395369
Depends on: 1396912
I tested this on Windows OSes and it is verified as fixed.
Test runs are here: https://public.etherpad-mozilla.org/p/1376597
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.