Skip to content

Commit

Permalink
Ensure messages send via the viewer integration are properly typed.
Browse files Browse the repository at this point in the history
  • Loading branch information
cramforce committed Jun 16, 2017
1 parent fac7c3a commit f5c1524
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 16 deletions.
11 changes: 6 additions & 5 deletions extensions/amp-viewer-integration/0.1/amp-viewer-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {getAmpDoc} from '../../../src/ampdoc';
import {isIframed} from '../../../src/dom';
import {listen, listenOnce} from '../../../src/event-helper';
import {dev} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {getSourceUrl} from '../../../src/url';
import {viewerForDoc} from '../../../src/services';

Expand Down Expand Up @@ -137,10 +138,10 @@ export class AmpViewerIntegration {
dev().fine(TAG, 'Send a handshake request');
const ampdocUrl = ampdoc.getUrl();
const srcUrl = getSourceUrl(ampdocUrl);
return messaging.sendRequest(RequestNames.CHANNEL_OPEN, {
url: ampdocUrl,
sourceUrl: srcUrl,
},
return messaging.sendRequest(RequestNames.CHANNEL_OPEN, dict({
'url': ampdocUrl,
'sourceUrl': srcUrl,
}),
true /* awaitResponse */)
.then(() => {
dev().fine(TAG, 'Channel has been opened!');
Expand Down Expand Up @@ -178,7 +179,7 @@ export class AmpViewerIntegration {
* @private
*/
handleUnload_(messaging) {
return messaging.sendRequest(RequestNames.UNLOADED, {}, true);
return messaging.sendRequest(RequestNames.UNLOADED, dict(), true);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export class Messaging {
/**
* I'm sending Bob a new outgoing request.
* @param {string} messageName
* @param {*} messageData
* @param {?JsonObject|string|undefined} messageData
* @param {boolean} awaitResponse
* @return {!Promise<*>|undefined}
*/
Expand Down
11 changes: 6 additions & 5 deletions extensions/amp-viewer-integration/0.1/touch-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@


import {listen} from '../../../src/event-helper';
import {dict} from '../../../src/utils/object';


/**
Expand Down Expand Up @@ -112,17 +113,17 @@ export class TouchHandler {
/**
* Makes a partial copy of the event.
* @param {!Event} e The event object to be copied.
* @return {!Object}
* @return {!JsonObject}
* @private
*/
copyTouchEvent_(e) {
const copiedEvent =
this.copyProperties_(e, EVENT_PROPERTIES);
if (e.touches) {
copiedEvent.touches = this.copyTouches_(e.touches);
copiedEvent['touches'] = this.copyTouches_(e.touches);
}
if (e.changedTouches) {
copiedEvent.changedTouches = this.copyTouches_(e.changedTouches);
copiedEvent['changedTouches'] = this.copyTouches_(e.changedTouches);
}
return copiedEvent;
}
Expand All @@ -146,11 +147,11 @@ export class TouchHandler {
* Copies specified properties of o to a new object.
* @param {!Object} o The source object.
* @param {!Array<string>} properties The properties to copy.
* @return {!Object} The copy of o.
* @return {!JsonObject} The copy of o.
* @private
*/
copyProperties_(o, properties) {
const copy = {};
const copy = dict();
for (let i = 0; i < properties.length; i++) {
const p = properties[i];
if (o[p] !== undefined) {
Expand Down
18 changes: 13 additions & 5 deletions src/service/viewer-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ export class Viewer {
/** @private {!Observable<!JsonObject>} */
this.broadcastObservable_ = new Observable();

/** @private {?function(string, *, boolean):(Promise<*>|undefined)} */
/**
* @private {?function(string, (?JsonObject|string|undefined), boolean):
* (Promise<*>|undefined)}
*/
this.messageDeliverer_ = null;

/** @private {?string} */
Expand All @@ -133,7 +136,7 @@ export class Viewer {
/**
* @private {!Array<!{
* eventType: string,
* data: *,
* data: (?JsonObject|string|undefined),
* awaitResponse: boolean,
* responsePromise: (Promise<*>|undefined),
* responseResolver: function(*)
Expand Down Expand Up @@ -769,7 +772,8 @@ export class Viewer {
/**
* Provides a message delivery mechanism by which AMP document can send
* messages to the viewer.
* @param {function(string, *, boolean):(!Promise<*>|undefined)} deliverer
* @param {function(string, (?JsonObject|string|undefined), boolean):
* (!Promise<*>|undefined)} deliverer
* @param {string} origin
* @export
*/
Expand Down Expand Up @@ -799,7 +803,9 @@ export class Viewer {
this.messageQueue_ = [];
queue.forEach(message => {
const responsePromise = this.messageDeliverer_(
message.eventType, message.data, message.awaitResponse);
message.eventType,
message.data,
message.awaitResponse);

if (message.awaitResponse) {
message.responseResolver(responsePromise);
Expand Down Expand Up @@ -855,7 +861,9 @@ export class Viewer {
// assimilating with the resolved (or rejected) internal value.
return /** @type {!Promise<?JsonObject|string|undefined>} */ (
Promise.resolve(this.messageDeliverer_(
eventType, data, awaitResponse)));
eventType,
/** @type {?JsonObject|string|undefined} */ (data),
awaitResponse)));
}

if (!this.messagingReadyPromise_) {
Expand Down

0 comments on commit f5c1524

Please sign in to comment.