1328445 - Streamlined stub installer

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Changes to the Windows stub installer for upcoming funnelcake (bug 1322718)

- Remove the introduction and options pages
- New design for download/install UI
  - no "Cancel" button
  - no distinction between download and install
  - BG graphic
  - timed succession of headlines
- Use Firefox icon
- Change name to Firefox Installer
- Improved cancel/error message

Comment 1

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 1. Back out Win 7 restriction and 64-bit support

Back out af88a04128f6 and 86a19f0dfa67 (bug 1305453) and 8638691b8a7a (bug 797208)

Comment 2

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Stub streamlining (apply over backout)

First pass at streamlining patch, expects to be applied over attachment 8823474 [details] [diff] [review]

Comment 3

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Stub streamlining (apply over backout)

Fix accidental debug setting

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Once this lands let's verify that it looks correct on m-c and uplift it as soon as possible - thanks.

Comment 5

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

This current patch definitely isn't in shape to be applied to m-c, I only learned that anyone was planning that today. My plan had been to use oak to build the modified stub installer alone. Additional work would be needed to make this not interfere with the preexisting stub, and I'm not sure that is even desirable for the onboarding funnelcake.

As I'm not comfortable with the workings of the flags, could you indicate this for me, lizzard?

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks Adam, I assumed it was part of what was needed for bug 1322718.  As far as I know, this funnelcake build is planned for the Firefox 51 release (for Jan. 23, so we want to try to land everything for it on beta as far ahead of that as possible so that we can test.)  

Dolske, gijs, is this something for a future funnelcake build or do we need it for this particular project?

Comment 7

[Romain Testard [:RT]]

Updating user story field with details discussed in PRD.

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 1. back out installer-related changes since beta

Comment 9

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 2. use release branding and build the stub installer

Comment 10

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 3. binaries (image and icon resources)

This includes a change to the self-extractor stub which replaces its icon with firefox64.ico, also in this patch.

Comment 11

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 4. Stub streamlining

These are the changes directly to the stub installer. I hope that by separating things out I have made it clearer.

This is still waiting on a final design.

Comment 12

[Verdi [:verdi] Best to slack me]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)
> 
> Dolske, gijs, is this something for a future funnelcake build or do we need
> it for this particular project?

Hi Liz - this is definitely something for the Fx51 funnelcake -  bug 1322718.

Comment 13

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 2. use release branding and build the stub installer

Comment 14

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Successful Try builds:

With backouts (patches 1 and 2):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2aa9d502c33cdc787f9fb1a4ba3919a2a4e5413

With new stub installer design (patches 1 through 4):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bce410b7e0abd4508ba5b1c68ff3460b3de7c35

Comment 15

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 4. Stub streamlining

Layout and timing adjustments

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

dolske mentioned a possible project branch and testing from that. Is that something you all are talking about with releng and QA?  
Catlee, do you know about this?  Can we do anything at the last minute to set up something for a funnelcake build?

Comment 17

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

The plan is to commit this to projects/oak, we should be able to build the stub installers for the funnelcake there.

I don't know what the plans are for QA, though, there are to my knowledge no automated tests for the installer.

Comment 18

[Verdi [:verdi] Best to slack me]

Attachment: stub-spec.png

Final design spec.

Comment 19

[Verdi [:verdi] Best to slack me]

Attachment: final-stub-background.png

Comment 20

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 3. binaries (image and icon resources, final bg)

Comment 21

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 4. Stub streamlining

Comment 22

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Build for testing

Comment 23

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Build for testing (fast cycle time)

Both this and the other test binary (attachment 8824627 [details]):
- are not signed
- do not submit a ping on completion
- do not contain a funnelcake id

Apart from that they should behave identically to the final delivery.

This build cycles every 2 seconds instead of every 20, to show all of the messages quickly.

Comment 24

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8824614 [details] [diff] [review]
3. binaries (image and icon resources, final bg)

