Skip to content

Commit

Permalink
Remove getChangeIntersectionEntry from LegacyAdIntersectionObserverHo…
Browse files Browse the repository at this point in the history
…st (#31693)

* Remove getChangeIntersectionEntry from adintersectionobserverhost

* update tests

* InOb workaround.

* remove comment

* fixes

* fix

* fix unit test

* anotha fix

* serilization helpers

* account for null

* Trigger Build
  • Loading branch information
samouri committed Dec 29, 2020
1 parent 8543a2c commit ecd7afd
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 39 deletions.
3 changes: 1 addition & 2 deletions examples/ac-creative.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ if (!window.context) {
document.head.appendChild(ampContextScript);
}

function intersectionCallback(payload) {
var changes = payload.changes;
function intersectionCallback(changes) {
// Code below is simply an example.
var latestChange = changes[changes.length - 1];

Expand Down
3 changes: 1 addition & 2 deletions examples/ampcontext-creative.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
document.head.appendChild(ampContextScript);
}

function intersectionCallback(payload) {
var changes = payload.changes;
function intersectionCallback(changes) {
// Code below is simply an example.
var latestChange = changes[changes.length - 1];

Expand Down
70 changes: 60 additions & 10 deletions extensions/amp-ad/0.1/legacy-ad-intersection-observer-host.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ export class LegacyAdIntersectionObserverHost {
/** @private {?IntersectionObserver} */
this.intersectionObserver_ = null;

/** @private {?IntersectionObserver} */
this.fireInOb_ = null;

/** @private {boolean} */
this.inViewport_ = false;

Expand Down Expand Up @@ -181,7 +184,11 @@ export class LegacyAdIntersectionObserverHost {
* Fires element intersection
*/
fire() {
this.sendElementIntersection_();
if (!this.fireInOb_) {
return;
}
this.fireInOb_.unobserve(this.baseElement_.element);
this.fireInOb_.observe(this.baseElement_.element);
}

/**
Expand All @@ -208,24 +215,33 @@ export class LegacyAdIntersectionObserverHost {
if (!this.intersectionObserver_) {
this.intersectionObserver_ = new IntersectionObserver((entries) => {
const lastEntry = entries[entries.length - 1];
this.onViewportCallback_(lastEntry.intersectionRatio != 0);
this.onViewportCallback_(lastEntry);
});
this.intersectionObserver_.observe(this.baseElement_.element);
}
if (!this.fireInOb_) {
this.fireInOb_ = new IntersectionObserver((entries) => {
const lastEntry = entries[entries.length - 1];
this.sendElementIntersection_(lastEntry);
});
}
this.fire();
}

/**
* Triggered when the ad either enters or exits the visible viewport.
* @param {boolean} inViewport true if the element is in viewport.
* @param {!IntersectionObserverEntry} entry handed over by the IntersectionObserver.
*/
onViewportCallback_(inViewport) {
onViewportCallback_(entry) {
const inViewport = entry.intersectionRatio != 0;
if (this.inViewport_ == inViewport) {
return;
}
this.inViewport_ = inViewport;

// Lets the ad know that it became visible or no longer is.
this.fire();
this.sendElementIntersection_(entry);

// And update the ad about its position in the viewport while
// it is visible.
if (inViewport) {
Expand All @@ -247,13 +263,12 @@ export class LegacyAdIntersectionObserverHost {
* Sends 'intersection' message to ad/iframe with intersection change records
* if this has been activated and we measured the layout box of the iframe
* at least once.
* @param {!IntersectionObserverEntry} entry - handed over by the IntersectionObserver.
* @private
*/
sendElementIntersection_() {
if (!this.intersectionObserver_) {
return;
}
const change = this.baseElement_.element.getIntersectionChangeEntry();
sendElementIntersection_(entry) {
const change = intersectionEntryToJson(entry);

if (
this.pendingChanges_.length > 0 &&
this.pendingChanges_[this.pendingChanges_.length - 1].time == change.time
Expand Down Expand Up @@ -295,8 +310,43 @@ export class LegacyAdIntersectionObserverHost {
this.intersectionObserver_.disconnect();
this.intersectionObserver_ = null;
}
if (this.fireInOb_) {
this.fireInOb_.disconnect();
this.fireInOb_ = null;
}
this.timer_.cancel(this.flushTimeout_);
this.unlistenOnOutViewport_();
this.postMessageApi_.destroy();
}
}

/**
* Convert a DOMRect to a regular object to make it serializable.
*
* @param {!DOMRect} domRect
* @return {!DOMRect}
*/
function domRectToJson(domRect) {
if (domRect == null) {
return domRect;
}

const {x, y, width, height, top, right, bottom, left} = domRect;
return {x, y, width, height, top, right, bottom, left};
}

/**
* Convert an IntersectionObserverEntry to a regular object to make it serializable.
*
* @param {!IntersectionObserverEntry} entry
* @return {!IntersectionObserverEntry}
*/
function intersectionEntryToJson(entry) {
return {
time: entry.time,
rootBounds: domRectToJson(entry.rootBounds),
boundingClientRect: domRectToJson(entry.boundingClientRect),
intersectionRect: domRectToJson(entry.intersectionRect),
intersectionRatio: entry.intersectionRatio,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,16 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {

let testIframe;
let element;
let getIntersectionChangeEntrySpy;
let onScrollSpy;
let onChangeSpy;
let clock;
let stubFireInOb;

function getInObEntry() {
const rootBounds = layoutRectLtwh(198, 299, 100, 100);
const layoutBox = layoutRectLtwh(50, 100, 150, 200);
return getIntersectionChangeEntry(layoutBox, null, rootBounds);
}

function getIframe(src) {
const i = document.createElement('iframe');
Expand All @@ -312,7 +318,6 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {

beforeEach(() => {
clock = window.sandbox.useFakeTimers();
getIntersectionChangeEntrySpy = window.sandbox.spy();
onScrollSpy = window.sandbox.spy();
onChangeSpy = window.sandbox.spy();
testIframe = getIframe(iframeSrc);
Expand All @@ -330,13 +335,26 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
},
};
};
element.element = document.createElement('amp-int');
element.element.getIntersectionChangeEntry = () => {
getIntersectionChangeEntrySpy();
const rootBounds = layoutRectLtwh(198, 299, 100, 100);
const layoutBox = layoutRectLtwh(50, 100, 150, 200);
return getIntersectionChangeEntry(layoutBox, null, rootBounds);

stubFireInOb = (host) => {
let fireObserved = false;
host.fireInOb_ = {
observe() {
if (!fireObserved) {
setTimeout(() => {
host.sendElementIntersection_(getInObEntry());
fireObserved = false;
}, 0);
}
fireObserved = true;
},
unobserve: () => (fireObserved = false),
disconnect: window.sandbox.spy(),
};
return host.fireInOb_;
};

element.element = document.createElement('amp-int');
});

afterEach(() => {
Expand All @@ -351,7 +369,8 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
insert(testIframe);
const postMessageSpy = window.sandbox /*OK*/
.spy(testIframe.contentWindow, 'postMessage');
ioInstance.sendElementIntersection_();
ioInstance.fire();
clock.tick(0);
expect(postMessageSpy).to.have.not.been.called;
expect(ioInstance.pendingChanges_).to.have.length(0);
});
Expand All @@ -362,6 +381,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
element,
testIframe
);
stubFireInOb(ioInstance);
insert(testIframe);
testIframe.contentWindow.postMessage = (message) => {
messages.push(deserializeMessage(message));
Expand All @@ -371,7 +391,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
{win: testIframe.contentWindow, origin: '*'},
];
ioInstance.startSendingIntersectionChanges_();
expect(getIntersectionChangeEntrySpy).to.be.calledOnce;
clock.tick(0);
expect(messages).to.have.length(1);
expect(ioInstance.pendingChanges_).to.have.length(0);
expect(messages[0].changes).to.have.length(1);
Expand All @@ -384,6 +404,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
element,
testIframe
);
stubFireInOb(ioInstance);
insert(testIframe);
testIframe.contentWindow.postMessage = (message) => {
messages.push(deserializeMessage(message));
Expand All @@ -392,7 +413,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
{win: testIframe.contentWindow, origin: '*'},
];
ioInstance.startSendingIntersectionChanges_();
expect(getIntersectionChangeEntrySpy).to.be.calledOnce;
clock.tick(0);
expect(messages).to.have.length(1);
expect(messages[0].changes).to.have.length(1);
expect(messages[0].changes[0].time).to.equal(0);
Expand All @@ -402,6 +423,7 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
ioInstance.fire();
ioInstance.fire(); // Same time
ioInstance.fire(); // Same time
clock.tick(0);
expect(ioInstance.pendingChanges_).to.have.length(2);
expect(messages).to.have.length(1);
clock.tick(1);
Expand All @@ -411,10 +433,12 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
expect(messages[1].changes[1].time).to.equal(99);
expect(ioInstance.pendingChanges_).to.have.length(0);
ioInstance.fire();
clock.tick(0);
expect(ioInstance.pendingChanges_).to.have.length(0);
expect(messages).to.have.length(3);
clock.tick(99);
ioInstance.fire();
clock.tick(0);
expect(ioInstance.pendingChanges_).to.have.length(1);
expect(messages).to.have.length(3);
clock.tick(1);
Expand All @@ -427,35 +451,36 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
});

it('should init listeners when element is in viewport', () => {
const fireSpy = window.sandbox.spy(
const sendElementIntersectionSpy = window.sandbox.spy(
LegacyAdIntersectionObserverHost.prototype,
'fire'
'sendElementIntersection_'
);
const ioInstance = new LegacyAdIntersectionObserverHost(
element,
testIframe
);
stubFireInOb(ioInstance);
insert(testIframe);
ioInstance.onViewportCallback_(true);
expect(fireSpy).to.be.calledOnce;
expect(sendElementIntersectionSpy).to.be.calledOnce;
expect(onScrollSpy).to.be.calledOnce;
expect(onChangeSpy).to.be.calledOnce;
expect(ioInstance.unlistenViewportChanges_).to.not.be.null;
});

it('should unlisten listeners when element is out of viewport', () => {
const fireSpy = window.sandbox.spy(
const sendElementIntersectionSpy = window.sandbox.spy(
LegacyAdIntersectionObserverHost.prototype,
'fire'
'sendElementIntersection_'
);
const ioInstance = new LegacyAdIntersectionObserverHost(
element,
testIframe
);
insert(testIframe);
ioInstance.onViewportCallback_(true);
ioInstance.onViewportCallback_();
expect(fireSpy).to.have.callCount(2);
ioInstance.onViewportCallback_(getInObEntry());
ioInstance.onViewportCallback_({intersectionRatio: false});
expect(sendElementIntersectionSpy).to.have.callCount(2);
expect(ioInstance.unlistenViewportChanges_).to.be.null;
});

Expand All @@ -465,21 +490,25 @@ describes.sandboxed('IntersectionObserverHostForAd', {}, () => {
element,
testIframe
);
const fireInOb = stubFireInOb(ioInstance);
insert(testIframe);
ioInstance.onViewportCallback_(true);
testIframe.contentWindow.postMessage = (message) => {
messages.push(deserializeMessage(message));
};
ioInstance.postMessageApi_.clientWindows_ = [
{win: testIframe.contentWindow, origin: '*'},
];

ioInstance.startSendingIntersectionChanges_();
ioInstance.onViewportCallback_(getInObEntry());
clock.tick(0);
expect(messages).to.have.length(1);
ioInstance.fire();
clock.tick(50);
ioInstance.destroy();
clock.tick(50);
expect(messages).to.have.length(1);
expect(ioInstance.unlistenViewportChanges_).to.be.null;
expect(fireInOb.disconnect).calledOnce;
});
});
6 changes: 1 addition & 5 deletions test/integration/test-amp-ad-3p.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ describe('amp-ad 3P', () => {
lastIO = changes[changes.length - 1];
});
await poll('wait for initial IO entry', () => {
return (
lastIO != null &&
lastIO.boundingClientRect.top == 1000 &&
lastIO.intersectionRatio == 1
);
return lastIO != null && lastIO.boundingClientRect.top == 1000;
});
await new Promise((resolve) => {
setTimeout(resolve, 110);
Expand Down

0 comments on commit ecd7afd

Please sign in to comment.