Skip to content

Commit

Permalink
measureIntersection: use root=document (#32402)
Browse files Browse the repository at this point in the history
* measureIntersection: track document in iframe

* test + lint

* fix threshold usage

* make closure types happier

* wip -- come back and fix

* Caculate expected rootBounds differently
  • Loading branch information
samouri committed Feb 10, 2021
1 parent daa95ee commit 0ab0609
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/service/video-manager-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ export class VideoManager {
this.viewportObserver_ = createViewportObserver(
viewportCallback,
this.ampdoc.win,
/* threshold */ MIN_VISIBILITY_RATIO_FOR_AUTOPLAY
{threshold: MIN_VISIBILITY_RATIO_FOR_AUTOPLAY}
);
}
this.viewportObserver_.observe(videoBE.element);
Expand Down
30 changes: 17 additions & 13 deletions src/utils/intersection.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,24 @@ function getInOb(win) {
}

if (!intersectionObservers.has(win)) {
const observer = createViewportObserver((entries) => {
const seen = new Set();
for (let i = entries.length - 1; i >= 0; i--) {
const {target} = entries[i];
if (seen.has(target)) {
continue;
}
seen.add(target);
const observer = createViewportObserver(
(entries) => {
const seen = new Set();
for (let i = entries.length - 1; i >= 0; i--) {
const {target} = entries[i];
if (seen.has(target)) {
continue;
}
seen.add(target);

observer.unobserve(target);
intersectionDeferreds.get(target).resolve(entries[i]);
intersectionDeferreds.delete(target);
}
}, win);
observer.unobserve(target);
intersectionDeferreds.get(target).resolve(entries[i]);
intersectionDeferreds.delete(target);
}
},
win,
{needsRootBounds: true}
);
intersectionObservers.set(win, observer);
return observer;
}
Expand Down
11 changes: 8 additions & 3 deletions src/viewport-observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,24 @@
*/
import {devAssert} from '../src/log';
import {getMode} from './mode';
import {isIframed} from './dom';
import {toWin} from './types';

/**
* Returns an IntersectionObserver tracking the Viewport.
*
* @param {function(!Array<!IntersectionObserverEntry>)} ioCallback
* @param {!Window} win
* @param {(number|!Array<number>)=} threshold
* @param {{threshold: (number|!Array<number>)=, needsRootBounds: boolean=}=} opts
*
* @return {!IntersectionObserver}
*/
export function createViewportObserver(ioCallback, win, threshold) {
return new win.IntersectionObserver(ioCallback, {threshold});
export function createViewportObserver(ioCallback, win, opts = {}) {
const {threshold, needsRootBounds} = opts;
return new win.IntersectionObserver(ioCallback, {
threshold,
root: isIframed(win) && needsRootBounds ? win.document : undefined,
});
}

/** @type {!WeakMap<!Window, !IntersectionObserver>} */
Expand Down
20 changes: 18 additions & 2 deletions test/integration/test-amp-ad-3p.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ import {installPlatformService} from '../../src/service/platform-impl';
import {layoutRectLtwh} from '../../src/layout-rect';
import {toggleExperiment} from '../../src/experiments';

const IFRAME_HEIGHT = 3000;
function createFixture() {
return createFixtureIframe('test/fixtures/3p-ad.html', 3000, () => {});
return createFixtureIframe(
'test/fixtures/3p-ad.html',
IFRAME_HEIGHT,
() => {}
);
}

describe('amp-ad 3P', () => {
Expand Down Expand Up @@ -97,7 +102,18 @@ describe('amp-ad 3P', () => {
});
const {initialIntersection} = context;
expect(initialIntersection.rootBounds).to.deep.equal(
layoutRectLtwh(0, 0, window.innerWidth, window.innerHeight)
layoutRectLtwh(
0,
0,
Math.min(
iframe.ownerDocument.body.clientWidth,
iframe.ownerDocument.defaultView.innerWidth
),
Math.min(
iframe.ownerDocument.body.clientHeight,
iframe.ownerDocument.defaultView.innerHeight
)
)
);

expect(initialIntersection.boundingClientRect).to.deep.equal(
Expand Down
20 changes: 17 additions & 3 deletions test/unit/test-viewport-observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,26 @@ describes.sandboxed('Viewport Observer', {}, (env) => {

it('Uses implicit root.', () => {
createViewportObserver(noop, win);
expect(ctorSpy).calledWith(noop, {threshold: undefined});
expect(ctorSpy).calledWith(noop, {threshold: undefined, root: undefined});
});

it('Pass along threshold argument', () => {
createViewportObserver(noop, win, 0.5);
expect(ctorSpy).calledWith(noop, {threshold: 0.5});
createViewportObserver(noop, win, {threshold: 0.5});
expect(ctorSpy).calledWith(noop, {threshold: 0.5, root: undefined});
});

it('Sets document root appropriately', () => {
// Implicit root when not iframed.
createViewportObserver(noop, win, {needsRootBounds: true});
expect(ctorSpy).calledWith(noop, {threshold: undefined, root: undefined});

// Document root when iframed.
win.parent = {};
createViewportObserver(noop, win, {needsRootBounds: true});
expect(ctorSpy).calledWith(noop, {
threshold: undefined,
root: win.document,
});
});
});

Expand Down

0 comments on commit 0ab0609

Please sign in to comment.