From 6c7636ea0b4e973eb43441894aef921a2541b67a Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Fri, 16 Jun 2017 10:44:03 -0700 Subject: [PATCH] Properly type event.data Part of #9876 --- 3p/ampcontext.js | 6 +++--- 3p/iframe-messaging-client.js | 7 ++++--- 3p/integration.js | 14 +++++++------- 3p/messaging.js | 17 +++++++++-------- build-system/conformance-config.textproto | 12 ++++++++++++ src/event-helper.js | 8 ++++++++ 6 files changed, 43 insertions(+), 21 deletions(-) diff --git a/3p/ampcontext.js b/3p/ampcontext.js index 2143221d17364..0b3a5ecbc6e50 100644 --- a/3p/ampcontext.js +++ b/3p/ampcontext.js @@ -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']}); }); } @@ -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']); }); }; /** @@ -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']); }); }; diff --git a/3p/iframe-messaging-client.js b/3p/iframe-messaging-client.js index 69e405f53e4d0..250159774fab5 100644 --- a/3p/iframe-messaging-client.js +++ b/3p/iframe-messaging-client.js @@ -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'; @@ -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) { @@ -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; } @@ -130,7 +131,7 @@ export class IframeMessagingClient { /** * @param {string} messageType - * @return {!Observable} + * @return {!Observable} */ getOrCreateObservableFor_(messageType) { if (!(messageType in this.observableFor_)) { diff --git a/3p/integration.js b/3p/integration.js index 7378c4df28a3b..7f7cb4b236618 100644 --- a/3p/integration.js +++ b/3p/integration.js @@ -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(); } }); @@ -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']); }); } @@ -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']); }); } @@ -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']); }); } @@ -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']); }); } diff --git a/3p/messaging.js b/3p/messaging.js index ef0daae3558cd..df3713ccfd8ba 100644 --- a/3p/messaging.js +++ b/3p/messaging.js @@ -32,7 +32,7 @@ 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 = []; @@ -40,7 +40,7 @@ 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) { @@ -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. @@ -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; diff --git a/build-system/conformance-config.textproto b/build-system/conformance-config.textproto index f0f81a2812476..7ada3d0e5c9b4 100644 --- a/build-system/conformance-config.textproto +++ b/build-system/conformance-config.textproto @@ -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' diff --git a/src/event-helper.js b/src/event-helper.js index d73f6d2cd5c1f..a05be1ad1d8bb 100644 --- a/src/event-helper.js +++ b/src/event-helper.js @@ -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