From c7afb49ebd1507c17eeec9eadc0d274ee64d8da6 Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Wed, 2 Oct 2019 10:47:44 -0400 Subject: [PATCH 1/8] Hook up event propagation for all browserview-related events --- src/browser/api/browser_view.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/browser/api/browser_view.ts b/src/browser/api/browser_view.ts index 76c35ed4a..b34f49c16 100644 --- a/src/browser/api/browser_view.ts +++ b/src/browser/api/browser_view.ts @@ -95,6 +95,8 @@ export async function attach(ofView: OfView, toIdentity: Identity) { if (oldWin) { oldWin.browserWindow.removeBrowserView(view); of_events.emit(route.window('view-detached', previousTarget.uuid, previousTarget.name), { + uuid: ofView.uuid, + name: ofView.name, viewIdentity: {uuid: ofView.uuid, name: ofView.name}, target: toIdentity, previousTarget @@ -126,10 +128,14 @@ export async function attach(ofView: OfView, toIdentity: Identity) { updateViewTarget(ofView, toIdentity); of_events.emit(route.view('attached', ofView.uuid, ofView.name), { + name: ofView.name, + uuid: ofView.uuid, target: toIdentity, previousTarget }); of_events.emit(route.window('view-attached', toIdentity.uuid, toIdentity.name), { + name: ofView.name, + uuid: ofView.uuid, viewIdentity: {uuid: ofView.uuid, name: ofView.name}, target: toIdentity, previousTarget @@ -140,7 +146,7 @@ export async function destroy (ofView: OfView) { const {uuid, name, target, view} = ofView; removeBrowserView(ofView); view.destroy(); - of_events.emit(route.view('destroyed', uuid, name), {target}); + of_events.emit(route.view('destroyed', uuid, name), {name, uuid, target}); } export async function setAutoResize(ofView: OfView, autoResize: AutoResizeOptions) { From c55acc48b1b630a2011a5307704f723b77e6e6ad Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Wed, 9 Oct 2019 16:01:20 -0400 Subject: [PATCH 2/8] Propagate browserview events up and through window --- src/browser/api/browser_view.ts | 8 +++++++- src/browser/of_events.ts | 29 +++++++++++++++++++++++++++++ test/errors.test.ts | 2 ++ test/event-propagation.ts | 3 ++- test/file_download.ts | 2 +- 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/browser/api/browser_view.ts b/src/browser/api/browser_view.ts index b34f49c16..558d30b1c 100644 --- a/src/browser/api/browser_view.ts +++ b/src/browser/api/browser_view.ts @@ -94,9 +94,15 @@ export async function attach(ofView: OfView, toIdentity: Identity) { const oldWin = getWindowByUuidName(previousTarget.uuid, previousTarget.name); if (oldWin) { oldWin.browserWindow.removeBrowserView(view); - of_events.emit(route.window('view-detached', previousTarget.uuid, previousTarget.name), { + of_events.emit(route.view('detached', ofView.uuid, ofView.name), { + name: ofView.name, uuid: ofView.uuid, + target: toIdentity, + previousTarget + }); + of_events.emit(route.window('view-detached', previousTarget.uuid, previousTarget.name), { name: ofView.name, + uuid: ofView.uuid, viewIdentity: {uuid: ofView.uuid, name: ofView.name}, target: toIdentity, previousTarget diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index 892d9a420..6590fd878 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -2,6 +2,7 @@ import { app } from 'electron'; import { EventEmitter } from 'events'; import { isFloat } from '../common/main'; import route from '../common/route'; +import {getBrowserViewByIdentity} from './core_state'; interface PastEvent { payload: any; @@ -33,6 +34,7 @@ class OFEvents extends EventEmitter { if (tokenizedRoute.length >= 2) { const [channel, topic] = tokenizedRoute; const uuid: string = (payload && payload.uuid) || tokenizedRoute[2] || '*'; + const name: string|false = (payload && payload.name); const source = tokenizedRoute.slice(2).join('/'); const envelope = { channel, topic, source, data }; const propagateToSystem = !topic.match(/-requested$/); @@ -67,11 +69,14 @@ class OFEvents extends EventEmitter { } } else if (channel === 'view') { const propTopic = `view-${topic}`; + this.propagateEventsToWindow(topic, propTopic, checkedPayload, uuid, name, eventPropagations); + eventPropagations.set(route.application(propTopic, uuid), { ...checkedPayload, type: propTopic, topic: 'application' }); + if (propagateToSystem) { eventPropagations.set(route.system(propTopic), { ...checkedPayload, type: propTopic, topic: 'system' }); } @@ -146,6 +151,30 @@ class OFEvents extends EventEmitter { this.isSavingEvents = false; }, STARTUP_SAVE_EVENTS_DURATION); } + + private propagateEventsToWindow( + topic: string, propTopic: string, checkedPayload: any, uuid: string, name: string|false, eventPropagations: Map) { + + const windowDontPropagate = [ + 'detached', + 'attached' + ]; + + if (!windowDontPropagate.some(t => t === topic)) { + let target = checkedPayload.target; + if (!target && name) { + const view = getBrowserViewByIdentity({ uuid, name }); + target = view && view.target; + } + if (target) { + eventPropagations.set(route.window(propTopic, target.uuid, target.name), { + ...checkedPayload, + type: propTopic, + topic: 'window' + }); + } + } + } } interface StringMap { diff --git a/test/errors.test.ts b/test/errors.test.ts index 926819305..27bb8d5d7 100644 --- a/test/errors.test.ts +++ b/test/errors.test.ts @@ -7,6 +7,8 @@ mockery.enable({ warnOnReplace: false, warnOnUnregistered: false }); + +mockery.registerMock('./api/external_application', {}); import * as errors from '../src/common/errors'; describe('Errors', () => { diff --git a/test/event-propagation.ts b/test/event-propagation.ts index 7f6e32fce..e21115d49 100644 --- a/test/event-propagation.ts +++ b/test/event-propagation.ts @@ -17,17 +17,18 @@ limitations under the License. import { mockElectron } from './electron'; import * as assert from 'assert'; import * as mockery from 'mockery'; -import ofEvents from '../src/browser/of_events'; import route from '../src/common/route'; const uuid = 'uuid4'; const name = 'name3'; mockery.registerMock('electron', mockElectron); +mockery.registerMock('./api/external_application', {}); mockery.enable({ warnOnReplace: false, warnOnUnregistered: false }); +import ofEvents from '../src/browser/of_events'; describe('Event Propagation', function () { it('propagates window events to system', function (done) { diff --git a/test/file_download.ts b/test/file_download.ts index e1670ebd6..97c0c00f2 100644 --- a/test/file_download.ts +++ b/test/file_download.ts @@ -1,7 +1,6 @@ import * as mockery from 'mockery'; import { mockElectron } from './electron'; import * as assert from 'assert'; -import ofEvents from '../src/browser/of_events'; import route from '../src/common/route'; import { EventEmitter } from 'events'; @@ -56,6 +55,7 @@ mockery.enable({ }); import { createWillDownloadEventListener, downloadLocationMap } from '../src/browser/api/file_download'; +import ofEvents from '../src/browser/of_events'; describe('FileDownload', () => { From 99ab0e7738e996610600908b47336eacdf79ba4a Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Thu, 10 Oct 2019 13:26:00 -0400 Subject: [PATCH 3/8] Add test comment --- test/errors.test.ts | 2 +- test/event-propagation.ts | 2 ++ test/file_download.ts | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/errors.test.ts b/test/errors.test.ts index 27bb8d5d7..8af7ae2ed 100644 --- a/test/errors.test.ts +++ b/test/errors.test.ts @@ -7,7 +7,7 @@ mockery.enable({ warnOnReplace: false, warnOnUnregistered: false }); - +// Do not move this external_application mock - Core PR #976 mockery.registerMock('./api/external_application', {}); import * as errors from '../src/common/errors'; diff --git a/test/event-propagation.ts b/test/event-propagation.ts index e21115d49..978ca0c17 100644 --- a/test/event-propagation.ts +++ b/test/event-propagation.ts @@ -23,11 +23,13 @@ const uuid = 'uuid4'; const name = 'name3'; mockery.registerMock('electron', mockElectron); +// Do not move this external_application mock - Core PR #976 mockery.registerMock('./api/external_application', {}); mockery.enable({ warnOnReplace: false, warnOnUnregistered: false }); +// Do not move this ofEvents import - Core PR #976 import ofEvents from '../src/browser/of_events'; describe('Event Propagation', function () { diff --git a/test/file_download.ts b/test/file_download.ts index 97c0c00f2..30a13fceb 100644 --- a/test/file_download.ts +++ b/test/file_download.ts @@ -55,6 +55,7 @@ mockery.enable({ }); import { createWillDownloadEventListener, downloadLocationMap } from '../src/browser/api/file_download'; +// Do not move this ofEvents import - Core PR #976 import ofEvents from '../src/browser/of_events'; describe('FileDownload', () => { From 27aee214f5320be4ce5d0518abb1b30e4331519c Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Wed, 16 Oct 2019 10:48:05 -0400 Subject: [PATCH 4/8] Change typing of name from false to undefined --- src/browser/of_events.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index 6590fd878..fc285a7d4 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -1,3 +1,4 @@ + import { app } from 'electron'; import { EventEmitter } from 'events'; import { isFloat } from '../common/main'; @@ -34,7 +35,7 @@ class OFEvents extends EventEmitter { if (tokenizedRoute.length >= 2) { const [channel, topic] = tokenizedRoute; const uuid: string = (payload && payload.uuid) || tokenizedRoute[2] || '*'; - const name: string|false = (payload && payload.name); + const name: string|undefined = (payload && payload.name); const source = tokenizedRoute.slice(2).join('/'); const envelope = { channel, topic, source, data }; const propagateToSystem = !topic.match(/-requested$/); From fd30fbd927aa1568f0278e5ac4a3e7acc86c31b8 Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Wed, 16 Oct 2019 15:04:58 -0400 Subject: [PATCH 5/8] Remove view-attached custom event emit --- src/browser/api/browser_view.ts | 7 ------- src/browser/of_events.ts | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/browser/api/browser_view.ts b/src/browser/api/browser_view.ts index b5865ccd1..5b400acc7 100644 --- a/src/browser/api/browser_view.ts +++ b/src/browser/api/browser_view.ts @@ -149,13 +149,6 @@ export async function attach(ofView: OfView, toIdentity: Identity) { target: toIdentity, previousTarget }); - of_events.emit(route.window('view-attached', toIdentity.uuid, toIdentity.name), { - name: ofView.name, - uuid: ofView.uuid, - viewIdentity: {uuid: ofView.uuid, name: ofView.name}, - target: toIdentity, - previousTarget - }); } } export async function destroy (ofView: OfView) { diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index fc285a7d4..0b44f6055 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -157,8 +157,7 @@ class OFEvents extends EventEmitter { topic: string, propTopic: string, checkedPayload: any, uuid: string, name: string|false, eventPropagations: Map) { const windowDontPropagate = [ - 'detached', - 'attached' + 'detached' ]; if (!windowDontPropagate.some(t => t === topic)) { From bad0f8507889992ee1b1669e544b36e67832d906 Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Thu, 17 Oct 2019 11:22:22 -0400 Subject: [PATCH 6/8] Change the way we fetch target for view event propagation --- src/browser/api/browser_view.ts | 9 +-------- src/browser/of_events.ts | 33 +++++++++++++++------------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/browser/api/browser_view.ts b/src/browser/api/browser_view.ts index 5b400acc7..728f3891f 100644 --- a/src/browser/api/browser_view.ts +++ b/src/browser/api/browser_view.ts @@ -102,20 +102,13 @@ export async function attach(ofView: OfView, toIdentity: Identity) { const oldwinMap = windowCloseListenerMap.get(oldWin); if (previousTarget.name !== toIdentity.name) { - oldWin.browserWindow.removeBrowserView(view); of_events.emit(route.view('detached', ofView.uuid, ofView.name), { name: ofView.name, uuid: ofView.uuid, target: toIdentity, previousTarget }); - of_events.emit(route.window('view-detached', previousTarget.uuid, previousTarget.name), { - name: ofView.name, - uuid: ofView.uuid, - viewIdentity: {uuid: ofView.uuid, name: ofView.name}, - target: toIdentity, - previousTarget - }); + oldWin.browserWindow.removeBrowserView(view); const oldwinMap = windowCloseListenerMap.get(oldWin); if (oldwinMap) { const listener = oldwinMap.get(ofView); diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index 0b44f6055..c84c6c19d 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -70,7 +70,7 @@ class OFEvents extends EventEmitter { } } else if (channel === 'view') { const propTopic = `view-${topic}`; - this.propagateEventsToWindow(topic, propTopic, checkedPayload, uuid, name, eventPropagations); + this.propagateEventsToWindow(propTopic, checkedPayload, uuid, name, eventPropagations); eventPropagations.set(route.application(propTopic, uuid), { ...checkedPayload, @@ -154,25 +154,22 @@ class OFEvents extends EventEmitter { } private propagateEventsToWindow( - topic: string, propTopic: string, checkedPayload: any, uuid: string, name: string|false, eventPropagations: Map) { + propTopic: string, checkedPayload: any, uuid: string, name: string|undefined, eventPropagations: Map) { - const windowDontPropagate = [ - 'detached' - ]; + let target; + if (name) { + const view = getBrowserViewByIdentity({ uuid, name }); + target = view && view.target; + } - if (!windowDontPropagate.some(t => t === topic)) { - let target = checkedPayload.target; - if (!target && name) { - const view = getBrowserViewByIdentity({ uuid, name }); - target = view && view.target; - } - if (target) { - eventPropagations.set(route.window(propTopic, target.uuid, target.name), { - ...checkedPayload, - type: propTopic, - topic: 'window' - }); - } + // Set the target in the checkedPayload, so that webcontents events have a target param in the payload. + target ? checkedPayload.target = target : target = checkedPayload.target; + if (target) { + eventPropagations.set(route.window(propTopic, target.uuid, target.name), { + ...checkedPayload, + type: propTopic, + topic: 'window' + }); } } } From a02955faf95a23d3778fb72e79d0693ae59264f5 Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Thu, 17 Oct 2019 14:28:35 -0400 Subject: [PATCH 7/8] Change eventing to target-changed and view-attached/detached for windows --- src/browser/api/browser_view.ts | 12 +++++++++--- src/browser/of_events.ts | 34 +++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/browser/api/browser_view.ts b/src/browser/api/browser_view.ts index 728f3891f..632d05d8e 100644 --- a/src/browser/api/browser_view.ts +++ b/src/browser/api/browser_view.ts @@ -102,13 +102,13 @@ export async function attach(ofView: OfView, toIdentity: Identity) { const oldwinMap = windowCloseListenerMap.get(oldWin); if (previousTarget.name !== toIdentity.name) { - of_events.emit(route.view('detached', ofView.uuid, ofView.name), { + oldWin.browserWindow.removeBrowserView(view); + of_events.emit(route.window('view-detached', previousTarget.uuid, previousTarget.name), { name: ofView.name, uuid: ofView.uuid, target: toIdentity, previousTarget }); - oldWin.browserWindow.removeBrowserView(view); const oldwinMap = windowCloseListenerMap.get(oldWin); if (oldwinMap) { const listener = oldwinMap.get(ofView); @@ -136,7 +136,13 @@ export async function attach(ofView: OfView, toIdentity: Identity) { windowCloseListenerMap.get(ofWin).set(ofView, listener); updateViewTarget(ofView, toIdentity); - of_events.emit(route.view('attached', ofView.uuid, ofView.name), { + of_events.emit(route.window('view-attached', toIdentity.uuid, toIdentity.name), { + name: ofView.name, + uuid: ofView.uuid, + target: toIdentity, + previousTarget + }); + of_events.emit(route.view('target-changed', ofView.uuid, ofView.name), { name: ofView.name, uuid: ofView.uuid, target: toIdentity, diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index c84c6c19d..a68b2b43e 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -58,7 +58,7 @@ class OFEvents extends EventEmitter { const dontPropagate = [ 'close-requested' ]; - if (!dontPropagate.some(t => t === topic)) { + if (!dontPropagate.some(t => t === topic) && !topic.startsWith('view')) { eventPropagations.set(route.application(propTopic, uuid), { ...checkedPayload, type: propTopic, @@ -156,20 +156,26 @@ class OFEvents extends EventEmitter { private propagateEventsToWindow( propTopic: string, checkedPayload: any, uuid: string, name: string|undefined, eventPropagations: Map) { - let target; - if (name) { - const view = getBrowserViewByIdentity({ uuid, name }); - target = view && view.target; - } + const dontPropagate = [ + 'view-target-changed' + ]; + + if (!dontPropagate.some(t => t === propTopic)) { + let target; + if (name) { + const view = getBrowserViewByIdentity({ uuid, name }); + target = view && view.target; + } - // Set the target in the checkedPayload, so that webcontents events have a target param in the payload. - target ? checkedPayload.target = target : target = checkedPayload.target; - if (target) { - eventPropagations.set(route.window(propTopic, target.uuid, target.name), { - ...checkedPayload, - type: propTopic, - topic: 'window' - }); + // Set the target in the checkedPayload, so that webcontents events have a target param in the payload. + target ? checkedPayload.target = target : target = checkedPayload.target; + if (target) { + eventPropagations.set(route.window(propTopic, target.uuid, target.name), { + ...checkedPayload, + type: propTopic, + topic: 'window' + }); + } } } } From f877a7ff2538c83ff4af59454d87f3fb3bb79cf7 Mon Sep 17 00:00:00 2001 From: Michael Coates Date: Thu, 17 Oct 2019 14:44:53 -0400 Subject: [PATCH 8/8] Unexclude view-target-changed propagation, but don't document it --- src/browser/of_events.ts | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/browser/of_events.ts b/src/browser/of_events.ts index a68b2b43e..092201323 100644 --- a/src/browser/of_events.ts +++ b/src/browser/of_events.ts @@ -156,26 +156,20 @@ class OFEvents extends EventEmitter { private propagateEventsToWindow( propTopic: string, checkedPayload: any, uuid: string, name: string|undefined, eventPropagations: Map) { - const dontPropagate = [ - 'view-target-changed' - ]; - - if (!dontPropagate.some(t => t === propTopic)) { - let target; - if (name) { - const view = getBrowserViewByIdentity({ uuid, name }); - target = view && view.target; - } + let target; + if (name) { + const view = getBrowserViewByIdentity({ uuid, name }); + target = view && view.target; + } - // Set the target in the checkedPayload, so that webcontents events have a target param in the payload. - target ? checkedPayload.target = target : target = checkedPayload.target; - if (target) { - eventPropagations.set(route.window(propTopic, target.uuid, target.name), { - ...checkedPayload, - type: propTopic, - topic: 'window' - }); - } + // Set the target in the checkedPayload, so that webcontents events have a target param in the payload. + target ? checkedPayload.target = target : target = checkedPayload.target; + if (target) { + eventPropagations.set(route.window(propTopic, target.uuid, target.name), { + ...checkedPayload, + type: propTopic, + topic: 'window' + }); } } }