Fix dispatch issues for actions pre windowReady

Fix #10866

Changes in this PR:
- Browser process now registers the process right away for the
  dispatcher..
- Callbacks are registered from the window store.
- Waits until the window is ready before dispatching, queues up messages
  sent before this.
- No extensions.createTab will be issued before a window is ready.

app/browser/tabs.js

@@ -33,6 +33,7 @@ const bookmarksState = require('../common/state/bookmarksState')
 const bookmarkFoldersState = require('../common/state/bookmarkFoldersState')
 const historyState = require('../common/state/historyState')
 const bookmarkOrderCache = require('../common/cache/bookmarkOrderCache')
+const {getWindow} = require('./windows')
 
 let currentPartitionNumber = 0
 const incrementPartitionNumber = () => ++currentPartitionNumber
@@ -736,9 +737,27 @@ const api = {
         createProperties.discarded = false
         createProperties.autoDiscardable = false
       }
-      extensions.createTab(createProperties, (tab) => {
-        cb && cb(tab)
-      })
+
+      const doCreate = () => {
+        extensions.createTab(createProperties, (tab) => {
+          cb && cb(tab)
+        })
+      }
+
+      if (createProperties.windowId) {
+        const win = getWindow(createProperties.windowId)
+        if (!win || win.isDestroyed()) {
+          console.error('Cannot create a tab for a window which does not exist or is destroyed')
+          return
+        }
+        if (!win.__ready) {
+          win.once(messages.WINDOW_RENDERER_READY, () => {
+            doCreate()
+          })
+          return
+        }
+      }
+      doCreate()
     })
   },
 

app/browser/windows.js

@@ -302,6 +302,7 @@ const api = {
       if (win && !win.isDestroyed()) {
         updatePinnedTabs(win)
         win.__ready = true
+        win.emit(messages.WINDOW_RENDERER_READY)
       }
     })
   },

app/common/actions/extensionActions.js

@@ -3,7 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 'use strict'
-const AppDispatcher = require('../../../js/dispatcher/appDispatcher')
+const appDispatcher = require('../../../js/dispatcher/appDispatcher')
 const ExtensionConstants = require('../constants/extensionConstants')
 
 const extensionActions = {
@@ -14,15 +14,15 @@ const extensionActions = {
    * @param {object} browserAction - browser action details
    */
   browserActionRegistered: function (extensionId, browserAction) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.BROWSER_ACTION_REGISTERED,
       extensionId,
       browserAction
     })
   },
 
   browserActionUpdated: function (extensionId, browserAction, tabId) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.BROWSER_ACTION_UPDATED,
       extensionId,
       browserAction,
@@ -36,7 +36,7 @@ const extensionActions = {
    * @param {string} extensionId - the extension id
    */
   extensionInstalled: function (extensionId, installInfo) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.EXTENSION_INSTALLED,
       extensionId,
       installInfo
@@ -49,7 +49,7 @@ const extensionActions = {
    * @param {string} extensionId - the extension id
    */
   extensionUninstalled: function (extensionId) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.EXTENSION_UNINSTALLED,
       extensionId
     })
@@ -61,7 +61,7 @@ const extensionActions = {
    * @param {string} extensionId - the extension id
    */
   extensionEnabled: function (extensionId) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.EXTENSION_ENABLED,
       extensionId
     })
@@ -73,7 +73,7 @@ const extensionActions = {
    * @param {string} extensionId - the extension id
    */
   extensionDisabled: function (extensionId) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.EXTENSION_DISABLED,
       extensionId
     })
@@ -88,7 +88,7 @@ const extensionActions = {
    * @param {string} icon - 16x16 extension icon
    */
   contextMenuCreated: function (extensionId, menuItemId, properties, icon) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.CONTEXT_MENU_CREATED,
       extensionId,
       menuItemId,
@@ -103,7 +103,7 @@ const extensionActions = {
    * @param {string} extensionId - the extension id
    */
   contextMenuAllRemoved: function (extensionId) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.CONTEXT_MENU_ALL_REMOVED,
       extensionId
     })
