Bug 1376597 Part 2 - Simplify progress bar handling. r?agashlin draft
authorMatt Howell <mhowell@mozilla.com>
Tue, 15 Aug 2017 12:43:44 -0700
changeset 650502 ee1a898b40fb83c70be248368987ab1c804c22e4
parent 650501 18c3463f5057c5b8b1de590c714156cf56d3541f
child 727431 08e7d65de9933b5eeec956c29255bc266e1bf5ce
push id75423
push usermhowell@mozilla.com
push dateTue, 22 Aug 2017 14:50:38 +0000
reviewersagashlin
bugs1376597
milestone57.0a1
Bug 1376597 Part 2 - Simplify progress bar handling. r?agashlin Working on the main patch for this bug (part 1), it took me longer than seemed reasonable to understand how the stub installer progress bar worked and to fit the new stage into it. So I thought I would take the opportunity to attempt a refactor and simplify the whole thing. MozReview-Commit-ID: 9INP1Hgfiuq
browser/installer/windows/nsis/stub.nsi
--- a/browser/installer/windows/nsis/stub.nsi
+++ b/browser/installer/windows/nsis/stub.nsi
@@ -42,28 +42,25 @@ Var FontInstalling
 Var FontBlurb
 Var FontFooter
 Var FontCheckbox
 
 Var CanWriteToInstallDir
 Var HasRequiredSpaceAvailable
 Var IsDownloadFinished
 Var DownloadSizeBytes
-Var HalfOfDownload
 Var DownloadReset
 Var ExistingTopDir
 Var SpaceAvailableBytes
 Var InitialInstallDir
 Var HandleDownload
 Var CanSetAsDefault
 Var InstallCounterStep
-Var InstallStepSize
 Var InstallTotalSteps
 Var ProgressCompleted
-Var ProgressTotal
 
 Var ExitCode
 Var FirefoxLaunchCode
 
 Var StartDownloadPhaseTickCount
 ; Since the Intro and Options pages can be displayed multiple times the total
 ; seconds spent on each of these pages is reported.
 Var IntroPhaseSeconds
@@ -90,17 +87,16 @@ Var OpenedDownloadPage
 Var DownloadServerIP
 Var PostSigningData
 Var PreviousInstallDir
 Var PreviousInstallArch
 Var ProfileCleanupPromptType
 Var ProfileCleanupHeaderString
 Var ProfileCleanupButtonString
 Var AppLaunchWaitTickCount
-Var AppLaunchWaitStepSize
 
 ; Uncomment the following to prevent pinging the metrics server when testing
 ; the stub installer
 ;!define STUB_DEBUG
 
 !define StubURLVersion "v8"
 
 ; Successful install exit code
@@ -156,55 +152,42 @@ Var AppLaunchWaitStepSize
 !define DownloadRetryIntervalMS 3000
 
 ; Interval for the download timer
 !define DownloadIntervalMS 200
 
 ; Interval for the install timer
 !define InstallIntervalMS 100
 
-; The first step for the install progress bar. By starting with a large step
-; immediate feedback is given to the user.
-!define InstallProgressFirstStep 20
-
-; The finish step size to quickly increment the progress bar after the
-; installation has finished.
-!define InstallProgressFinishStep 40
-
 ; Number of steps for the install progress.
 ; This might not be enough when installing on a slow network drive so it will
-; fallback to downloading the full installer if it reaches this number. The size
-; of the install progress step is increased when the full installer finishes
-; instead of waiting.
+; fallback to downloading the full installer if it reaches this number.
 
-; Approximately 150 seconds with a 100 millisecond timer and a first step of 20
-; as defined by InstallProgressFirstStep.
-!define /math InstallCleanTotalSteps ${InstallProgressFirstStep} + 1500
+; Approximately 150 seconds with a 100 millisecond timer.
+!define InstallCleanTotalSteps 1500
 
