Skip to content

Commit

Permalink
Wait for communication channel to setup before sending/broadcasting
Browse files Browse the repository at this point in the history
  • Loading branch information
Dima Voytenko committed Feb 15, 2016
1 parent dc54118 commit 245c081
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 21 deletions.
57 changes: 39 additions & 18 deletions src/service/viewer-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {getService} from '../service';
import {log} from '../log';
import {parseQueryString, parseUrl, removeFragment} from '../url';
import {platform} from '../platform';
import {timer} from '../timer';


const TAG_ = 'Viewer';
Expand Down Expand Up @@ -232,6 +233,21 @@ export class Viewer {
// Wait for document to become visible.
this.docState_.onVisibilityChanged(this.onVisibilityChange_.bind(this));

/**
* This promise will resolve when communications channel has been
* established or timeout in 5 seconds. The timeout is needed to avoid
* this promise becoming a memory leak with accumulating undelivered
* messages.
* @private @const {!Promise<!Viewer>}
*/
this.messagingReadyPromise_ = timer.timeoutPromise(
5000,
new Promise(resolve => {
/** @private @const {function(!Viewer)} */
this.messagingReadyResolver_ = resolve;
}));

// Trusted viewer and referrer.
let trustedViewerResolved;
let trustedViewerPromise;
if (!this.isEmbedded_) {
Expand Down Expand Up @@ -599,14 +615,6 @@ export class Viewer {
return this.sendMessage_('cid', undefined, true);
}

/**
* Broadcasts a message to all other AMP documents under the same viewer.
* @param {!JSONObject} message
*/
broadcast(message) {
this.sendMessage_('broadcast', message, false);
}

/**
* Triggers "tick" event for the viewer.
* @param {!JSONObject} message
Expand All @@ -630,15 +638,6 @@ export class Viewer {
this.sendMessage_('setFlushParams', message, false);
}

/**
* Registers receiver for the broadcast events.
* @param {function(!JSONObject)} handler
* @return {!Unlisten}
*/
onBroadcast(handler) {
return this.broadcastObservable_.add(handler);
}

/**
* Requests AMP document to receive a message from Viewer.
* @param {string} eventType
Expand Down Expand Up @@ -701,6 +700,7 @@ export class Viewer {
assert(!this.messageDeliverer_, 'message deliverer can only be set once');
log.fine(TAG_, 'message channel established with origin: ', origin);
this.messageDeliverer_ = deliverer;
this.messagingReadyResolver_(this);
// TODO(dvoytenko, #1764): Make `origin` required when viewers catch up.
this.messagingOrigin_ = origin;
if (this.trustedViewerResolver_) {
Expand All @@ -724,7 +724,28 @@ export class Viewer {
* @return {!Promise<*>|undefined}
*/
sendMessage(eventType, data, awaitResponse) {
return this.sendMessage_(eventType, data, awaitResponse);
return this.messagingReadyPromise_.then(() => {
return this.sendMessage_(eventType, data, awaitResponse);
});
}

/**
* Broadcasts a message to all other AMP documents under the same viewer.
* @param {!JSONObject} message
*/
broadcast(message) {
this.messagingReadyPromise_.then(() => {
return this.sendMessage_('broadcast', message, false);
});
}

/**
* Registers receiver for the broadcast events.
* @param {function(!JSONObject)} handler
* @return {!Unlisten}
*/
onBroadcast(handler) {
return this.broadcastObservable_.add(handler);
}

/**
Expand Down
78 changes: 75 additions & 3 deletions test/functional/test-viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@

import {Viewer} from '../../src/service/viewer-impl';
import {platform} from '../../src/platform';
import * as sinon from 'sinon';


describe('Viewer', () => {

let sandbox;
let clock;
let windowMock;
let viewer;
let windowApi;
let timeouts;

beforeEach(() => {
sandbox = sinon.sandbox.create();
clock = sandbox.useFakeTimers();
timeouts = [];
const WindowApi = function() {};
WindowApi.prototype.setTimeout = function(handler) {
Expand Down Expand Up @@ -182,10 +185,18 @@ describe('Viewer', () => {
});

it('should post broadcast event', () => {
const delivered = [];
viewer.setMessageDeliverer((eventType, data) => {
delivered.push({eventType: eventType, data: data});
}, 'https://acme.com');
viewer.broadcast({type: 'type1'});
const m = viewer.messageQueue_[0];
expect(m.eventType).to.equal('broadcast');
expect(m.data.type).to.equal('type1');
expect(viewer.messageQueue_.length).to.equal(0);
return viewer.messagingReadyPromise_.then(() => {
expect(delivered.length).to.equal(1);
const m = delivered[0];
expect(m.eventType).to.equal('broadcast');
expect(m.data.type).to.equal('type1');
});
});

it('should queue non-dupe events', () => {
Expand Down Expand Up @@ -218,6 +229,67 @@ describe('Viewer', () => {
expect(delivered[1].data.width).to.equal(13);
});

it('should wait for messaging channel', () => {
let m1Resolved = false;
let m2Resolved = false;
const m1 = viewer.sendMessage('message1', {}, /* awaitResponse */ false)
.then(() => {
m1Resolved = true;
});
const m2 = viewer.sendMessage('message2', {}, /* awaitResponse */ true)
.then(() => {
m2Resolved = true;
});
return Promise.resolve().then(() => {
// Not resolved yet.
expect(m1Resolved).to.be.false;
expect(m2Resolved).to.be.false;

// Set message deliverer.
viewer.setMessageDeliverer(() => {
return Promise.resolve();
}, 'https://acme.com');
expect(m1Resolved).to.be.false;
expect(m2Resolved).to.be.false;

return Promise.all([m1, m2]);
}).then(() => {
// All resolved now.
expect(m1Resolved).to.be.true;
expect(m2Resolved).to.be.true;
});
});

it('should timeout messaging channel', () => {
let m1Resolved = false;
let m2Resolved = false;
const m1 = viewer.sendMessage('message1', {}, /* awaitResponse */ false)
.then(() => {
m1Resolved = true;
});
const m2 = viewer.sendMessage('message2', {}, /* awaitResponse */ true)
.then(() => {
m2Resolved = true;
});
return Promise.resolve().then(() => {
// Not resolved yet.
expect(m1Resolved).to.be.false;
expect(m2Resolved).to.be.false;

// Timeout.
clock.tick(5001);
viewer.setMessageDeliverer(() => {
return Promise.resolve();
}, 'https://acme.com');
return Promise.all([m1, m2]);
}, error => {
expect(error).to.match(/timeout/);
// Not resolved ever.
expect(m1Resolved).to.be.true;
expect(m2Resolved).to.be.true;
});
});

describe('isTrustedViewer', () => {

function test(origin, toBeTrusted) {
Expand Down

0 comments on commit 245c081

Please sign in to comment.