@@ -117,7 +117,7 @@ const extensionActions = {
    * @param {object} info - the arg of onclick callback
    */
   contextMenuClicked: function (extensionId, tabId, info) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: ExtensionConstants.CONTEXT_MENU_CLICKED,
       extensionId,
       tabId,

js/actions/syncActions.js

@@ -3,32 +3,32 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 'use strict'
-const AppDispatcher = require('../dispatcher/appDispatcher')
+const appDispatcher = require('../dispatcher/appDispatcher')
 const syncConstants = require('../constants/syncConstants')
 
 const syncActions = {
   addSites: function (items) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: syncConstants.SYNC_ADD_SITES,
       items
     })
   },
 
   removeSites: function (items) {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: syncConstants.SYNC_REMOVE_SITES,
       items
     })
   },
 
   clearHistory: function () {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: syncConstants.SYNC_CLEAR_HISTORY
     })
   },
 
   clearSiteSettings: function () {
-    AppDispatcher.dispatch({
+    appDispatcher.dispatch({
       actionType: syncConstants.SYNC_CLEAR_SITE_SETTINGS
     })
   }

js/constants/messages.js

@@ -118,6 +118,8 @@ const messages = {
   SET_CERT_ERROR_DETAIL: _,
   SET_SECURITY_STATE: _, /** @arg {number} key of frame, @arg {Object} security state */
   HTTPSE_RULE_APPLIED: _, /** @arg {string} name of ruleset file, @arg {Object} details of rewritten request */
+  // Dispatch related message
+  WINDOW_RENDERER_READY: _,
   // Extensions
   NEW_POPUP_WINDOW: _,
   // Localization

js/dispatcher/appDispatcher.js

@@ -49,6 +49,16 @@ class AppDispatcher {
     if (process.type === 'renderer') {
       ipc.send('app-dispatcher-register')
     }
+    return this.registerLocalCallback(callback)
+  }
+
+  /**
+   * Same as above, but registers the specified callback
+   * locally only.  This is used by the windowStore since
+   * the store process is registered as soon as the window
+   * is created.
+   */
+  registerLocalCallback (callback) {
     this.callbacks.push(callback)
     return this.callbacks.length - 1 // index
   }
@@ -131,33 +141,8 @@ class AppDispatcher {
   shutdown () {
     appDispatcher.dispatch = (payload) => {}
   }
-}
-
-const appDispatcher = new AppDispatcher()
-
-const doneDispatching = () => {
-  if (dispatchCargo.idle()) {
-    appDispatcher.dispatching = false
-  }
-}
 
-const dispatchCargo = async.cargo((task, callback) => {
-  for (let i = 0; i < task.length; i++) {
-    appDispatcher.dispatchInternal(task[i], () => {})
-  }
-  callback()
-  doneDispatching()
-}, 200)
-
-const ipcCargo = async.cargo((tasks, callback) => {
-  ipc.send(messages.DISPATCH_ACTION, Serializer.serialize(tasks))
-  callback()
-}, 200)
-
-if (processType === 'browser') {
-  ipc.on('app-dispatcher-register', (event) => {
-    const registrant = event.sender
-    const hostWebContents = event.sender.hostWebContents || event.sender
+  registerWindow (registrant, hostWebContents) {
     const win = BrowserWindow.fromWebContents(hostWebContents)
     const windowId = win.id
 
@@ -168,6 +153,15 @@ if (processType === 'browser') {
       callback()
     }, 20)
 
+    // If the window isn't ready yet then wait until it is ready before delivering
+    // messages to it.
+    if (!win.__ready) {
+      registrantCargo.pause()
+      win.on(messages.WINDOW_RENDERER_READY, () => {
+        registrantCargo.resume()
+      })
+    }
+
     const callback = function (payload) {
       try {
         if (registrant.isDestroyed()) {
@@ -196,13 +190,42 @@ if (processType === 'browser') {
         appDispatcher.unregister(callback)
       }
     }
-    event.sender.on('crashed', () => {
+    registrant.on('crashed', () => {
       appDispatcher.unregister(callback)
     })
-    event.sender.on('destroyed', () => {
+    registrant.on('destroyed', () => {
       appDispatcher.unregister(callback)
     })
     appDispatcher.register(callback)
+  }
+}
+
+const appDispatcher = new AppDispatcher()
+
+const doneDispatching = () => {
+  if (dispatchCargo.idle()) {
+    appDispatcher.dispatching = false
+  }
+}
+
+const dispatchCargo = async.cargo((task, callback) => {
+  for (let i = 0; i < task.length; i++) {
+    appDispatcher.dispatchInternal(task[i], () => {})
+  }
+  callback()
+  doneDispatching()
+}, 200)
+
+const ipcCargo = async.cargo((tasks, callback) => {
+  ipc.send(messages.DISPATCH_ACTION, Serializer.serialize(tasks))
+  callback()
+}, 200)
+
+if (processType === 'browser') {
+  ipc.on('app-dispatcher-register', (event) => {
+    const registrant = event.sender
+    const hostWebContents = event.sender.hostWebContents || event.sender
+    appDispatcher.registerWindow(registrant, hostWebContents)
   })
 
   const dispatchEventPayload = (event, payload) => {

js/state/contentSettings.js

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const AppDispatcher = require('../dispatcher/appDispatcher')
+const appDispatcher = require('../dispatcher/appDispatcher')
 const AppStore = require('../stores/appStore')
 const appConstants = require('../constants/appConstants')
 const appConfig = require('../constants/appConfig')
@@ -383,21 +383,21 @@ const doAction = (action) => {
     case appConstants.APP_REMOVE_SITE_SETTING:
     case appConstants.APP_CHANGE_SITE_SETTING:
     case appConstants.APP_ADD_NOSCRIPT_EXCEPTIONS:
-      AppDispatcher.waitFor([AppStore.dispatchToken], () => {
+      appDispatcher.waitFor([AppStore.dispatchToken], () => {
         userPrefsUpdateTrigger(action.temporary)
         contentSettingsUpdateTrigger(action.temporary)
       })
       break
     case appConstants.APP_CHANGE_SETTING:
     case appConstants.APP_SET_RESOURCE_ENABLED:
-      AppDispatcher.waitFor([AppStore.dispatchToken], () => {
+      appDispatcher.waitFor([AppStore.dispatchToken], () => {
         userPrefsUpdateTrigger()
         contentSettingsUpdateTrigger()
       })
       break
     case appConstants.APP_ALLOW_FLASH_ONCE:
     case appConstants.APP_ALLOW_FLASH_ALWAYS:
-      AppDispatcher.waitFor([AppStore.dispatchToken], () => {
+      appDispatcher.waitFor([AppStore.dispatchToken], () => {
         userPrefsUpdateTrigger(action.isPrivate)
         contentSettingsUpdateTrigger(action.isPrivate)
       })
@@ -415,5 +415,5 @@ module.exports.init = () => {
     updateContentSettings(AppStore.getState(), appConfig, incognito)
   )
 
-  AppDispatcher.register(doAction)
+  appDispatcher.register(doAction)
 }

js/state/privacy.js

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const AppDispatcher = require('../dispatcher/appDispatcher')
+const appDispatcher = require('../dispatcher/appDispatcher')
 const AppStore = require('../stores/appStore')
 const appConstants = require('../constants/appConstants')
 const {passwordManagers} = require('../constants/passwordManagers')
@@ -24,13 +24,13 @@ let updateTrigger
 // Register callback to handle all updates
 const doAction = (action) => {
   if (action.actionType === appConstants.APP_CHANGE_SETTING) {
-    AppDispatcher.waitFor([AppStore.dispatchToken], () => {
+    appDispatcher.waitFor([AppStore.dispatchToken], () => {
       updateTrigger()
     })
   }
 }
 
 module.exports.init = () => {
   updateTrigger = registerUserPrefs(() => getPrivacySettings())
-  AppDispatcher.register(doAction)
+  appDispatcher.register(doAction)
 }

js/stores/appStore.js

@@ -6,7 +6,7 @@
 const appConstants = require('../constants/appConstants')
 const windowConstants = require('../constants/windowConstants')
 const ExtensionConstants = require('../../app/common/constants/extensionConstants')
-const AppDispatcher = require('../dispatcher/appDispatcher')
+const appDispatcher = require('../dispatcher/appDispatcher')
 const settings = require('../constants/settings')
 const {STATE_SITES} = require('../constants/stateConstants')
 const syncUtil = require('../state/syncUtil')
@@ -225,7 +225,7 @@ const handleAppAction = (action) => {
       calculateTopSites(true, true)
       break
     case appConstants.APP_SHUTTING_DOWN:
-      AppDispatcher.shutdown()
+      appDispatcher.shutdown()
       app.quit()
       break
     case appConstants.APP_CHANGE_NEW_TAB_DETAIL:
@@ -697,6 +697,6 @@ const handleAppAction = (action) => {
   emitChanges()
 }
 
-appStore.dispatchToken = AppDispatcher.register(handleAppAction)
+appStore.dispatchToken = appDispatcher.register(handleAppAction)
 
 module.exports = appStore