Skip to content

Commit

Permalink
Refactor AmpContext dependencies for building (ampproject#8258)
Browse files Browse the repository at this point in the history
* Fix types for AmpContext

* Move and split dependencies for ampcontext-lib

* Enable type checks for ampcontext-lib
  • Loading branch information
alanorozco authored and mrjoro committed Apr 28, 2017
1 parent 0d49613 commit 559e2ca
Show file tree
Hide file tree
Showing 16 changed files with 240 additions and 118 deletions.
10 changes: 8 additions & 2 deletions 3p/ampcontext-lib.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,17 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import './polyfills';
import {AmpContext} from './ampcontext.js';
import {initLogConstructor, setReportError} from '../src/log';
import {reportError} from '../src/error';


initLogConstructor();
setReportError(reportError);


// TODO(alanorozco): Refactor src/error.reportError so it does not contain big
// transitive dependencies and can be included here.
setReportError(() => {});


/**
Expand Down
52 changes: 37 additions & 15 deletions 3p/ampcontext.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,44 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import './polyfills';
import {dev} from '../src/log';
import {IframeMessagingClient} from './iframe-messaging-client';
import {MessageType} from '../src/3p-frame';
import {MessageType} from '../src/3p-frame-messaging';
import {tryParseJson} from '../src/json';

export class AmpContext {

/**
* @param {Window} win The window that the instance is built inside.
* @param {!Window} win The window that the instance is built inside.
*/
constructor(win) {
/** @private {!Window} */
this.win_ = win;

/** @type {?string} */
this.location = null;

/** @type {?string} */
this.canonicalUrl = null;

/** @type {?string} */
this.pageViewId = null;

/** @type {?string} */
this.sentinel = null;

// TODO(alanorozco): confirm type
/** @type {?} */
this.startTime = null;

// TODO(alanorozco): confirm type
/** @type {?string} */
this.referrer = null;

this.findAndSetMetadata_();
this.client_ = new IframeMessagingClient(win);
this.client_.setHostWindow(this.getHostWindow_());
this.client_.setSentinel(this.sentinel);
this.client_.setSentinel(dev().assertString(this.sentinel));
}

/**
Expand Down Expand Up @@ -64,8 +84,8 @@ export class AmpContext {
/**
* Send message to runtime requesting to resize ad to height and width.
* This is not guaranteed to succeed. All this does is make the request.
* @param {int} width The new width for the ad we are requesting.
* @param {int} height The new height for the ad we are requesting.
* @param {number} width The new width for the ad we are requesting.
* @param {number} height The new height for the ad we are requesting.
*/
requestResize(width, height) {
this.client_.sendMessage(MessageType.EMBED_SIZE, {width, height});
Expand All @@ -75,8 +95,8 @@ export class AmpContext {
* Allows a creative to set the callback function for when the resize
* request returns a success. The callback should be set before resizeAd
* is ever called.
* @param {function(requestedHeight, requestedWidth)} callback Function
* to call if the resize request succeeds.
* @param {function(number, number)} callback Function to call if the resize
* request succeeds.
*/
onResizeSuccess(callback) {
this.client_.registerCallback(MessageType.EMBED_SIZE_CHANGED, obj => {
Expand All @@ -87,8 +107,8 @@ export class AmpContext {
* Allows a creative to set the callback function for when the resize
* request is denied. The callback should be set before resizeAd
* is ever called.
* @param {function(requestedHeight, requestedWidth)} callback Function
* to call if the resize request is denied.
* @param {function(number, number)} callback Function to call if the resize
* request is denied.
*/
onResizeDenied(callback) {
this.client_.registerCallback(MessageType.EMBED_SIZE_DENIED, obj => {
Expand All @@ -99,7 +119,7 @@ export class AmpContext {
/**
* Takes the current name on the window, and attaches it to
* the name of the iframe.
* @param {HTMLIframeElement} iframe The iframe we are adding the context to.
* @param {HTMLIFrameElement} iframe The iframe we are adding the context to.
*/
addContextToIframe(iframe) {
iframe.name = this.win_.name;
Expand All @@ -108,15 +128,15 @@ export class AmpContext {
/**
* Parse the metadata attributes from the name and add them to
* the class instance.
* @param {!Object|string} contextData
* @param {!Object|string} data
* @private
*/
setupMetadata_(data) {
data = tryParseJson(data);
if (!data) {
const dataObject = typeof data === 'string' ? tryParseJson(data) : data;
if (!dataObject) {
throw new Error('Could not setup metadata.');
}
const context = data._context;
const context = dataObject._context;
this.location = context.location;
this.canonicalUrl = context.canonicalUrl;
this.pageViewId = context.pageViewId;
Expand Down Expand Up @@ -149,6 +169,8 @@ export class AmpContext {
findAndSetMetadata_() {
// If the context data is set on window, that means we don't need
// to check the name attribute as it has been bypassed.
// TODO(alanorozco): why the heck could AMP_CONTEXT_DATA be two different
// types? FIX THIS.
if (!this.win_.AMP_CONTEXT_DATA) {
this.setupMetadata_(this.win_.name);
} else if (typeof this.win_.AMP_CONTEXT_DATA == 'string') {
Expand Down
7 changes: 5 additions & 2 deletions 3p/iframe-messaging-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {listen} from '../src/event-helper';
import {map} from '../src/utils/object';
import {serializeMessage, deserializeMessage} from '../src/3p-frame';
import {
listen,
serializeMessage,
deserializeMessage,
} from '../src/3p-frame-messaging';
import {getMode} from '../src/mode';
import {dev} from '../src/log';

Expand Down
2 changes: 1 addition & 1 deletion ads/inabox/inabox-messaging-host.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
serializeMessage,
deserializeMessage,
MessageType,
} from '../../src/3p-frame';
} from '../../src/3p-frame-messaging';
import {dev} from '../../src/log';

/** @const */
Expand Down
2 changes: 2 additions & 0 deletions build-system/amp.extern.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ window.AMP_CONFIG.thirdPartyFrameRegex;
window.AMP_CONFIG.cdnUrl;
window.AMP_CONFIG.errorReportingUrl;

window.AMP_CONTEXT_DATA;

// amp-viz-vega related externs.
/**
* @typedef {{spec: function(!JSONType, function())}}
Expand Down
9 changes: 9 additions & 0 deletions build-system/tasks/presubmit-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,15 @@ var forbiddenTerms = {
'src/inabox/inabox-viewer.js',
],
},
'internalListenImplementation': {
message: 'Use `listen()` in either `event-helper` or `3p-frame-messaging`' +
', depending on your use case.',
whitelist: [
'src/3p-frame-messaging.js',
'src/event-helper.js',
'src/event-helper-listen.js',
],
},
'setTimeout.*throw': {
message: 'Use dev.error or user.error instead.',
whitelist: [
Expand Down
7 changes: 7 additions & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,13 @@ function checkTypes() {
includePolyfills: true,
checkTypes: true,
}),
closureCompile(['./3p/ampcontext-lib.js'], './dist',
'ampcontext-check-types.js', {
externs: ['ads/ads.extern.js'],
include3pDirectories: true,
includePolyfills: true,
checkTypes: true,
}),
]);
}

Expand Down
107 changes: 107 additions & 0 deletions src/3p-frame-messaging.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {dev} from './log';
import {internalListenImplementation} from './event-helper-listen';


/** @const */
const AMP_MESSAGE_PREFIX = 'amp-';


/** @enum {string} */
export const MessageType = {
// For amp-ad
SEND_EMBED_STATE: 'send-embed-state',
EMBED_STATE: 'embed-state',
SEND_EMBED_CONTEXT: 'send-embed-context',
EMBED_CONTEXT: 'embed-context',
SEND_INTERSECTIONS: 'send-intersections',
INTERSECTION: 'intersection',
EMBED_SIZE: 'embed-size',
EMBED_SIZE_CHANGED: 'embed-size-changed',
EMBED_SIZE_DENIED: 'embed-size-denied',

// For amp-inabox
SEND_POSITIONS: 'send-positions',
POSITION: 'position',
};


/**
* Listens for the specified event on the element.
* @param {!EventTarget} element
* @param {string} eventType
* @param {function(!Event)} listener
* @param {boolean=} opt_capture
* @return {!UnlistenDef}
*/
export function listen(element, eventType, listener, opt_capture) {
return internalListenImplementation(
element, eventType, listener, opt_capture);
}


/**
* Serialize an AMP post message. Output looks like:
* 'amp-011481323099490{"type":"position","sentinel":"12345","foo":"bar"}'
* @param {string} type
* @param {string} sentinel
* @param {Object=} data
* @param {?string=} rtvVersion
* @returns {string}
*/
export function serializeMessage(type, sentinel, data = {}, rtvVersion = null) {
// TODO: consider wrap the data in a "data" field. { type, sentinal, data }
const message = data;
message.type = type;
message.sentinel = sentinel;
return AMP_MESSAGE_PREFIX + (rtvVersion || '') + JSON.stringify(message);
}


/**
* Deserialize an AMP post message.
* Returns null if it's not valid AMP message format.
*
* @param {*} message
* @returns {?JSONType}
*/
export function deserializeMessage(message) {
if (!isAmpMessage(message)) {
return null;
}
const startPos = message.indexOf('{');
dev().assert(startPos != -1, 'JSON missing in %s', message);
try {
return /** @type {!JSONType} */ (JSON.parse(message.substr(startPos)));
} catch (e) {
dev().error('MESSAGING', 'Failed to parse message: ' + message, e);
return null;
}
}


/**
* Returns true if message looks like it is an AMP postMessage
* @param {*} message
* @return {!boolean}
*/
export function isAmpMessage(message) {
return (typeof message == 'string' &&
message.indexOf(AMP_MESSAGE_PREFIX) == 0 &&
message.indexOf('{') != -1);
}
71 changes: 0 additions & 71 deletions src/3p-frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,74 +310,3 @@ export function generateSentinelAndContext(iframe, window) {
export function resetCountForTesting() {
count = {};
}


/** @const */
const AMP_MESSAGE_PREFIX = 'amp-';

/** @enum {string} */
export const MessageType = {
// For amp-ad
SEND_EMBED_STATE: 'send-embed-state',
EMBED_STATE: 'embed-state',
SEND_EMBED_CONTEXT: 'send-embed-context',
EMBED_CONTEXT: 'embed-context',
SEND_INTERSECTIONS: 'send-intersections',
INTERSECTION: 'intersection',
EMBED_SIZE: 'embed-size',
EMBED_SIZE_CHANGED: 'embed-size-changed',
EMBED_SIZE_DENIED: 'embed-size-denied',

// For amp-inabox
SEND_POSITIONS: 'send-positions',
POSITION: 'position',
};

/**
* Serialize an AMP post message. Output looks like:
* 'amp-011481323099490{"type":"position","sentinel":"12345","foo":"bar"}'
* @param {string} type
* @param {string} sentinel
* @param {Object=} data
* @param {?string=} rtvVersion
* @returns {string}
*/
export function serializeMessage(type, sentinel, data = {}, rtvVersion = null) {
// TODO: consider wrap the data in a "data" field. { type, sentinal, data }
const message = data;
message.type = type;
message.sentinel = sentinel;
return AMP_MESSAGE_PREFIX + (rtvVersion || '') + JSON.stringify(message);
}

/**
* Deserialize an AMP post message.
* Returns null if it's not valid AMP message format.
*
* @param {*} message
* @returns {?JSONType}
*/
export function deserializeMessage(message) {
if (!isAmpMessage(message)) {
return null;
}
const startPos = message.indexOf('{');
dev().assert(startPos != -1, 'JSON missing in %s', message);
try {
return /** @type {!JSONType} */ (JSON.parse(message.substr(startPos)));
} catch (e) {
dev().error('MESSAGING', 'Failed to parse message: ' + message, e);
return null;
}
}

/**
* Returns true if message looks like it is an AMP postMessage
* @param {*} message
* @return {!boolean}
*/
export function isAmpMessage(message) {
return (typeof message == 'string' &&
message.indexOf(AMP_MESSAGE_PREFIX) == 0 &&
message.indexOf('{') != -1);
}
Loading

0 comments on commit 559e2ca

Please sign in to comment.