-; Approximately 165 seconds (minus 0.2 seconds for each file that is removed)
-; with a 100 millisecond timer and a first step of 20 as defined by
-; InstallProgressFirstStep .
-!define /math InstallPaveOverTotalSteps ${InstallProgressFirstStep} + 1800
+; Approximately 165 seconds with a 100 millisecond timer.
+!define InstallPaveOverTotalSteps 1650
 
-; 5 more seconds at the end of the progress bar to wait for the app to launch.
-!define AppLaunchWaitSteps 500
-
-; Number of steps per interval to advance the progress bar. Calibrate this based
-; on how long the progress bar should spend filling up while awaiting app launch.
-!define AppLaunchWaitStepSizeMultiplier 20
+; Blurb duty cycle
+!define BlurbDisplayMS 19500
+!define BlurbBlankMS 500
 
 ; Interval between checks for the application window and progress bar updates.
 !define AppLaunchWaitIntervalMS 100
 
 ; Total time to wait for the application to start before just exiting.
 !define AppLaunchWaitTimeoutMS 10000
 
-; Blurb duty cycle
-!define BlurbDisplayMS 19500
-!define BlurbBlankMS 500
+; Maximum value of the download/install/launch progress bar, and the end values
+; for each individual stage.
+!define PROGRESS_BAR_TOTAL_STEPS 500 
+!define PROGRESS_BAR_DOWNLOAD_END_STEP 300
+!define PROGRESS_BAR_INSTALL_END_STEP 475
+!define PROGRESS_BAR_APP_LAUNCH_END_STEP 500
 
 ; Amount of physical memory required for the 64-bit build to be selected (2 GB).
 ; Machines with this or less RAM get the 32-bit build, even with a 64-bit OS.
 !define RAM_NEEDED_FOR_64BIT 0x80000000
 
 ; Attempt to elevate Standard Users in addition to users that
 ; are a member of the Administrators group.
 !define NONADMIN_ELEVATE
@@ -558,17 +541,16 @@ Function .onGUIEnd
   ${UnloadUAC}
 FunctionEnd
 
 Function .onUserAbort
   ${NSD_KillTimer} StartDownload
   ${NSD_KillTimer} OnDownload
   ${NSD_KillTimer} CheckInstall
   ${NSD_KillTimer} FinishInstall
-  ${NSD_KillTimer} FinishProgressBar
   ${NSD_KillTimer} DisplayDownloadError
   ${NSD_KillTimer} NextBlurb
   ${NSD_KillTimer} ClearBlurb
 
   ${If} "$IsDownloadFinished" != ""
     Call DisplayDownloadError
   ${Else}
     Call SendPing
@@ -899,27 +881,17 @@ Function StartDownload
   ${NSD_CreateTimer} OnDownload ${DownloadIntervalMS}
   ${If} ${FileExists} "$INSTDIR\${TO_BE_DELETED}"
     RmDir /r "$INSTDIR\${TO_BE_DELETED}"
   ${EndIf}
 FunctionEnd
 
 Function SetProgressBars
   SendMessage $Progressbar ${PBM_SETPOS} $ProgressCompleted 0
-  ${ITBL3SetProgressValue} "$ProgressCompleted" "$ProgressTotal"
-FunctionEnd
-
-Function RemoveFileProgressCallback
-  IntOp $InstallCounterStep $InstallCounterStep + 2
-  System::Int64Op $ProgressCompleted + $InstallStepSize
-  Pop $ProgressCompleted
-  Call SetProgressBars
-  System::Int64Op $ProgressCompleted + $InstallStepSize
-  Pop $ProgressCompleted
-  Call SetProgressBars
+  ${ITBL3SetProgressValue} "$ProgressCompleted" "${PROGRESS_BAR_TOTAL_STEPS}"
 FunctionEnd
 
 Function NextBlurb
   ${NSD_KillTimer} NextBlurb
 
   IntOp $CurrentBlurbIdx $CurrentBlurbIdx + 1
   IntOp $CurrentBlurbIdx $CurrentBlurbIdx % 3
 
@@ -1015,31 +987,20 @@ Function OnDownload
         ${NSD_CreateTimer} DisplayDownloadError ${InstallIntervalMS}
       ${Else}
         ${NSD_CreateTimer} StartDownload ${DownloadIntervalMS}
       ${EndIf}
       Return
     ${EndIf}
 
     StrCpy $DownloadSizeBytes "$4"
-    System::Int64Op $4 / 2
-    Pop $HalfOfDownload
-    System::Int64Op $HalfOfDownload / $InstallTotalSteps
-    Pop $InstallStepSize
-    StrCpy $AppLaunchWaitStepSize $InstallStepSize
     SendMessage $Progressbar ${PBM_SETMARQUEE} 0 0 ; start=1|stop=0 interval(ms)=+N
     ${RemoveStyle} $Progressbar ${PBS_MARQUEE}
-    System::Int64Op $HalfOfDownload + $DownloadSizeBytes
-    Pop $ProgressTotal
-    System::Int64Op $AppLaunchWaitStepSize * ${AppLaunchWaitSteps}
-    Pop $R0
-    System::Int64Op $ProgressTotal + $R0
-    Pop $ProgressTotal
     StrCpy $ProgressCompleted 0
-    SendMessage $Progressbar ${PBM_SETRANGE32} $ProgressCompleted $ProgressTotal
+    SendMessage $Progressbar ${PBM_SETRANGE32} $ProgressCompleted ${PROGRESS_BAR_TOTAL_STEPS}
   ${EndIf}
 
   ; Don't update the status until after the download starts
   ${If} $2 != 0
   ${AndIf} "$4" == ""
     Return
   ${EndIf}
 
@@ -1057,20 +1018,16 @@ Function OnDownload
     ${EndIf}
     Return
   ${EndIf}
 
   ${If} $IsDownloadFinished != "true"
     ${If} $2 == 0
       ${NSD_KillTimer} OnDownload
       StrCpy $IsDownloadFinished "true"
-      ; The first step of the install progress bar is determined by the
-      ; InstallProgressFirstStep define and provides the user with immediate
-      ; feedback.
-      StrCpy $InstallCounterStep "${InstallProgressFirstStep}"
       System::Call "kernel32::GetTickCount()l .s"
       Pop $EndDownloadPhaseTickCount
 
       StrCpy $DownloadedBytes "$DownloadSizeBytes"
 
       ; When a download has finished handle the case where the  downloaded size
       ; is less than the minimum expected size or greater than the maximum
       ; expected size during the download.
@@ -1085,23 +1042,19 @@ Function OnDownload
         ${Else}
           ${NSD_CreateTimer} StartDownload ${DownloadIntervalMS}
         ${EndIf}
         Return
       ${EndIf}
 
       ; Update the progress bars first in the UI change so they take affect
       ; before other UI changes.
-      StrCpy $ProgressCompleted "$DownloadSizeBytes"
+      StrCpy $ProgressCompleted "${PROGRESS_BAR_DOWNLOAD_END_STEP}"
       Call SetProgressBars
-      System::Int64Op $InstallStepSize * ${InstallProgressFirstStep}
-      Pop $R9
-      System::Int64Op $ProgressCompleted + $R9
-      Pop $ProgressCompleted
-      Call SetProgressBars
+
       ; Disable the Cancel button during the install
       GetDlgItem $5 $HWNDPARENT 2
       EnableWindow $5 0
 
       ; Open a handle to prevent modification of the full installer
       StrCpy $R9 "${INVALID_HANDLE_VALUE}"
       System::Call 'kernel32::CreateFileW(w "$PLUGINSDIR\download.exe", \
                                           i ${GENERIC_READ}, \
@@ -1170,18 +1123,17 @@ Function OnDownload
       WriteINIStr "$PLUGINSDIR\${CONFIG_INI}" "Install" "MaintenanceService" "false"
 !endif
 
       ; Delete the taskbar shortcut history to ensure we do the right thing based on
       ; the config file above.
       ${GetShortcutsLogPath} $0
       Delete "$0"
 
-      GetFunctionAddress $0 RemoveFileProgressCallback
-      ${RemovePrecompleteEntries} $0
+      ${RemovePrecompleteEntries} "false"
 
       ; Delete the install.log and let the full installer create it. When the
       ; installer closes it we can detect that it has completed.
       Delete "$INSTDIR\install.log"
 
       ; Delete firefox.exe.moz-upgrade and firefox.exe.moz-delete if it exists
       ; since it being present will require an OS restart for the full
       ; installer.
@@ -1189,22 +1141,21 @@ Function OnDownload
       Delete "$INSTDIR\${FileMainEXE}.moz-delete"
 
       System::Call "kernel32::GetTickCount()l .s"
       Pop $EndPreInstallPhaseTickCount
 
       Exec "$\"$PLUGINSDIR\download.exe$\" /INI=$PLUGINSDIR\${CONFIG_INI}"
       ${NSD_CreateTimer} CheckInstall ${InstallIntervalMS}
     ${Else}
-      ${If} $HalfOfDownload != "true"
-      ${AndIf} $3 > $HalfOfDownload
-        StrCpy $HalfOfDownload "true"
-      ${EndIf}
       StrCpy $DownloadedBytes "$3"
-      StrCpy $ProgressCompleted "$DownloadedBytes"
+      System::Int64Op $DownloadedBytes * ${PROGRESS_BAR_DOWNLOAD_END_STEP}
+      Pop $ProgressCompleted
+      System::Int64Op $ProgressCompleted / $DownloadSizeBytes
+      Pop $ProgressCompleted
       Call SetProgressBars
     ${EndIf}
   ${EndIf}
 FunctionEnd
 
 Function SendPing
   ${NSD_KillTimer} NextBlurb
   ${NSD_KillTimer} ClearBlurb
@@ -1480,18 +1431,17 @@ Function CheckInstall
     ; Close the handle that prevents modification of the full installer
     System::Call 'kernel32::CloseHandle(i $HandleDownload)'
     StrCpy $ExitCode "${ERR_INSTALL_TIMEOUT}"
     ; Use a timer so the UI has a chance to update
     ${NSD_CreateTimer} DisplayDownloadError ${InstallIntervalMS}
     Return
   ${EndIf}
 
-  System::Int64Op $ProgressCompleted + $InstallStepSize
-  Pop $ProgressCompleted
+  IntOp $ProgressCompleted $ProgressCompleted + 1
   Call SetProgressBars
 
   ${If} ${FileExists} "$INSTDIR\install.log"
     Delete "$INSTDIR\install.tmp"
     CopyFiles /SILENT "$INSTDIR\install.log" "$INSTDIR\install.tmp"
 
     ; The unfocus and refocus that happens approximately here is caused by the
     ; installer calling SHChangeNotify to refresh the shortcut icons.
@@ -1504,62 +1454,32 @@ Function CheckInstall
       ${NSD_KillTimer} CheckInstall
       ; Close the handle that prevents modification of the full installer
       System::Call 'kernel32::CloseHandle(i $HandleDownload)'
       Rename "$INSTDIR\install.tmp" "$INSTDIR\install.log"
       Delete "$PLUGINSDIR\download.exe"
       Delete "$PLUGINSDIR\${CONFIG_INI}"
       System::Call "kernel32::GetTickCount()l .s"
       Pop $EndInstallPhaseTickCount
-      System::Int64Op $InstallStepSize * ${InstallProgressFinishStep}
-      Pop $InstallStepSize
-      ${NSD_CreateTimer} FinishInstall ${InstallIntervalMS}
+      Call FinishInstall
     ${EndUnless}
   ${EndIf}
 FunctionEnd
 
 Function FinishInstall
-  ; The full installer has completed but the progress bar still needs to finish
-  ; so increase the size of the step.
-  IntOp $InstallCounterStep $InstallCounterStep + ${InstallProgressFinishStep}
-  ${If} $InstallTotalSteps < $InstallCounterStep
-    StrCpy $InstallCounterStep "$InstallTotalSteps"
-  ${EndIf}
-
-  ${If} $InstallTotalSteps != $InstallCounterStep
-    System::Int64Op $ProgressCompleted + $InstallStepSize
-    Pop $ProgressCompleted
-    Call SetProgressBars
-    Return
-  ${EndIf}
-
-  ${NSD_KillTimer} FinishInstall
+  StrCpy $ProgressCompleted "${PROGRESS_BAR_INSTALL_END_STEP}"
+  Call SetProgressBars
 
   ${If} ${FileExists} "$INSTDIR\${FileMainEXE}.moz-upgrade"
     Delete "$INSTDIR\${FileMainEXE}"
     Rename "$INSTDIR\${FileMainEXE}.moz-upgrade" "$INSTDIR\${FileMainEXE}"
   ${EndIf}
 
   StrCpy $ExitCode "${ERR_SUCCESS}"
 
-  StrCpy $InstallCounterStep 0
-  ${NSD_CreateTimer} FinishProgressBar ${InstallIntervalMS}
-FunctionEnd
-
-Function FinishProgressBar
-  IntOp $InstallCounterStep $InstallCounterStep + 1
-
-  ${If} $InstallCounterStep < 10
-    Return
-  ${EndIf}
-
-  ${NSD_KillTimer} FinishProgressBar
-  ${NSD_KillTimer} NextBlurb
-  ${NSD_KillTimer} ClearBlurb
-
   Call CopyPostSigningData
   Call LaunchApp
 FunctionEnd
 
 Function RelativeGotoPage
   IntCmp $R9 0 0 Move Move
   StrCmp $R9 "X" 0 Move
   StrCpy $R9 "120"
@@ -1691,37 +1611,35 @@ Function LaunchAppFromElevatedProcess
 FunctionEnd
 
 Function WaitForAppLaunch
   FindWindow $0 "${MainWindowClass}"
   FindWindow $1 "${DialogWindowClass}"
   ${If} $0 <> 0
   ${OrIf} $1 <> 0
     ${NSD_KillTimer} WaitForAppLaunch
-    StrCpy $ProgressCompleted "$ProgressTotal"
+    StrCpy $ProgressCompleted "${PROGRESS_BAR_APP_LAUNCH_END_STEP}"
     Call SetProgressBars
     Call SendPing
     Return
   ${EndIf}
 
   IntOp $AppLaunchWaitTickCount $AppLaunchWaitTickCount + 1
   IntOp $0 $AppLaunchWaitTickCount * ${AppLaunchWaitIntervalMS}
   ${If} $0 >= ${AppLaunchWaitTimeoutMS}
     ; We've waited an unreasonably long time, so just exit.
     ${NSD_KillTimer} WaitForAppLaunch
     Call SendPing
     Return
   ${EndIf}
 
-  ${If} $AppLaunchWaitTickCount < ${AppLaunchWaitSteps}
-    IntOp $0 $AppLaunchWaitStepSize * ${AppLaunchWaitStepSizeMultiplier}
-    System::Int64Op $ProgressCompleted + $0
-    Pop $ProgressCompleted
+  ${If} $ProgressCompleted < ${PROGRESS_BAR_APP_LAUNCH_END_STEP}
+    IntOp $ProgressCompleted $ProgressCompleted + 1
+    Call SetProgressBars
   ${EndIf}
-  Call SetProgressBars
 FunctionEnd
 
 Function CopyPostSigningData
   ${LineRead} "$EXEDIR\postSigningData" "1" $PostSigningData
   ${If} ${Errors}
     ClearErrors
     StrCpy $PostSigningData "0"
   ${Else}