Skip to content

Commit

Permalink
Added logic to delay resize requests to be made after an invalid requ…
Browse files Browse the repository at this point in the history
…est is made until 500ms passes (#35603)

* added fixed and nit tests

* Fixed a few issues: made a variable private

* rebuilding as buidl was failing
  • Loading branch information
jshamble committed Aug 13, 2021
1 parent 6f3375f commit 88302b9
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 8 deletions.
49 changes: 41 additions & 8 deletions extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const MIN_INABOX_POSITION_EVENT_INTERVAL = 100;
/** @type {string} */
const TAG = 'amp-ad-xorigin-iframe';

/** @type {number} */
const MSEC_REPEATED_REQUEST_DELAY = 500;

export class AmpAdXOriginIframeHandler {
/**
* @param {!./amp-ad-3p-impl.AmpAd3PImpl|!../../amp-a4a/0.1/amp-a4a.AmpA4A} baseInstance
Expand All @@ -64,6 +67,15 @@ export class AmpAdXOriginIframeHandler {
/** @type {?HTMLIFrameElement} iframe instance */
this.iframe = null;

/* This variable keeps keeps track when an invalid resize request is made, and
* is associated with each iframe. If the request is invalid, then a new request
* cannot be made until a certain amount of time has passed, 500 ms by default
* (see MSEC_REPEATED_REQUEST_DELAY). Once the timer has cooled down,
* a new request can be made.
*/
/** @private {number} */
this.lastRejectedResizeTime_ = 0;

/** @private {?LegacyAdIntersectionObserverHost} */
this.legacyIntersectionObserverApiHost_ = null;

Expand Down Expand Up @@ -170,14 +182,29 @@ export class AmpAdXOriginIframeHandler {
if (!!data['hasOverflow']) {
this.element_.warnOnMissingOverflow = false;
}
this.handleResize_(
data['id'],
data['height'],
data['width'],
source,
origin,
event
);
if (
Date.now() - this.lastRejectedResizeTime_ >=
MSEC_REPEATED_REQUEST_DELAY
) {
this.handleResize_(
data['id'],
data['height'],
data['width'],
source,
origin,
event
);
} else {
// need to wait 500ms until next resize request is allowed.
this.sendEmbedSizeResponse_(
false,
data['id'],
data['width'],
data['height'],
source,
origin
);
}
},
true,
true
Expand Down Expand Up @@ -455,6 +482,12 @@ export class AmpAdXOriginIframeHandler {
.updateSize(height, width, iframeHeight, iframeWidth, event)
.then(
(info) => {
if (!info.success) {
// invalid request parameters, disable requests for 500ms
this.lastRejectedResizeTime_ = Date.now();
} else {
this.lastRejectedResizeTime_ = 0;
}
this.uiHandler_.onResizeSuccess();
this.sendEmbedSizeResponse_(
info.success,
Expand Down
100 changes: 100 additions & 0 deletions extensions/amp-ad/0.1/test/test-amp-ad-xorigin-iframe-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describes.sandboxed('amp-ad-xorigin-iframe-handler', {}, (env) => {
let renderStartedSpy;
let iframeHandler;
let iframe;
let clock;
let testIndex = 0;

beforeEach(() => {
Expand Down Expand Up @@ -338,6 +339,105 @@ describes.sandboxed('amp-ad-xorigin-iframe-handler', {}, (env) => {
});
});

describe('request valid and invalid ad resize useing embed-size API', async () => {
beforeEach(async () => {
clock = env.sandbox.useFakeTimers();
const updateSizeWrapper = env.sandbox.stub(
adImpl.uiHandler,
'updateSize'
);

updateSizeWrapper.resolves({
success: false,
newWidth: 114,
newHeight: 217,
});

// expect to fail, first call invalid request
iframe.postMessageToParent({
width: 114,
height: 217,
type: 'embed-size',
sentinel: 'amp3ptest' + testIndex,
});

const data = await iframe.expectMessageFromParent('embed-size-denied');

expect(data).to.jsonEqual({
requestedWidth: 114,
requestedHeight: 217,
type: 'embed-size-denied',
sentinel: 'amp3ptest' + testIndex,
});

updateSizeWrapper.resolves({
success: true,
newWidth: 114,
newHeight: 217,
});

// expect to fail invalid request right before 500ms
iframe.postMessageToParent({
width: 120,
height: 217,
type: 'embed-size',
sentinel: 'amp3ptest' + testIndex,
});

const data2 = await iframe.expectMessageFromParent('embed-size-denied');

expect(data2).to.jsonEqual({
requestedWidth: 120,
requestedHeight: 217,
type: 'embed-size-denied',
sentinel: 'amp3ptest' + testIndex,
});
});

it('should be able to use embed-size API, change size deny repeated attempts, but then denies valid attempt before 500ms delay', async () => {
// expect to fail, valid request, but called before 500ms
clock.tick(250);

iframe.postMessageToParent({
width: 114,
height: 217,
type: 'embed-size',
sentinel: 'amp3ptest' + testIndex,
});

const data3 = await iframe.expectMessageFromParent('embed-size-denied');
expect(data3).to.jsonEqual({
requestedWidth: 114,
requestedHeight: 217,
type: 'embed-size-denied',
sentinel: 'amp3ptest' + testIndex,
});
});

it('should be able to use embed-size API, change size deny repeated attempts, but then works after 500ms delay', async () => {
// expect to succeed: valid request right after 500ms
clock.tick(500);

iframe.postMessageToParent({
width: 114,
height: 217,
type: 'embed-size',
sentinel: 'amp3ptest' + testIndex,
});

const data3 = await iframe.expectMessageFromParent(
'embed-size-changed'
);

expect(data3).to.jsonEqual({
requestedWidth: 114,
requestedHeight: 217,
type: 'embed-size-changed',
sentinel: 'amp3ptest' + testIndex,
});
});
});

it('should be able to use embed-size API, change size succeed', () => {
env.sandbox.stub(adImpl.uiHandler, 'updateSize').callsFake(() => {
return Promise.resolve({
Expand Down

0 comments on commit 88302b9

Please sign in to comment.