diff --git a/src/annotator/cross-frame.js b/src/annotator/cross-frame.js index bbe15620269..f6e2f463b25 100644 --- a/src/annotator/cross-frame.js +++ b/src/annotator/cross-frame.js @@ -64,9 +64,9 @@ export class CrossFrame { }; // Initiate connection to the sidebar. - const onDiscoveryCallback = (source, origin, token) => - bridge.createChannel(source, origin, token); - discovery.startDiscovery(onDiscoveryCallback); + discovery.startDiscovery((source, origin, token) => + bridge.createChannel({ source, origin, token }) + ); frameObserver.observe(injectIntoFrame, iframeUnloaded); /** diff --git a/src/annotator/test/cross-frame-test.js b/src/annotator/test/cross-frame-test.js index f3e6e49dea7..07221aeada9 100644 --- a/src/annotator/test/cross-frame-test.js +++ b/src/annotator/test/cross-frame-test.js @@ -88,7 +88,11 @@ describe('CrossFrame', () => { createCrossFrame(); fakeDiscovery.startDiscovery.yield('SOURCE', 'ORIGIN', 'TOKEN'); assert.called(fakeBridge.createChannel); - assert.calledWith(fakeBridge.createChannel, 'SOURCE', 'ORIGIN', 'TOKEN'); + assert.calledWith(fakeBridge.createChannel, { + source: 'SOURCE', + origin: 'ORIGIN', + token: 'TOKEN', + }); }); }); diff --git a/src/shared/bridge.js b/src/shared/bridge.js index a9739933c8a..283b60136a2 100644 --- a/src/shared/bridge.js +++ b/src/shared/bridge.js @@ -14,7 +14,7 @@ export default class Bridge { this.links = []; /** @type {Record void>} */ this.channelListeners = {}; - /** @type {Array<(channel: RPC, window: Window) => void>} */ + /** @type {Array<(channel: RPC) => void>} */ this.onConnectListeners = []; } @@ -27,18 +27,84 @@ export default class Bridge { this.links.forEach(channel => channel.destroy()); } + /** + * Deprecated - Remove after MessagePort conversion + * + * @typedef WindowOptions + * @prop {Window} source - The source window + * @prop {string} origin - The origin of the document in `source` + * @prop {string} token - Shared token between the `source` and this window + * agreed during the discovery process + */ + + /** + * Create a communication channel between frames using either `MessagePort` or + * `Window`. + * + * The created channel is added to the list of channels which `call` + * and `on` send and receive messages over. + * + * @param {WindowOptions|MessagePort} options + * @return {RPC} - Channel for communicating with the window. + */ + createChannel(options) { + if (options instanceof MessagePort) { + return this._createChannelForPort(options); + } else { + // Deprecated - Remove after MessagePort conversion + return this._createChannelForWindow(options); + } + } + + /** + * Create a communication channel using a `MessagePort`. + * + * The created channel is added to the list of channels which `call` + * and `on` send and receive messages over. + * + * @param {MessagePort} port + * @return {RPC} - Channel for communicating with the window. + */ + _createChannelForPort(port) { + const listeners = { connect: cb => cb(), ...this.channelListeners }; + + // Set up a channel + const channel = new RPC( + window /* dummy */, + port, + '*' /* dummy */, + listeners + ); + + let connected = false; + const ready = () => { + if (connected) { + return; + } + connected = true; + this.onConnectListeners.forEach(cb => cb(channel)); + }; + + // Fire off a connection attempt + channel.call('connect', ready); + + // Store the newly created channel in our collection + this.links.push(channel); + + return channel; + } + /** * Create a communication channel between this window and `source`. * * The created channel is added to the list of channels which `call` * and `on` send and receive messages over. * - * @param {Window} source - The source window. - * @param {string} origin - The origin of the document in `source`. - * @param {string} token + * @param {WindowOptions} options * @return {RPC} - Channel for communicating with the window. + * @deprecated */ - createChannel(source, origin, token) { + _createChannelForWindow({ source, origin, token }) { let channel = null; let connected = false; @@ -47,13 +113,12 @@ export default class Bridge { return; } connected = true; - this.onConnectListeners.forEach(cb => cb(channel, source)); + this.onConnectListeners.forEach(cb => cb(channel)); }; const connect = (_token, cb) => { if (_token === token) { cb(); - ready(); } }; @@ -162,7 +227,7 @@ export default class Bridge { /** * Add a listener to be called upon a new connection. * - * @param {(channel: RPC, window: Window) => void} listener + * @param {(channel: RPC) => void} listener */ onConnect(listener) { this.onConnectListeners.push(listener); diff --git a/src/shared/test/bridge-test.js b/src/shared/test/bridge-test.js index f0734e9b50c..eec374c2a51 100644 --- a/src/shared/test/bridge-test.js +++ b/src/shared/test/bridge-test.js @@ -21,8 +21,16 @@ describe('shared/bridge', () => { beforeEach(() => { bridge = new Bridge(); - createChannel = () => - bridge.createChannel(fakeWindow, 'http://example.com', 'TOKEN'); + createChannel = source => { + if (!source) { + source = { + source: fakeWindow, + origin: 'http://example.com', + token: 'TOKEN', + }; + } + return bridge.createChannel(source); + }; fakeWindow = { postMessage: sandbox.stub(), @@ -39,18 +47,39 @@ describe('shared/bridge', () => { }); describe('#createChannel', () => { - it('creates a new channel with the provided options', () => { - const channel = createChannel(); - assert.equal(channel.sourceFrame, window); - assert.equal(channel.destFrame, fakeWindow); - assert.equal(channel.origin, 'http://example.com'); + context('with a Window source', () => { + it('creates a new channel', () => { + const channel = createChannel(); + assert.equal(channel.sourceFrame, window); + assert.equal(channel.destFrame, fakeWindow); + assert.equal(channel.origin, 'http://example.com'); + }); + + it('adds the channel to the `links` property', () => { + const channel = createChannel(); + assert.isTrue( + bridge.links.some(registeredChannel => registeredChannel === channel) + ); + }); }); - it('adds the channel to the .links property', () => { - const channel = createChannel(); - assert.isTrue( - bridge.links.some(registeredChannel => registeredChannel === channel) - ); + context('with a MessagePort source', () => { + it('creates a new channel', () => { + const messageChannel = new MessageChannel(); + + const channel = createChannel(messageChannel.port1); + + assert.equal(channel.sourceFrame, window); + assert.equal(channel.destFrame, messageChannel.port1); + assert.equal(channel.origin, '*'); + }); + + it('adds the channel to the `links` property', () => { + const channel = createChannel(); + assert.isTrue( + bridge.links.some(registeredChannel => registeredChannel === channel) + ); + }); }); it('registers any existing listeners on the channel', () => { @@ -85,11 +114,11 @@ describe('shared/bridge', () => { it('calls a callback when all channels return successfully', done => { const channel1 = createChannel(); - const channel2 = bridge.createChannel( - fakeWindow, - 'http://example.com', - 'NEKOT' - ); + const channel2 = bridge.createChannel({ + source: fakeWindow, + origin: 'http://example.com', + token: 'NEKOT', + }); channel1.call.yields(null, 'result1'); channel2.call.yields(null, 'result2'); @@ -105,11 +134,11 @@ describe('shared/bridge', () => { it('calls a callback with an error when a channels fails', done => { const error = new Error('Uh oh'); const channel1 = createChannel(); - const channel2 = bridge.createChannel( - fakeWindow, - 'http://example.com', - 'NEKOT' - ); + const channel2 = bridge.createChannel({ + source: fakeWindow, + origin: 'http://example.com', + token: 'NEKOT', + }); channel1.call.throws(error); channel2.call.yields(null, 'result2'); @@ -205,30 +234,60 @@ describe('shared/bridge', () => { }); describe('#onConnect', () => { - it('adds a callback that is called when a channel is connected', () => { + // Simulate a Bridge attached to the other end of a channel receiving + // the `connect` RPC request and handling it using the `connect` handler + // registered by the Bridge. + const runConnectHandler = channel => { + const connectCall = channel.call + .getCalls() + .find(call => call.firstArg === 'connect'); + + // Invoke the `connect` handler. Here we're invoking it on `channel` but + // in the actual app this would be called on the counterpart channel in + // the other frame. This distinction doesn't matter because all the + // handler does is check a token (which is the same on both sides) and + // call the result callback. + channel.methods.connect(...connectCall.args.slice(1)); + }; + + it('runs callbacks when a Window channel connects with correct token', () => { const onConnectCallback = sinon.stub(); bridge.onConnect(onConnectCallback); const channel = createChannel(); - // Simulate "connect" RPC call by Bridge instance in channel's destination frame. - channel.methods.connect('TOKEN', sinon.stub()); + runConnectHandler(channel); - assert.calledWith(onConnectCallback, channel, fakeWindow); + assert.calledWith(onConnectCallback, channel); }); - it('does not run `onConnect` callbacks if the token is wrong', () => { + it('does not run callback if a Window channel connects with wrong token', () => { const onConnectCallback = sinon.stub(); bridge.onConnect(onConnectCallback); const channel = createChannel(); // Simulate "connect" RPC call by Bridge instance in channel's destination frame. - channel.methods.connect('WRONG-TOKEN', sinon.stub()); + const connectCall = channel.call + .getCalls() + .find(call => call.firstArg === 'connect'); + channel.methods.connect('WRONG-TOKEN', connectCall.lastArg); assert.notCalled(onConnectCallback); }); + it('runs callbacks when a MessagePort channel connects', () => { + const onConnectCallback = sinon.stub(); + bridge.onConnect(onConnectCallback); + + const messageChannel = new MessageChannel(); + const channel = createChannel(messageChannel.port1); + + runConnectHandler(channel); + + assert.calledWith(onConnectCallback, channel); + }); + it('allows multiple callbacks to be registered', () => { const onConnectCallback1 = sinon.stub(); const onConnectCallback2 = sinon.stub(); @@ -237,11 +296,10 @@ describe('shared/bridge', () => { const channel = createChannel(); - // Simulate "connect" RPC call by Bridge instance in channel's destination frame. - channel.methods.connect('TOKEN', sinon.stub()); + runConnectHandler(channel); - assert.calledWith(onConnectCallback1, channel, fakeWindow); - assert.calledWith(onConnectCallback2, channel, fakeWindow); + assert.calledWith(onConnectCallback1, channel); + assert.calledWith(onConnectCallback2, channel); }); it('only invokes `onConnect` callback once', () => { @@ -250,9 +308,8 @@ describe('shared/bridge', () => { const channel = createChannel(); - // Simulate "connect" RPC call by Bridge instance in channel's destination frame. - channel.methods.connect('TOKEN', sinon.stub()); - channel.methods.connect('TOKEN', sinon.stub()); + runConnectHandler(channel); + runConnectHandler(channel); assert.calledOnce(onConnectCallback); }); @@ -260,16 +317,16 @@ describe('shared/bridge', () => { describe('#destroy', () => it('destroys all opened channels', () => { - const channel1 = bridge.createChannel( - fakeWindow, - 'http://example.com', - 'foo' - ); - const channel2 = bridge.createChannel( - fakeWindow, - 'http://example.com', - 'bar' - ); + const channel1 = bridge.createChannel({ + source: fakeWindow, + origin: 'http://example.com', + token: 'foo', + }); + const channel2 = bridge.createChannel({ + source: fakeWindow, + origin: 'http://example.com', + token: 'bar', + }); bridge.destroy(); diff --git a/src/sidebar/services/frame-sync.js b/src/sidebar/services/frame-sync.js index 4a22a289808..b8dd2b1a7d6 100644 --- a/src/sidebar/services/frame-sync.js +++ b/src/sidebar/services/frame-sync.js @@ -236,7 +236,9 @@ export class FrameSyncService { }; const discovery = new Discovery(window, { server: true }); - discovery.startDiscovery(this._bridge.createChannel.bind(this._bridge)); + discovery.startDiscovery((source, origin, token) => + this._bridge.createChannel({ source, origin, token }) + ); this._bridge.onConnect(addFrame); this._setupSyncToFrame();