From 0b1e3e952b08326c12787551e8b55a368e0d0379 Mon Sep 17 00:00:00 2001 From: Pierre Baize Date: Fri, 20 Jul 2018 08:34:03 -0600 Subject: [PATCH 1/5] RUN-4266 cannot-destructure-uuid-undefined (#480) --- src/browser/subscription_manager.ts | 3 ++- test/global_hotkey.tests.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/browser/subscription_manager.ts b/src/browser/subscription_manager.ts index 2da4aa13e..dff09247f 100644 --- a/src/browser/subscription_manager.ts +++ b/src/browser/subscription_manager.ts @@ -28,7 +28,8 @@ export default class SubscriptionManager { constructor() { this.subscriptionList = new Map(); - ofEvents.on(route.window('closed', '*'), (identity: Identity) => { + ofEvents.on(route.window('closed', '*'), (event: any) => { + const identity: Identity = event.data[0]; this.removeAllSubscriptions(identity); }); diff --git a/test/global_hotkey.tests.ts b/test/global_hotkey.tests.ts index 41074000a..dd7bf0d98 100644 --- a/test/global_hotkey.tests.ts +++ b/test/global_hotkey.tests.ts @@ -202,7 +202,7 @@ describe('GlobalHotkey', () => { GlobalHotkey.register(identity, hotkey, spy); //we simulate a window close. - ofEvents.emit(route.window('closed', '*'), identity); + ofEvents.emit(route.window('closed', identity.uuid, identity.name), identity); const isRegistered = GlobalHotkey.isRegistered(hotkey); assert.deepStrictEqual(isRegistered, false, 'Expected hotkey to not be registered'); }); @@ -242,7 +242,7 @@ describe('GlobalHotkey', () => { GlobalHotkey.register(identity2, hotkey, spy2); //we simulate a window close. - ofEvents.emit(route.window('closed', '*'), identity2); + ofEvents.emit(route.window('closed', identity2.uuid, identity2.name), identity2); mockElectron.globalShortcut.mockRaiseEvent(hotkey); assert.ok(spy.calledOnce, 'Expected the global shortcut to be called'); From 6b6e3d0463efe52f9227f9ae1e95304bccc2590f Mon Sep 17 00:00:00 2001 From: Thomas O'Connor Date: Fri, 20 Jul 2018 11:11:54 -0400 Subject: [PATCH 2/5] RUN-4266 - Type protection in elipc (#483) --- src/browser/core_state.ts | 95 +++++++++++++++++++++------------------ 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/src/browser/core_state.ts b/src/browser/core_state.ts index e6cb8e85b..95abaed3a 100644 --- a/src/browser/core_state.ts +++ b/src/browser/core_state.ts @@ -726,20 +726,23 @@ export function getInfoByUuidFrame(targetIdentity: Shapes.Identity): Shapes.Fram } for (const { openfinWindow } of app.children) { - const { name, parentFrameId } = openfinWindow; - - if (name === frame) { - const winParent = getWinById(parentFrameId); - const parent = getParentIdentity({uuid, name}); - - return { - name, - uuid, - parent, - entityType: 'window' - }; - } else if (openfinWindow.frames.get(frame)) { - return openfinWindow.frames.get(frame); + if (openfinWindow) { + const { name } = openfinWindow; + + if (name === frame) { + const parent = getParentIdentity({uuid, name}); + + return { + name, + uuid, + parent, + entityType: 'window' + }; + } else if (openfinWindow.frames.get(frame)) { + return openfinWindow.frames.get(frame); + } + } else { + writeToLog(1, `unable to find openfinWindow of child of ${app.uuid}`, true); } } } @@ -752,38 +755,42 @@ export function getRoutingInfoByUuidFrame(uuid: string, frame: string) { } for (const { openfinWindow } of app.children) { - const { uuid, name, parentFrameId } = openfinWindow; - let browserWindow: Shapes.BrowserWindow; - browserWindow = openfinWindow.browserWindow; - - if (!openfinWindow.mainFrameRoutingId) { - // save bit time here by not calling webContents.mainFrameRoutingId every time - // mainFrameRoutingId is wrong during setWindowObj - if (!browserWindow.isDestroyed()) { - openfinWindow.mainFrameRoutingId = browserWindow.webContents.mainFrameRoutingId; - writeToLog(1, `set mainFrameRoutingId ${uuid} ${name} ${openfinWindow.mainFrameRoutingId}`, true); - } else { - writeToLog(1, `unable to set mainFrameRoutingId ${uuid} ${name}`, true); + if (openfinWindow) { + const { uuid, name } = openfinWindow; + let browserWindow: Shapes.BrowserWindow; + browserWindow = openfinWindow.browserWindow; + + if (!openfinWindow.mainFrameRoutingId) { + // save bit time here by not calling webContents.mainFrameRoutingId every time + // mainFrameRoutingId is wrong during setWindowObj + if (!browserWindow.isDestroyed()) { + openfinWindow.mainFrameRoutingId = browserWindow.webContents.mainFrameRoutingId; + writeToLog(1, `set mainFrameRoutingId ${uuid} ${name} ${openfinWindow.mainFrameRoutingId}`, true); + } else { + writeToLog(1, `unable to set mainFrameRoutingId ${uuid} ${name}`, true); + } } - } - if (name === frame) { - return { - name, - browserWindow, - frameRoutingId: openfinWindow.mainFrameRoutingId, - mainFrameRoutingId: openfinWindow.mainFrameRoutingId, - frameName: name - }; - } else if (openfinWindow.frames.get(frame)) { - const {name, frameRoutingId} = openfinWindow.frames.get(frame); - return { - name, - browserWindow, - frameRoutingId, - mainFrameRoutingId: openfinWindow.mainFrameRoutingId, - frameName: name - }; + if (name === frame) { + return { + name, + browserWindow, + frameRoutingId: openfinWindow.mainFrameRoutingId, + mainFrameRoutingId: openfinWindow.mainFrameRoutingId, + frameName: name + }; + } else if (openfinWindow.frames.get(frame)) { + const {name, frameRoutingId} = openfinWindow.frames.get(frame); + return { + name, + browserWindow, + frameRoutingId, + mainFrameRoutingId: openfinWindow.mainFrameRoutingId, + frameName: name + }; + } + } else { + writeToLog(1, `unable to find openfinWindow of child of ${app.uuid}`, true); } } } From fba6bd0e77895ca274ed23b15f8ad19ccb730e14 Mon Sep 17 00:00:00 2001 From: Luis Espinola Date: Fri, 20 Jul 2018 15:54:40 -0400 Subject: [PATCH 3/5] RUN-4316 application-initialized event now returns uuid (#488) * application-initialized event now returns uuid * fixed initialized event, returns only uuid now * refactored --- src/renderer/api-decorator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/api-decorator.js b/src/renderer/api-decorator.js index fb7ead02b..98e244b31 100644 --- a/src/renderer/api-decorator.js +++ b/src/renderer/api-decorator.js @@ -240,7 +240,7 @@ limitations under the License. // main window if (uuid === name) { - eventMap.push([`application/initialized/${uuid}`, undefined]); + eventMap.push([`application/initialized/${uuid}`, { uuid }]); } eventMap.push([`window/dom-content-loaded/${uuid}-${name}`, winIdentity]); From b0fc58bdf57fbb1777f0d6d1c573cdefa346cc43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20De=20Pe=C3=B1a?= Date: Mon, 23 Jul 2018 16:22:52 -0400 Subject: [PATCH 4/5] RUN-4269 Moved window teardown from 'closed' to 'will-close' (#485) * Moved window teardown to 'will-close' event from 'closed' to reduce the time between objects leaving memory and core state begin updated. * RUN-4269 Code review items: Removed duplicate removeChildById call. Logging on success and failure. * RUN-4269 Removing Object assign. --- src/browser/api/window.js | 138 ++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 59 deletions(-) diff --git a/src/browser/api/window.js b/src/browser/api/window.js index 576fb75da..402e09d7c 100644 --- a/src/browser/api/window.js +++ b/src/browser/api/window.js @@ -125,9 +125,6 @@ let browserWindowEventMap = { }, 'unmaximize': { topic: 'restored' - }, - 'will-close': { - topic: 'closing' } // 'move': { // topic: 'bounds-changing' @@ -504,7 +501,7 @@ Window.create = function(id, opts) { _externalWindowEventAdapter = new ExternalWindowEventAdapter(browserWindow); } - let windowTeardown = createWindowTearDown(identity, id); + let windowTeardown = createWindowTearDown(identity, id, browserWindow, _boundsChangedHandler); // once the window is closed, be sure to close all the children // it may have and remove it from the @@ -536,23 +533,19 @@ Window.create = function(id, opts) { ofEvents.removeAllListeners(closeEventString); }); - browserWindow.on('closed', () => { - if (browserWindow._options.saveWindowState) { - let cachedBounds = _boundsChangedHandler.getCachedBounds(); - saveBoundsToDisk(identity, cachedBounds, err => { - if (err) { - log.writeToLog('info', err); - } - windowTeardown(); - // These were causing an exception on close if the window was reloaded - _boundsChangedHandler.teardown(); - browserWindow.removeAllListeners(); + browserWindow.once('will-close', () => { + const type = 'closing'; + windowTeardown() + .then(() => log.writeToLog('info', `Window tear down complete ${uuid} ${name}`)) + .catch(err => { + log.writeToLog('info', `Error while tearing down ${uuid} ${name}`); + log.writeToLog('info', err); }); - } else { - windowTeardown(); - _boundsChangedHandler.teardown(); - browserWindow.removeAllListeners(); - } + ofEvents.emit(route.window(type, uuid, name), { topic: 'window', type: type, uuid, name }); + }); + + webContents.once('close', () => { + webContents.removeAllListeners(); }); const isMainWindow = (uuid === name); @@ -566,10 +559,6 @@ Window.create = function(id, opts) { } }; - webContents.once('close', () => { - webContents.removeAllListeners(); - }); - webContents.on('crashed', (event, killed, terminationStatus) => { emitToAppIfMainWin('crashed', { reason: terminationStatus @@ -1854,56 +1843,87 @@ function emitReloadedEvent(identity, url) { }); } -function createWindowTearDown(identity, id) { +function createWindowTearDown(identity, id, browserWindow, _boundsChangedHandler) { + const promises = []; + + //we want to treat the close events as a step in the teardown, wrapping it in a promise. + promises.push(new Promise(resolve => { + browserWindow.once('closed', resolve); + })); + + //wrap the operation of closing a child window in a promise. + function closeChildWin(childId) { + return new Promise((resolve, reject) => { + const child = coreState.getWinObjById(childId); + + // TODO right now this is forceable to handle the event that there was a close + // requested on a child window and the main window closes. This needs + // looking into + if (child) { + let childIdentity = { + name: child.name, + uuid: child.uuid + }; + + Window.close(childIdentity, true, () => { + resolve(); + }); + } else { + resolve(); + } + }); + } + + //Even if disk operations fail we need to resolve this promise to avoid zombie processes. + function handleSaveStateAlwaysResolve() { + return new Promise((resolve, reject) => { + if (browserWindow._options.saveWindowState) { + const cachedBounds = _boundsChangedHandler.getCachedBounds(); + saveBoundsToDisk(identity, cachedBounds, err => { + if (err) { + log.writeToLog('info', err); + } + // These were causing an exception on close if the window was reloaded + _boundsChangedHandler.teardown(); + resolve(); + }); + } else { + _boundsChangedHandler.teardown(); + resolve(); + } + }); + } + + //Window tear down will: + // Update core state by removing the window. + // Save the window state to disk + // Close all child windows + // Wait for the close event. return function() { let ofWindow = Window.wrap(identity.uuid, identity.name); - let childWindows = coreState.getChildrenByWinId(id); - + let childWindows = coreState.getChildrenByWinId(id) || []; // remove from core state earlier rather than later coreState.removeChildById(id); // remove window from any groups it belongs to WindowGroups.leaveGroup(ofWindow); - if (childWindows && childWindows.length > 0) { - let closedChildren = 0; + promises.push(handleSaveStateAlwaysResolve()); - childWindows.forEach(childId => { - let child = coreState.getWinObjById(childId); - - // TODO right now this is forceable to handle the event that there was a close - // requested on a child window and the main window closes. This needs - // looking into - if (child) { - let childIdentity = { - name: child.name, - uuid: child.uuid - }; + childWindows.forEach(childId => { + promises.push(closeChildWin(childId)); + }); - Window.close(childIdentity, true, () => { - closedChildren++; - if (closedChildren === childWindows.length) { - emitCloseEvents(identity); - coreState.removeChildById(id); - } - }); - } else { - closedChildren++; - if (closedChildren === childWindows.length) { - emitCloseEvents(identity); - coreState.removeChildById(id); - } - } - }); - } else { + return Promise.all(promises).then(() => { emitCloseEvents(identity); - } + browserWindow.removeAllListeners(); + }); }; } function saveBoundsToDisk(identity, bounds, callback) { - let cacheFile = getBoundsCacheSafeFileName(identity); - let data = { + const cacheFile = getBoundsCacheSafeFileName(identity); + const data = { 'active': 'true', 'height': bounds.height, 'width': bounds.width, From 6ef8e6f0e71c6ac80ff34ffa7ab847ac7fec27dc Mon Sep 17 00:00:00 2001 From: Pierre Baize Date: Mon, 23 Jul 2018 14:33:08 -0600 Subject: [PATCH 5/5] RUN-4270 added version to getInfo; (#489) --- src/browser/api/application.js | 1 + .../api_protocol/api_handlers/mesh_middleware.ts | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/browser/api/application.js b/src/browser/api/application.js index 9fec141b8..dac88dac0 100644 --- a/src/browser/api/application.js +++ b/src/browser/api/application.js @@ -348,6 +348,7 @@ Application.getInfo = function(identity, callback) { const app = Application.wrap(identity.uuid); const response = { + runtime: { version: System.getRuntimeInfo(identity).version }, launchMode: app.launchMode }; diff --git a/src/browser/api_protocol/api_handlers/mesh_middleware.ts b/src/browser/api_protocol/api_handlers/mesh_middleware.ts index d116d69ab..5b6bbd56b 100644 --- a/src/browser/api_protocol/api_handlers/mesh_middleware.ts +++ b/src/browser/api_protocol/api_handlers/mesh_middleware.ts @@ -19,6 +19,7 @@ import * as log from '../../log'; import { default as connectionManager } from '../../connection_manager'; import ofEvents from '../../of_events'; import { isLocalUuid } from '../../core_state'; +import { IdentityAddress } from '../../runtime_p2p/peer_connection_manager'; const SUBSCRIBE_ACTION = 'subscribe'; const PUBLISH_ACTION = 'publish-message'; @@ -137,8 +138,19 @@ function ferryActionMiddleware(msg: MessagePackage, next: () => void) { if (isValidUuid && isForwardAction && isValidIdentity && isRemoteEntity && isLocalAction) { try { connectionManager.resolveIdentity({uuid}) - .then((id: any) => { + .then((id: IdentityAddress) => { id.runtime.fin.System.executeOnRemote(identity, data) + .then(res => { + switch (action) { + case 'get-info': + if (res && res.data && !res.data.runtime) { + Object.assign(res.data, {runtime: {version: id.runtime.portInfo.version}}); + } + return res; + default: + return res; + } + }) .then(ack) .catch(nack); }).catch((e: Error) => {