Skip to content

Commit

Permalink
Properly type event.data
Browse files Browse the repository at this point in the history
  • Loading branch information
cramforce committed Jun 16, 2017
1 parent af5aad6 commit 6c7636e
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 21 deletions.
6 changes: 3 additions & 3 deletions 3p/ampcontext.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class AbstractAmpContext {
*/
onPageVisibilityChange(callback) {
return this.client_.registerCallback(MessageType.EMBED_STATE, data => {
callback({hidden: data.pageHidden});
callback({hidden: data['pageHidden']});
});
}

Expand Down Expand Up @@ -194,7 +194,7 @@ export class AbstractAmpContext {
*/
onResizeSuccess(callback) {
this.client_.registerCallback(MessageType.EMBED_SIZE_CHANGED, obj => {
callback(obj.requestedHeight, obj.requestedWidth); });
callback(obj['requestedHeight'], obj['requestedWidth']); });
};

/**
Expand All @@ -206,7 +206,7 @@ export class AbstractAmpContext {
*/
onResizeDenied(callback) {
this.client_.registerCallback(MessageType.EMBED_SIZE_DENIED, obj => {
callback(obj.requestedHeight, obj.requestedWidth);
callback(obj['requestedHeight'], obj['requestedWidth']);
});
};

Expand Down
7 changes: 4 additions & 3 deletions 3p/iframe-messaging-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
serializeMessage,
deserializeMessage,
} from '../src/3p-frame-messaging';
import {getData} from '../src/event-helper';
import {getMode} from '../src/mode';
import {dev} from '../src/log';

Expand Down Expand Up @@ -66,7 +67,7 @@ export class IframeMessagingClient {
* All future calls will overwrite any previously registered
* callbacks.
* @param {string} messageType The type of the message.
* @param {function(Object)} callback The callback function to call
* @param {function(?JsonObject)} callback The callback function to call
* when a message with type messageType is received.
*/
registerCallback(messageType, callback) {
Expand Down Expand Up @@ -105,7 +106,7 @@ export class IframeMessagingClient {
return;
}

const message = deserializeMessage(event.data);
const message = deserializeMessage(getData(event));
if (!message || message['sentinel'] != this.sentinel_) {
return;
}
Expand All @@ -130,7 +131,7 @@ export class IframeMessagingClient {

/**
* @param {string} messageType
* @return {!Observable<Object>}
* @return {!Observable<?JsonObject>}
*/
getOrCreateObservableFor_(messageType) {
if (!(messageType in this.observableFor_)) {
Expand Down
14 changes: 7 additions & 7 deletions 3p/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,8 @@ function getHtml(selector, attributes, callback) {
}));

const unlisten = listenParent(window, 'get-html-result', data => {
if (data.messageId === messageId) {
callback(data.content);
if (data['messageId'] === messageId) {
callback(data['content']);
unlisten();
}
});
Expand All @@ -609,7 +609,7 @@ function observeIntersection(observerCallback) {
// Send request to received records.
nonSensitiveDataPostMessage('send-intersections');
return listenParent(window, 'intersection', data => {
observerCallback(data.changes);
observerCallback(data['changes']);
});
}

Expand All @@ -620,8 +620,8 @@ function observeIntersection(observerCallback) {
*/
function updateVisibilityState(global) {
listenParent(window, 'embed-state', function(data) {
global.context.hidden = data.pageHidden;
dispatchVisibilityChangeEvent(global, data.pageHidden);
global.context.hidden = data['pageHidden'];
dispatchVisibilityChangeEvent(global, data['pageHidden']);
});
}

Expand All @@ -641,7 +641,7 @@ function dispatchVisibilityChangeEvent(win, isHidden) {
*/
function onResizeSuccess(observerCallback) {
return listenParent(window, 'embed-size-changed', data => {
observerCallback(data.requestedHeight, data.requestedWidth);
observerCallback(data['requestedHeight'], data['requestedWidth']);
});
}

Expand All @@ -653,7 +653,7 @@ function onResizeSuccess(observerCallback) {
*/
function onResizeDenied(observerCallback) {
return listenParent(window, 'embed-size-denied', data => {
observerCallback(data.requestedHeight, data.requestedWidth);
observerCallback(data['requestedHeight'], data['requestedWidth']);
});
}

Expand Down
17 changes: 9 additions & 8 deletions 3p/messaging.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ export function nonSensitiveDataPostMessage(type, opt_object) {

/**
* Message event listeners.
* @const {!Array<{type: string, cb: function(!Object)}>}
* @const {!Array<{type: string, cb: function(!JsonObject)}>}
*/
const listeners = [];

/**
* Listen to message events from document frame.
* @param {!Window} win
* @param {string} type Type of messages
* @param {function(*)} callback Called with data payload of message.
* @param {function(!JsonObject)} callback Called with data payload of message.
* @return {function()} function to unlisten for messages.
*/
export function listenParent(win, type, callback) {
Expand Down Expand Up @@ -70,16 +70,17 @@ function startListening(win) {
win.AMP_LISTENING = true;
win.addEventListener('message', function(event) {
// Cheap operations first, so we don't parse JSON unless we have to.
const eventData = getData(event);
if (event.source != win.parent ||
event.origin != win.context.location.origin ||
typeof event.data != 'string' ||
event.data.indexOf('amp-') != 0) {
typeof eventData != 'string' ||
eventData.indexOf('amp-') != 0) {
return;
}
// Parse JSON only once per message.
const data = /** @type {!Object} */ (
JSON.parse(event.data.substr(4)));
if (win.context.sentinel && data.sentinel != win.context.sentinel) {
const data = /** @type {!JsonObject} */ (
JSON.parse(eventData.substr(4)));
if (win.context.sentinel && data['sentinel'] != win.context.sentinel) {
return;
}
// Don't let other message handlers interpret our events.
Expand All @@ -88,7 +89,7 @@ function startListening(win) {
}
// Find all the listeners for this type.
for (let i = 0; i < listeners.length; i++) {
if (listeners[i].type != data.type) {
if (listeners[i].type != data['type']) {
continue;
}
const cb = listeners[i].cb;
Expand Down
12 changes: 12 additions & 0 deletions build-system/conformance-config.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,18 @@ requirement: {
value: 'Symbol'
}

requirement: {
type: BANNED_PROPERTY_READ
error_message: 'Use eventHelper#getData to read the data property. OK to whitelist 3p ads for now.'
value: 'Event.prototype.data'
value: 'MessageEvent.prototype.data'
whitelist: 'src/event-helper.js'
whitelist: 'ads/adfox.js'
whitelist: 'ads/google/imaVideo.js'
whitelist: 'ads/netletix.js'
whitelist: 'ads/yandex.js'
}

requirement: {
type: RESTRICTED_METHOD_CALL
error_message: 'postMessage must be called with a string or a JsonObject'
Expand Down
8 changes: 8 additions & 0 deletions src/event-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ export function listen(element, eventType, listener, opt_capture) {
element, eventType, listener, opt_capture);
}

/**
* Returns the data property of an event with the correct type.
* @param {!Event} event
* @return {?JsonObject|string|undefined}
*/
export function getData(event) {
return /** @type {?JsonObject|string|undefined} */ (event.data);
}

/**
* Listens for the specified event on the element and removes the listener
Expand Down

0 comments on commit 6c7636e

Please sign in to comment.