Some menu items which need a window will create / show one if needed

Fix #13689

This bug was caused because menu items were using a hidden window for their new tabs.

We should _not_ use `electron{.remote}.BrowserWindow.getAllWindows()` or `.get{Active, Focused}Window` directly. There are two reasons that could have bad results:
1. We create BrowserWindows which are not normal tabbed renderer windows (e.g. some code that runs on Bookmark creation)
2. We have a hidden window sometimes (the Buffer Window)
Instead call `app/browser/window.js`: `getAllRendererWindows` and `getActiveWindowId`.
Fix for main menu actions, which can be called when there are no windows, but when searching for `BrowserWindow.`

Test Plan:
- Check all menu items open new tab in currently active window
- Check all menu items open new tab in currently active window, when app is not focused
- Check all menu items open new tab in new window if there is no visible window
- Check tabs in all windows are persisted on restart when app / windows close
- Check process quits in Windows when last visible window is closed

app/browser/reducers/tabsReducer.js

@@ -12,6 +12,7 @@ const windowActions = require('../../../js/actions/windowActions')
 
 // State
 const tabState = require('../../common/state/tabState')
+const windowState = require('../../common/state/windowState')
 const siteSettings = require('../../../js/state/siteSettings')
 const siteSettingsState = require('../../common/state/siteSettingsState')
 const {frameOptsFromFrame} = require('../../../js/state/frameStateUtil')
@@ -23,6 +24,7 @@ const appConstants = require('../../../js/constants/appConstants')
 const windowConstants = require('../../../js/constants/windowConstants')
 const dragTypes = require('../../../js/constants/dragTypes')
 const tabActionConsts = require('../../common/constants/tabAction')
+const appActions = require('../../../js/actions/appActions')
 
 // Utils
 const tabs = require('../tabs')
@@ -141,8 +143,16 @@ const tabsReducer = (state, action, immutableAction) => {
         const senderWindowId = action.getIn(['senderWindowId'])
         if (senderWindowId != null) {
           action = action.setIn(['createProperties', 'windowId'], senderWindowId)
-        } else if (BrowserWindow.getActiveWindow()) {
-          action = action.setIn(['createProperties', 'windowId'], BrowserWindow.getActiveWindow().id)
+        } else {
+          // no specified window, so use active one, or create one
+          const activeWindowId = windows.getActiveWindowId()
+          if (activeWindowId === windowState.WINDOW_ID_NONE) {
+            setImmediate(() => appActions.newWindow(action.get('createProperties')))
+            // this action will get dispatched again
+            // once the new window is ready to have tabs
+            break
+          }
+          action = action.setIn(['createProperties', 'windowId'], activeWindowId)
         }
       }
 

app/browser/windows.js

@@ -348,7 +348,7 @@ const api = {
         // that will not get saved to state as the last-closed window which should be restored
         // since we won't save state if there are no frames.
         if (!platformUtil.isDarwin() && api.getBufferWindow()) {
-          const remainingWindows = api.getAllRendererWindows().filter(win => win !== api.getBufferWindow())
+          const remainingWindows = api.getAllRendererWindows()
           if (!remainingWindows.length) {
             api.closeBufferWindow()
           }
@@ -784,21 +784,31 @@ const api = {
     return currentWindows[windowId]
   },
 
-  getActiveWindowId: () => {
-    if (BrowserWindow.getFocusedWindow()) {
-      return BrowserWindow.getFocusedWindow().id
+  getActiveWindow: () => {
+    const focusedWindow = BrowserWindow.getFocusedWindow()
+    if (api.getAllRendererWindows().includes(focusedWindow)) {
+      return focusedWindow
     }
-    return windowState.WINDOW_ID_NONE
+    return null
+  },
+
+  getActiveWindowId: () => {
+    const activeWindow = api.getActiveWindow()
+    return activeWindow ? activeWindow.id : windowState.WINDOW_ID_NONE
   },
 
   /**
    * Provides an array of all Browser Windows which are actual
    * main windows (not background workers), and are not destroyed
    */
-  getAllRendererWindows: () => {
+  getAllRendererWindows: (includingBufferWindow = false) => {
     return Object.keys(currentWindows)
       .map(key => currentWindows[key])
-      .filter(win => win && !win.isDestroyed())
+      .filter(win =>
+        win &&
+        !win.isDestroyed() &&
+        (includingBufferWindow || win !== api.getBufferWindow())
+      )
   },
 
   on: (...args) => publicEvents.on(...args),

app/common/commonMenu.js

@@ -14,25 +14,12 @@ const communityURL = 'https://community.brave.com/'
 const isDarwin = process.platform === 'darwin'
 const electron = require('electron')
 
-let BrowserWindow
-if (process.type === 'browser') {
-  BrowserWindow = electron.BrowserWindow
-} else {
-  BrowserWindow = electron.remote.BrowserWindow
-}
-
 const ensureAtLeastOneWindow = (frameOpts) => {
-  if (process.type === 'browser') {
-    if (BrowserWindow.getAllWindows().length === 0) {
-      appActions.newWindow(frameOpts || {})
-      return
-    }
-  }
-
-  if (!frameOpts) {
-    return
-  }
-
+  // If this action is dispatched from a renderer window (windows)
+  // it will create the tab in the current window
+  // If it was dispatched by the browser (mac / linux)
+  // then it will create the tab in the active window
+  // or a new window if there is no active window
   appActions.createTabRequested(frameOpts)
 }