Skip to content

Commit

Permalink
Inabox host script - Cannot read property \'top\' of undefined (amppr…
Browse files Browse the repository at this point in the history
…oject#17749)

* initial commit

* lint fix
  • Loading branch information
keithwrightbos authored and torch2424 committed Aug 29, 2018
1 parent 7ab3689 commit e738e2b
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
15 changes: 9 additions & 6 deletions ads/inabox/inabox-host.js
Expand Up @@ -70,6 +70,13 @@ export class InaboxHost {
win.AMP[INABOX_UNREGISTER_IFRAME] = host.unregisterIframe.bind(host);
}
const queuedMsgs = win[PENDING_MESSAGES];
const processMessageFn = /** @type function(Event)*/(evt => {
try {
host.processMessage(evt);
} catch (err) {
dev().error(TAG, 'Error processing inabox message', evt, err);
}
});
if (queuedMsgs) {
if (Array.isArray(queuedMsgs)) {
queuedMsgs.forEach(message => {
Expand All @@ -78,11 +85,7 @@ export class InaboxHost {
if (!validateMessage(message)) {
return;
}
try {
host.processMessage(message);
} catch (err) {
dev().error(TAG, 'Error processing inabox message', message, err);
}
processMessageFn(message);
});
} else {
dev().info(TAG, `Invalid ${PENDING_MESSAGES}`, queuedMsgs);
Expand All @@ -91,7 +94,7 @@ export class InaboxHost {
// Empty and ensure that future messages are no longer stored in the array.
win[PENDING_MESSAGES] = [];
win[PENDING_MESSAGES]['push'] = () => {};
win.addEventListener('message', /** @type function(Event)*/ (host.processMessage.bind(host)));
win.addEventListener('message', processMessageFn.bind(host));
}
}

Expand Down
9 changes: 7 additions & 2 deletions ads/inabox/inabox-messaging-host.js
Expand Up @@ -253,8 +253,10 @@ export class InaboxMessagingHost {
if (this.iframeMap_[sentinel]) {
return this.iframeMap_[sentinel].measurableFrame;
}
const measurableFrame =
/** @type {HTMLIFrameElement} */(this.getMeasureableFrame(source));
const measurableFrame = this.getMeasureableFrame(source);
if (!measurableFrame) {
return null;
}
const measurableWin = measurableFrame.contentWindow;
for (let i = 0; i < this.iframes_.length; i++) {
const iframe = this.iframes_[i];
Expand Down Expand Up @@ -284,6 +286,9 @@ export class InaboxMessagingHost {
* @visibleForTesting
*/
getMeasureableFrame(win) {
if (!win) {
return null;
}
// First, we try to find the top-most x-domain window in win's parent
// hierarchy. If win is not nested within x-domain framing, then
// this loop breaks immediately.
Expand Down
11 changes: 11 additions & 0 deletions test/functional/inabox/test-inabox-messaging-host.js
Expand Up @@ -92,6 +92,17 @@ describes.realWin('inabox-host:messaging', {}, env => {
}),
})).to.be.false;
});

it('should tolerate message with null source', () => {
host.processMessage({
source: null,
origin: 'www.example.com',
data: 'amp-' + JSON.stringify({
sentinel: '0-123',
type: 'send-positions',
}),
});
});
});

describe('send-positions', () => {
Expand Down

0 comments on commit e738e2b

Please sign in to comment.