Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix dispatch issues for actions pre windowReady #10917

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const {cleanupWebContents, currentWebContents, getWebContents, updateWebContents
const {FilterOptions} = require('ad-block')
const {isResourceEnabled} = require('../filtering')
const autofill = require('../autofill')
const {getWindow} = require('./windows')

let currentPartitionNumber = 0
const incrementPartitionNumber = () => ++currentPartitionNumber
Expand Down Expand Up @@ -737,9 +738,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()
})
},

Expand Down
1 change: 1 addition & 0 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ const api = {
win.__alreadyPinnedSites = new Immutable.Set()
updatePinnedTabs(win)
win.__ready = true
win.emit(messages.WINDOW_RENDERER_READY)
}
})
},
Expand Down
20 changes: 10 additions & 10 deletions app/common/actions/extensionActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -49,7 +49,7 @@ const extensionActions = {
* @param {string} extensionId - the extension id
*/
extensionUninstalled: function (extensionId) {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: ExtensionConstants.EXTENSION_UNINSTALLED,
extensionId
})
Expand All @@ -61,7 +61,7 @@ const extensionActions = {
* @param {string} extensionId - the extension id
*/
extensionEnabled: function (extensionId) {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: ExtensionConstants.EXTENSION_ENABLED,
extensionId
})
Expand All @@ -73,7 +73,7 @@ const extensionActions = {
* @param {string} extensionId - the extension id
*/
extensionDisabled: function (extensionId) {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: ExtensionConstants.EXTENSION_DISABLED,
extensionId
})
Expand All @@ -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,
Expand All @@ -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
})
Expand All @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions js/actions/syncActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@
* 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 = {
removeSite: function (item) {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: syncConstants.SYNC_REMOVE_SITE,
item
})
},

clearHistory: function () {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: syncConstants.SYNC_CLEAR_HISTORY
})
},

clearSiteSettings: function () {
AppDispatcher.dispatch({
appDispatcher.dispatch({
actionType: syncConstants.SYNC_CLEAR_SITE_SETTINGS
})
}
Expand Down
2 changes: 2 additions & 0 deletions js/constants/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 51 additions & 28 deletions js/dispatcher/appDispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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

Expand All @@ -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()) {
Expand Down Expand Up @@ -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) => {
Expand Down
10 changes: 5 additions & 5 deletions js/state/contentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -342,21 +342,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)
})
Expand All @@ -374,5 +374,5 @@ module.exports.init = () => {
updateContentSettings(AppStore.getState(), appConfig, incognito)
)

AppDispatcher.register(doAction)
appDispatcher.register(doAction)
}
6 changes: 3 additions & 3 deletions js/state/privacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
}
Loading