feat: use default-app behavior in packaged apps (#16310)

Unify the behavior between default app and packaged apps:
- create default application menu unless the app has one
- default window-all-closed handling unless the app handles the event

default_app/main.js

@@ -7,8 +7,6 @@ const Module = require('module')
 const path = require('path')
 const url = require('url')
 
-const { setDefaultApplicationMenu } = require('./menu')
-
 // Parse command line options.
 const argv = process.argv.slice(1)
 
@@ -59,18 +57,6 @@ if (nextArgIsRequire) {
   process.exit(1)
 }
 
-// Quit when all windows are closed and no other one is listening to this.
-app.on('window-all-closed', () => {
-  if (app.listeners('window-all-closed').length === 1 && !option.interactive) {
-    app.quit()
-  }
-})
-
-// Create default menu.
-app.once('ready', () => {
-  setDefaultApplicationMenu()
-})
-
 // Set up preload modules
 if (option.modules.length > 0) {
   Module._preloadModules(option.modules)
@@ -142,6 +128,9 @@ function startRepl () {
     process.exit(1)
   }
 
+  // prevent quitting
+  app.on('window-all-closed', () => {})
+
   const repl = require('repl')
   repl.start('> ').on('exit', () => {
     process.exit(0)

docs/api/menu.md

@@ -25,10 +25,11 @@ indicate which letter should get a generated accelerator. For example, using
 opens the associated menu. The indicated character in the button label gets an
 underline. The `&` character is not displayed on the button label.
 
-Passing `null` will remove the menu bar on Windows and Linux but has no
-effect on macOS.
+Passing `null` will suppress the default menu. On Windows and Linux,
+this has the additional effect of removing the menu bar from the window.
 
-**Note:** This API has to be called after the `ready` event of `app` module.
+**Note:** The default menu will be created automatically if the app does not set one.
+It contains standard items such as `File`, `Edit`, `View`, `Window` and `Help`.
 
 #### `Menu.getApplicationMenu()`
 

filenames.gni

@@ -35,6 +35,7 @@ filenames = {
     "lib/browser/api/web-contents.js",
     "lib/browser/api/web-contents-view.js",
     "lib/browser/chrome-extension.js",
+    "lib/browser/default-menu.js",
     "lib/browser/guest-view-manager.js",
     "lib/browser/guest-window-manager.js",
     "lib/browser/init.js",
@@ -92,7 +93,6 @@ filenames = {
     "default_app/icon.png",
     "default_app/index.html",
     "default_app/main.js",
-    "default_app/menu.js",
     "default_app/package.json",
     "default_app/renderer.js",
     "default_app/styles.css",

lib/browser/api/menu.js

@@ -145,6 +145,8 @@ Menu.setApplicationMenu = function (menu) {
   }
 
   applicationMenu = menu
+  v8Util.setHiddenValue(global, 'applicationMenuSet', true)
+
   if (process.platform === 'darwin') {
     if (!menu) return
     menu._callMenuWillShow()

lib/browser/default-menu.js

@@ -1,11 +1,12 @@
 'use strict'
 
 const { shell, Menu } = require('electron')
+const v8Util = process.atomBinding('v8_util')
 
 const isMac = process.platform === 'darwin'
 
 const setDefaultApplicationMenu = () => {
-  if (Menu.getApplicationMenu()) return
+  if (v8Util.getHiddenValue(global, 'applicationMenuSet')) return
 
   const helpMenu = {
     role: 'help',

lib/browser/init.js

@@ -184,5 +184,17 @@ if (currentPlatformSupportsAppIndicator()) {
   process.env.XDG_CURRENT_DESKTOP = 'Unity'
 }
 
+// Quit when all windows are closed and no other one is listening to this.
+app.on('window-all-closed', () => {
+  if (app.listenerCount('window-all-closed') === 1) {
+    app.quit()
+  }
+})
+
+const { setDefaultApplicationMenu } = require('@electron/internal/browser/default-menu')
+
+// Create default menu.
+app.once('ready', setDefaultApplicationMenu)
+
 // Finally load app's main.js and transfer control to C++.
 Module._load(path.join(packagePath, mainStartupScript), Module, true)

spec/api-app-spec.js

@@ -1179,12 +1179,12 @@ describe('app module', () => {
 
   describe('commandLine.hasSwitch (existing argv)', () => {
     it('returns true when present', async () => {
-      const { hasSwitch } = await runCommandLineTestApp('--foobar')
+      const { hasSwitch } = await runTestApp('command-line', '--foobar')
       expect(hasSwitch).to.be.true()
     })
 
     it('returns false when not present', async () => {
-      const { hasSwitch } = await runCommandLineTestApp()
+      const { hasSwitch } = await runTestApp('command-line')
       expect(hasSwitch).to.be.false()
     })
   })
@@ -1207,31 +1207,62 @@ describe('app module', () => {
 
   describe('commandLine.getSwitchValue (existing argv)', () => {
     it('returns the value when present', async () => {
-      const { getSwitchValue } = await runCommandLineTestApp('--foobar=test')
+      const { getSwitchValue } = await runTestApp('command-line', '--foobar=test')
       expect(getSwitchValue).to.equal('test')
     })
 
     it('returns an empty string when present without value', async () => {
-      const { getSwitchValue } = await runCommandLineTestApp('--foobar')
+      const { getSwitchValue } = await runTestApp('command-line', '--foobar')
       expect(getSwitchValue).to.equal('')
     })
 
     it('returns an empty string when not present', async () => {
-      const { getSwitchValue } = await runCommandLineTestApp()
+      const { getSwitchValue } = await runTestApp('command-line')
       expect(getSwitchValue).to.equal('')
     })
   })
-
-  async function runCommandLineTestApp (...args) {
-    const appPath = path.join(__dirname, 'fixtures', 'api', 'command-line')
-    const electronPath = remote.getGlobal('process').execPath
-    const appProcess = cp.spawn(electronPath, [appPath, ...args])
-
-    let output = ''
-    appProcess.stdout.on('data', (data) => { output += data })
-
-    await emittedOnce(appProcess.stdout, 'end')
-
-    return JSON.parse(output)
-  }
 })
+
+describe('default behavior', () => {
+  describe('application menu', () => {
+    it('creates the default menu if the app does not set it', async () => {
+      const result = await runTestApp('default-menu')
+      expect(result).to.equal(false)
+    })
+
+    it('does not create the default menu if the app sets a custom menu', async () => {
+      const result = await runTestApp('default-menu', '--custom-menu')
+      expect(result).to.equal(true)
+    })
+
+    it('does not create the default menu if the app sets a null menu', async () => {
+      const result = await runTestApp('default-menu', '--null-menu')
+      expect(result).to.equal(true)
+    })
+  })
+
+  describe('window-all-closed', () => {
+    it('quits when the app does not handle the event', async () => {
+      const result = await runTestApp('window-all-closed')
+      expect(result).to.equal(false)
+    })
+
+    it('does not quit when the app handles the event', async () => {
+      const result = await runTestApp('window-all-closed', '--handle-event')
+      expect(result).to.equal(true)
+    })
+  })
+})
+
+async function runTestApp (name, ...args) {
+  const appPath = path.join(__dirname, 'fixtures', 'api', name)
+  const electronPath = remote.getGlobal('process').execPath
+  const appProcess = cp.spawn(electronPath, [appPath, ...args])
+
+  let output = ''
+  appProcess.stdout.on('data', (data) => { output += data })
+
+  await emittedOnce(appProcess.stdout, 'end')
+
+  return JSON.parse(output)
+}

spec/fixtures/api/default-menu/main.js

@@ -0,0 +1,32 @@
+const { app, Menu } = require('electron')
+
+function output (value) {
+  process.stdout.write(JSON.stringify(value))
+  process.stdout.end()
+
+  app.quit()
+}
+
+try {
+  let expectedMenu
+
+  if (app.commandLine.hasSwitch('custom-menu')) {
+    expectedMenu = new Menu()
+    Menu.setApplicationMenu(expectedMenu)
+  } else if (app.commandLine.hasSwitch('null-menu')) {
+    expectedMenu = null
+    Menu.setApplicationMenu(null)
+  }
+
+  app.on('ready', () => {
+    setImmediate(() => {
+      try {
+        output(Menu.getApplicationMenu() === expectedMenu)
+      } catch (error) {
+        output(null)
+      }
+    })
+  })
+} catch (error) {
+  output(null)
+}

spec/fixtures/api/default-menu/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "default-menu",
+  "main": "main.js"
+}

spec/fixtures/api/window-all-closed/main.js

@@ -0,0 +1,20 @@
+const { app, BrowserWindow } = require('electron')
+
+let handled = false
+
+if (app.commandLine.hasSwitch('handle-event')) {
+  app.on('window-all-closed', () => {
+    handled = true
+    app.quit()
+  })
+}
+
+app.on('quit', () => {
+  process.stdout.write(JSON.stringify(handled))
+  process.stdout.end()
+})
+
+app.on('ready', () => {
+  const win = new BrowserWindow()
+  win.close()
+})

spec/fixtures/api/window-all-closed/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "window-all-closed",
+  "main": "main.js"
+}