The only change to 7zSD.sfx was to replace the icon with the newly added firefox64.ico via Resource Hacker 4.5.30. sha256 before (from ac7cef05200f) was
15a7bb2d0f08229017c97770350ee4896466d121c17be2e1029a96a61f1d01a4
and after
8cb1e4f35c4b40eafeba77baf9ca5c33e984e790ec42071490480a7e9ccb290e

Comment 25

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8823474 [details] [diff] [review]
1. Back out Win 7 restriction and 64-bit support

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

Per comment 17, I am reviewing these patches only for landing on oak (even though pushing to oak doesn't normally require review). They should not be pushed anywhere else, at least until a final decision has been made on whether to make the streamlined installer permanent.

This patch contains no new code, it just backs out some major behavior changes to the installer since 51, because the test of this installer will be run on 51.

Comment 26

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8824202 [details] [diff] [review]
2. use release branding and build the stub installer

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

See above re: landing these patches. This one is just config changes to force oak to build a stub installer and to use release branding.

Comment 27

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8824614 [details] [diff] [review]
3. binaries (image and icon resources, final bg)

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

Confirmed manually that the only change to 7zSD.sfx was the icon resources, and the new images are correct.

Comment 28

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8824615 [details] [diff] [review]
4. Stub streamlining

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

A few small-to-medium issues here, nothing catastrophic. If a few of these can't be done in time, it's not a big deal.

::: browser/branding/branding-common.mozbuild
@@ +17,5 @@
>          FINAL_TARGET_FILES.VisualElements += [
>              'VisualElements_150.png',
>              'VisualElements_70.png',
>          ]
>          BRANDING_FILES += [

Remove the files that aren't used anymore from this list (also from the one in Makefile.in, and probably also the actual files).

::: browser/installer/windows/nsis/stub.nsi
@@ +821,3 @@
>    ; Since the text color for controls is set in this Dialog the foreground and
>    ; background colors of the Dialog must also be hardcoded.
> +  SetCtlColors $Dialog 0xFFFFFF 0x0000c0

Keep these macro constants and just override them in this file, like you did with INSTALL_BLURB_TEXT_COLOR.

@@ +846,5 @@
> +!define __NSD_LabelRight_CLASS STATIC
> +!define __NSD_LabelRight_STYLE ${DEFAULT_STYLES}|${SS_NOTIFY}|${SS_RIGHT}
> +!define __NSD_LabelRight_EXSTYLE ${WS_EX_TRANSPARENT}
> +
> +  ${NSD_CreateLabelRight} -433u ${INSTALL_FOOTER_TOP_DU} 400u 20u "$(STUB_BLURB_FOOTER)"

Use nsDialogs::CreateControl instead of defining these macros just to add a style.

@@ +1261,5 @@
>      ${EndIf}
>    ${EndIf}
> +
> +  StrCpy $0 "200" ; DownloadIntervalMS is 200
> +  Call UpdateBlurb

Did you try giving UpdateBlurb its own timer, instead of having to tickle it periodically like this?

::: browser/locales/en-US/installer/nsisstrings.properties
@@ +23,2 @@
>  
>  INTRO_BLURB1=Thanks for choosing $BrandFullName, the browser that chooses you above everything else.

Go ahead and remove the strings that we're no longer using. Just need to be sure not to use any of their names.

@@ +35,5 @@
>  WARN_MIN_SUPPORTED_OSVER_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires ${MinSupportedVer} or newer. Please click the OK button for additional information.
>  WARN_MIN_SUPPORTED_CPU_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires a processor with ${MinSupportedCPU} support. Please click the OK button for additional information.
>  WARN_MIN_SUPPORTED_OSVER_CPU_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires ${MinSupportedVer} or newer and a processor with ${MinSupportedCPU} support. Please click the OK button for additional information.
>  WARN_WRITE_ACCESS=You don't have access to write to the installation directory.\n\nClick OK to select a different directory.
> +WARN_WRITE_ACCESS_QUIT=You don't have access to write to the installation directory.

The messages about the installation directory should indicate what directory we tried to use.

Comment 29

[Justin Dolske [:Dolske]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)

> Dolske, gijs, is this something for a future funnelcake build or do we need
> it for this particular project?

My understanding is that this is up to Verdi and the stub installer team to coordinate with releng, since it requires a custom funnelcake build to ship a modified stub installer. Otherwise, the work in bug 1322718 is just a standard & simple repack-based funnelcake.

Comment 30

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 4. Stub streamlining, revised

Comment 31

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Try build on these latest patches was successful:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c13351cd98e818e1cd27f717082bea8c6560ddf9

Comment 32

[Chris AtLee [:catlee]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16)
> dolske mentioned a possible project branch and testing from that. Is that
> something you all are talking about with releng and QA?  
> Catlee, do you know about this?  Can we do anything at the last minute to
> set up something for a funnelcake build?

(In reply to Adam Gashlin [:agashlin] from comment #17)
> The plan is to commit this to projects/oak, we should be able to build the
> stub installers for the funnelcake there.
> 
> I don't know what the plans are for QA, though, there are to my knowledge no
> automated tests for the installer.

Comment 33

[Nick Thomas [:nthomas] (UTC+12)]

Try/Oak are a good place to do development work and initial QA. The nightly builds on Oak are signed with the same certificate used for releases, while dep builds on Oak and Try use a Mozilla-generated certificate. The stub checks the signing of the full installer it downloads so stub from try is likely to fail (see CertNameDownload, CertIssuerDownload, and URLStubDownload in dxr). Nightly builds can be started at https://secure.pub.build.mozilla.org/buildapi/self-serve/oak - you can cancel unnecessary platforms after they start - otherwise they run at 4am Pacific if the code has changed.

Once QA and yourselves are happy with Oak we'd recommend packaging the changes up to be landed on a branch in mozilla-release. I think this makes it much clearer when we look back from the future - ie that the code used was 51.0 (or 51.0.x) with a small set of modifications, rather the state of oak from <some date> plus changes. Patches 1 and 2 wouldn't be needed in m-r.

BTW, we'd would need to modify the FunnelcakeVersion variable at https://dxr.mozilla.org/mozilla-beta/source/browser/installer/windows/nsis/defines.nsi.in#11 to ensure we download the funnelcake build. Not for testing, but for the final stub.

Comment 34

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

(In reply to Nick Thomas (UTC+13) [:nthomas] from comment #33)
> The stub checks the signing of the full installer it downloads so stub from try is likely to fail

URLStubDownload is always set so the stub goes to download.mozilla.org/?os=win&lang=${AB_CD}&product=firefox${whatever}-latest to get the latest genuine signed build for a channel, so it works fine built from try. Not that we will use that beyond testing. (Am I misunderstanding your concern here?)

> Nightly builds can be started at
> https://secure.pub.build.mozilla.org/buildapi/self-serve/oak - you can
> cancel unnecessary platforms after they start - otherwise they run at 4am
> Pacific if the code has changed.

Good to know, thanks!

> Once QA and yourselves are happy with Oak we'd recommend packaging the
> changes up to be landed on a branch in mozilla-release.

This makes a lot of sense, I agree. Will you be able to help us with the branch when the time comes?

> BTW, we'd would need to modify the FunnelcakeVersion variable

Right, I think (as you've mention by email) we'll actually need two branches here, one for these patches with FunnelcakeVersion 99, the other just setting the FunnelcakeVersion to 98 for the control. Or vice versa, I've only heard about 98/99, don't know which is to be which.

Comment 35

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to Adam Gashlin [:agashlin] from comment #34)
> URLStubDownload is always set so the stub goes to
> download.mozilla.org/?os=win&lang=${AB_CD}&product=firefox${whatever}-latest
> to get the latest genuine signed build for a channel, so it works fine built
> from try. Not that we will use that beyond testing. (Am I misunderstanding
> your concern here?)

Yes, I think you're right. I got tangled up in the cert details and forgot about the URL it'd be using.
 
> This makes a lot of sense, I agree. Will you be able to help us with the
> branch when the time comes?

Yes, no problem. To follow the one earlier case we'd use a branch name of FUNNELCAKE99_BRANCH.
 
> Right, I think (as you've mention by email) we'll actually need two branches
> here, one for these patches with FunnelcakeVersion 99, the other just
> setting the FunnelcakeVersion to 98 for the control. Or vice versa, I've
> only heard about 98/99, don't know which is to be which.

cmore has chosen 98 for the control, 99 for the experiment. For anyone following along, there's discussion elsewhere on if we need a custom stub & funnelcake full installer for the control, or can use the stub attribution project.

Comment 36

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: 4. Stub streamlining

Changing the error/quit message as per email discussion with verdi.

Comment 37

[Molly Howell (she/her) (mostly inactive)]

https://hg.mozilla.org/projects/oak/rev/d484aeab6c82b0e0997ba9dd0871f9d8a0b10e2e
Bug 1328445 Part -1 - Backout Windows 7 restriction and 64-bit support in the stub installer

https://hg.mozilla.org/projects/oak/rev/4c48901c30969f471b5ce931c748a429fc653722
Bug 1328445 Part 0 - Configure for release branding and enable stub installer

https://hg.mozilla.org/projects/oak/rev/f0f776eb939b284ba54ac03788e24f246c27dd85
Bug 1328445 Part 1 - Image files for stramlined stub installer

https://hg.mozilla.org/projects/oak/rev/afca607e9cac2b3ed2d59c970943c165556ee176
Bug 1328445 Part 2 - Streamlined stub installer

Comment 38

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Here is the signed "nightly" build from Matt's comment 37 push to Oak:

https://archive.mozilla.org/pub/firefox/nightly/2017/01/2017-01-13-04-02-04-oak/firefox-53.0a1.en-US.win32.installer-stub.exe

Comment 39

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

It looks like we will be doing three different funnelcake ids, 98 as the unmodified stub, 99 and 100 as streamlined, each pulling down a differently configured full install by the funnelcake id mechanism.

As Nick suggested in comment 33 we plan on doing the final builds from a release branch after 51 is released. I wanted to run this by Release Management to see if there are any objections or preparations needed in advance. Patches 3 & 4 on this bug should be all we need besides a one-line patch to set the funnelcake id, and they apply fine against beta right now (which was expected, as patch 1 was for rolling m-c back to beta).

Comment 40

[Liz Henry (:lizzard) (relman/hg->git project)]

We just released an RC2 desktop build (on the beta channel but built from the mozilla-release repo).  So, if you were testing against builds from mozilla-beta, that is not perfectly in sync at this point with what we will be releasing. Once you have your new builds will you also be testing them or do you have testing lined up? Thanks!

Comment 41

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Ok, I just checked and everything is fine with current mozilla-release as well, I didn't expect any surprises in the installer. I've been doing manual testing of the stub on VMs for XP, Win 7, and Win 10, and I'll do that again for the new builds.

I've seen mention of a QA testing period of one week for another aspect of the onboarding experiment
https://bugzilla.mozilla.org/show_bug.cgi?id=1326252#c7
but I'm not sure if that will include the stub as well, Chris can you answer this?

Comment 42

[Romain Testard [:RT]]

It should include the stub as well. QA is involved on this bug for funnelcake tests : bug 1322718

Comment 43

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I've changed plans slightly and instead put the funnelcake id patches on separate bug 1333873.

Comment 44

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

This was landed on a project branch in bug 1326252 comment 17, builds are available for testing in bug 1326252 comment 27. All of my testing indicates that the requirements are met, if more work is needed it might make sense to open another bug for the specifics.