Closed Bug 1405438 Opened 7 years ago Closed 7 years ago

Stub installer exit code never initialized if exited from profile cleanup prompt

Categories

(Firefox :: Installer, defect, P1)

Product:
Firefox
Component:
Installer
Platform:
All
Windows
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: molly, Assigned: molly)

References

Details

Attachments

(1 file)

Bug 1405438 - Make sure the stub installer exit code is initialized before the user can exit it.
59 bytes, text/x-review-board-request
agashlin
: review+
ritu
: approval-mozilla-beta+
Details 
If a user exits the stub installer from the profile cleanup prompt (bug 1369255), we still send a ping to record that the prompt was shown. However, the installer exit status code is not initialized in that case, and so these pings appear as failed installs with an error of 'Other'. We should initialize ExitCode to something meaningful so that we can differentiate those pings.
Status: NEW → ASSIGNED
status-firefox57: --- → affected
Comment on attachment 8914918 [details]
Bug 1405438 - Make sure the stub installer exit code is initialized before the user can exit it.

https://reviewboard.mozilla.org/r/186162/#review191250

::: browser/installer/windows/nsis/stub.nsi:1175
(Diff revision 3)
>      ; When the value of $IsDownloadFinished is false the download was started
>      ; but didn't finish. In this case the tick count stored in
>      ; $EndFinishPhaseTickCount is used to determine how long the download was
>      ; in progress.
>      ${If} "$IsDownloadFinished" == "false"
>      ${OrIf} "$EndDownloadPhaseTickCount" == ""

Maybe remove this as it will now never be encountered?
Attachment #8914918 - Flags: review?(agashlin) → review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a34b7b78438
Make sure the stub installer exit code is initialized before the user can exit it. r=agashlin
https://hg.mozilla.org/mozilla-central/rev/4a34b7b78438
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8914918 [details]
Bug 1405438 - Make sure the stub installer exit code is initialized before the user can exit it.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1369255

[User impact if declined]:
Incorrect/misleading stub installer ping submissions.

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Yes, I manually verified the fix, and several days worth of pings submitted from nightly seem to show the fix is working (https://sql.telemetry.mozilla.org/queries/4692#9520).

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
This is a low-risk patch because all it does is move up the point where a few variables get initialized. This patch has been on nightly for several days, the data starting from the time it landed there shows clear improvement, and there have been no reports of issues.

[String changes made/needed]:
None
Attachment #8914918 - Flags: approval-mozilla-beta?
Comment on attachment 8914918 [details]
Bug 1405438 - Make sure the stub installer exit code is initialized before the user can exit it.

Low risk, awesome job(!) verifying telemetry data from Nightly, Beta57+
Attachment #8914918 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Matt Howell [:mhowell] from comment #8)

> [Needs manual test from QE? If yes, steps to reproduce]: 
> No

Marking this as qe-verify - based on Matt's assessment on manual testing needs.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: