Merge pull request desktop#15416 from desktop/update-before-closing

Prevent closing the app while it's updating

app/src/lib/feature-flag.ts

@@ -117,3 +117,8 @@ export function enableStartingPullRequests(): boolean {
 export function enableStackedPopups(): boolean {
   return enableDevelopmentFeatures()
 }
+
+/** Should we enable mechanism to prevent closing while the app is updating? */
+export function enablePreventClosingWhileUpdating(): boolean {
+  return enableBetaFeatures()
+}

app/src/lib/ipc-shared.ts

@@ -46,6 +46,8 @@ export type RequestChannels = {
   'menu-event': (name: MenuEvent) => void
   log: (level: LogLevel, message: string) => void
   'will-quit': () => void
+  'will-quit-even-if-updating': () => void
+  'cancel-quitting': () => void
   'crash-ready': () => void
   'crash-quit': () => void
   'window-state-changed': (windowState: WindowState) => void
@@ -63,6 +65,7 @@ export type RequestChannels = {
   blur: () => void
   'update-accounts': (accounts: ReadonlyArray<EndpointToken>) => void
   'quit-and-install-updates': () => void
+  'quit-app': () => void
   'minimize-window': () => void
   'maximize-window': () => void
   'unmaximize-window': () => void
@@ -77,6 +80,7 @@ export type RequestChannels = {
   'focus-window': () => void
   'notification-event': NotificationCallback<DesktopAliveEvent>
   'set-window-zoom-factor': (zoomFactor: number) => void
+  'show-installing-update': () => void
 }
 
 /**

app/src/lib/stores/app-store.ts

@@ -77,6 +77,10 @@ import {
   updatePreferredAppMenuItemLabels,
   updateAccounts,
   setWindowZoomFactor,
+  onShowInstallingUpdate,
+  sendWillQuitEvenIfUpdatingSync,
+  quitApp,
+  sendCancelQuittingSync,
 } from '../../ui/main-process-proxy'
 import {
   API,
@@ -585,6 +589,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
     this.notificationsStore.onPullRequestReviewSubmitNotification(
       this.onPullRequestReviewSubmitNotification
     )
+
+    onShowInstallingUpdate(this.onShowInstallingUpdate)
   }
 
   private initializeWindowState = async () => {
@@ -655,6 +661,12 @@ export class AppStore extends TypedBaseStore<IAppState> {
     })
   }
 
+  private onShowInstallingUpdate = () => {
+    this._showPopup({
+      type: PopupType.InstallingUpdate,
+    })
+  }
+
   /** Figure out what step of the tutorial the user needs to do next */
   private async updateCurrentTutorialStep(
     repository: Repository
@@ -7473,6 +7485,18 @@ export class AppStore extends TypedBaseStore<IAppState> {
       }
     )
   }
+
+  public _quitApp(evenIfUpdating: boolean) {
+    if (evenIfUpdating) {
+      sendWillQuitEvenIfUpdatingSync()
+    }
+
+    quitApp()
+  }
+
+  public _cancelQuittingApp() {
+    sendCancelQuittingSync()
+  }
 }
 
 /**

app/src/main-process/app-window.ts

@@ -26,13 +26,15 @@ import {
   terminateDesktopNotifications,
 } from './notifications'
 import { addTrustedIPCSender } from './trusted-ipc-sender'
+import { enablePreventClosingWhileUpdating } from '../lib/feature-flag'
 
 export class AppWindow {
   private window: Electron.BrowserWindow
   private emitter = new Emitter()
 
   private _loadTime: number | null = null
   private _rendererReadyTime: number | null = null
+  private isDownloadingUpdate: boolean = false
 
   private minWidth = 960
   private minHeight = 660
@@ -86,6 +88,7 @@ export class AppWindow {
     this.shouldMaximizeOnShow = savedWindowState.isMaximized
 
     let quitting = false
+    let quittingEvenIfUpdating = false
     app.on('before-quit', () => {
       quitting = true
     })
@@ -95,7 +98,40 @@ export class AppWindow {
       event.returnValue = true
     })
 
+    ipcMain.on('will-quit-even-if-updating', event => {
+      quitting = true
+      quittingEvenIfUpdating = true
+      event.returnValue = true
+    })
+
+    ipcMain.on('cancel-quitting', event => {
+      quitting = false
+      quittingEvenIfUpdating = false
+      event.returnValue = true
+    })
+
     this.window.on('close', e => {
+      // On macOS, closing the window doesn't mean the app is quitting. If the
+      // app is updating, we will prevent the window from closing only when the
+      // app is also quitting.
+      if (
+        enablePreventClosingWhileUpdating() &&
+        (!__DARWIN__ || quitting) &&
+        !quittingEvenIfUpdating &&
+        this.isDownloadingUpdate
+      ) {
+        e.preventDefault()
+        ipcWebContents.send(this.window.webContents, 'show-installing-update')
+
+        // Make sure the window is visible, so the user can see why we're
+        // preventing the app from quitting. This is important on macOS, where
+        // the window could be hidden/closed when the user tries to quit.
+        // It could also happen on Windows if the user quits the app from the
+        // task bar while it's in the background.
+        this.show()
+        return
+      }
+
       // on macOS, when the user closes the window we really just hide it. This
       // lets us activate quickly and keep all our interesting logic in the
       // renderer.
@@ -213,7 +249,7 @@ export class AppWindow {
     return !!this.loadTime && !!this.rendererReadyTime
   }
 
-  public onClose(fn: () => void) {
+  public onClosed(fn: () => void) {
     this.window.on('closed', fn)
   }
 
@@ -344,31 +380,36 @@ export class AppWindow {
 
   public setupAutoUpdater() {
     autoUpdater.on('error', (error: Error) => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(this.window.webContents, 'auto-updater-error', error)
     })
 
     autoUpdater.on('checking-for-update', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-checking-for-update'
       )
     })
 
     autoUpdater.on('update-available', () => {
+      this.isDownloadingUpdate = true
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-available'
       )
     })
 
     autoUpdater.on('update-not-available', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-not-available'
       )
     })
 
     autoUpdater.on('update-downloaded', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-downloaded'

app/src/main-process/main.ts

@@ -490,6 +490,8 @@ app.on('ready', () => {
     mainWindow?.quitAndInstallUpdate()
   )
 
+  ipcMain.on('quit-app', () => app.quit())
+
   ipcMain.on('minimize-window', () => mainWindow?.minimizeWindow())
 
   ipcMain.on('maximize-window', () => mainWindow?.maximizeWindow())
@@ -738,7 +740,7 @@ function createWindow() {
     }
   }
 
-  window.onClose(() => {
+  window.onClosed(() => {
     mainWindow = null
     if (!__DARWIN__ && !preventQuit) {
       app.quit()

app/src/models/popup.ts

@@ -87,6 +87,7 @@ export enum PopupType {
   PullRequestReview = 'PullRequestReview',
   UnreachableCommits = 'UnreachableCommits',
   StartPullRequest = 'StartPullRequest',
+  InstallingUpdate = 'InstallingUpdate',
 }
 
 interface IBasePopup {
@@ -379,5 +380,8 @@ export type PopupDetail =
       nonLocalCommitSHA: string | null
       showSideBySideDiff: boolean
     }
+  | {
+      type: PopupType.InstallingUpdate
+    }
 
 export type Popup = IBasePopup & PopupDetail

app/src/ui/app.tsx

@@ -163,6 +163,7 @@ import { UnreachableCommitsDialog } from './history/unreachable-commits-dialog'
 import { OpenPullRequestDialog } from './open-pull-request/open-pull-request-dialog'
 import { sendNonFatalException } from '../lib/helpers/non-fatal-exception'
 import { createCommitURL } from '../lib/commit-url'
+import { InstallingUpdate } from './installing-update/installing-update'
 
 const MinuteInMilliseconds = 1000 * 60
 const HourInMilliseconds = MinuteInMilliseconds * 60
@@ -2313,6 +2314,15 @@ export class App extends React.Component<IAppProps, IAppState> {
           />
         )
       }
+      case PopupType.InstallingUpdate: {
+        return (
+          <InstallingUpdate
+            key="installing-update"
+            dispatcher={this.props.dispatcher}
+            onDismissed={onPopupDismissedFn}
+          />
+        )
+      }
       default:
         return assertNever(popup, `Unknown popup type: ${popup}`)
     }

app/src/ui/dispatcher/dispatcher.ts

@@ -4009,4 +4009,24 @@ export class Dispatcher {
   public updatePullRequestBaseBranch(repository: Repository, branch: Branch) {
     this.appStore._updatePullRequestBaseBranch(repository, branch)
   }
+
+  /**
+   * Attempts to quit the app if it's not updating, unless requested to quit
+   * even if it is updating.
+   *
+   * @param evenIfUpdating Whether to quit even if the app is updating.
+   */
+  public quitApp(evenIfUpdating: boolean) {
+    this.appStore._quitApp(evenIfUpdating)
+  }
+
+  /**
+   * Cancels quitting the app. This could be needed if, on macOS, the user tries
+   * to quit the app while an update is in progress, but then after being
+   * informed about the issues that could cause they decided to not close the
+   * app yet.
+   */
+  public cancelQuittingApp() {
+    this.appStore._cancelQuittingApp()
+  }
 }

app/src/ui/installing-update/installing-update.tsx

@@ -0,0 +1,96 @@
+import * as React from 'react'
+
+import { Row } from '../lib/row'
+import {
+  Dialog,
+  DialogContent,
+  OkCancelButtonGroup,
+  DialogFooter,
+} from '../dialog'
+import { updateStore, IUpdateState, UpdateStatus } from '../lib/update-store'
+import { Disposable } from 'event-kit'
+import { DialogHeader } from '../dialog/header'
+import { Dispatcher } from '../dispatcher'
+
+interface IInstallingUpdateProps {
+  /**
+   * Event triggered when the dialog is dismissed by the user in the
+   * ways described in the Dialog component's dismissable prop.
+   */
+  readonly onDismissed: () => void
+
+  readonly dispatcher: Dispatcher
+}
+
+/**
+ * A dialog that presents information about the
+ * running application such as name and version.
+ */
+export class InstallingUpdate extends React.Component<IInstallingUpdateProps> {
+  private updateStoreEventHandle: Disposable | null = null
+
+  private onUpdateStateChanged = (updateState: IUpdateState) => {
+    // If the update is not being downloaded (`UpdateStatus.UpdateAvailable`),
+    // i.e. if it's already downloaded or not available, close the window.
+    if (updateState.status !== UpdateStatus.UpdateAvailable) {
+      this.props.dispatcher.quitApp(false)
+    }
+  }
+
+  public componentDidMount() {
+    this.updateStoreEventHandle = updateStore.onDidChange(
+      this.onUpdateStateChanged
+    )
+
+    // Manually update the state to ensure we're in sync with the store
+    this.onUpdateStateChanged(updateStore.state)
+  }
+
+  public componentWillUnmount() {
+    if (this.updateStoreEventHandle) {
+      this.updateStoreEventHandle.dispose()
+      this.updateStoreEventHandle = null
+    }
+
+    // This will ensure the app doesn't try to quit after the update is
+    // installed once the dialog is closed (explicitly or implicitly, by
+    // opening another dialog on top of this one).
+    this.props.dispatcher.cancelQuittingApp()
+  }
+
+  private onQuitAnywayButtonClicked = () => {
+    this.props.dispatcher.quitApp(true)
+  }
+
+  public render() {
+    return (
+      <Dialog
+        id="installing-update"
+        onSubmit={this.props.onDismissed}
+        dismissable={false}
+        type="warning"
+      >
+        <DialogHeader
+          title={__DARWIN__ ? 'Installing Update…' : 'Installing update…'}
+          loading={true}
+          dismissable={true}
+          onDismissed={this.props.onDismissed}
+        />
+        <DialogContent>
+          <Row className="updating-message">
+            Do not close GitHub Desktop while the update is in progress. Closing
+            now may break your installation.
+          </Row>
+        </DialogContent>
+        <DialogFooter>
+          <OkCancelButtonGroup
+            okButtonText={__DARWIN__ ? 'Quit Anyway' : 'Quit anyway'}
+            onOkButtonClick={this.onQuitAnywayButtonClicked}
+            onCancelButtonClick={this.props.onDismissed}
+            destructive={true}
+          />
+        </DialogFooter>
+      </Dialog>
+    )
+  }
+}