From 28b4911a95b2676f00b3ee6a3f262d90def739bb Mon Sep 17 00:00:00 2001 From: Jake Fried Date: Wed, 23 Dec 2020 11:44:43 -0500 Subject: [PATCH 1/4] amp-video-iframe: use InOb instead of ce.getIntersectionChangeEntry. --- .../amp-video-iframe/0.1/amp-video-iframe.js | 17 ++++++++++++++++- .../0.1/test/test-amp-video-iframe.js | 12 +++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js index 96ab3533f3c0..24bd84793e45 100644 --- a/extensions/amp-video-iframe/0.1/amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/amp-video-iframe.js @@ -360,12 +360,27 @@ class AmpVideoIframe extends AMP.BaseElement { } /** + * Creates an IntersectionObserver to fire a single intersection event. + * * @param {number} messageId * @private */ postIntersection_(messageId) { - const {time, intersectionRatio} = this.element.getIntersectionChangeEntry(); + const intersectionObserver = new IntersectionObserver((entries) => { + this.intersectionCallback_(messageId, entries); + intersectionObserver.disconnect(); + }); + intersectionObserver.observe(this.element); + } + /** + * @param {number} messageId + * @param {Array} entries + * @private + */ + intersectionCallback_(messageId, entries) { + const lastEntry = entries[entries.length - 1]; + const {time, intersectionRatio} = lastEntry; // Only post ratio > 0 when in autoplay range to prevent internal autoplay // implementations that differ from ours. const postedRatio = diff --git a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js index ef6c63c466dc..f9371a03549a 100644 --- a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js @@ -106,11 +106,13 @@ describes.realWin( ); } - function stubIntersectionEntry(element, time, intersectionRatio) { + function stubPostIntersection(videoIframe, time, intersectionRatio) { const entry = {time, intersectionRatio}; env.sandbox - ./*OK*/ stub(element, 'getIntersectionChangeEntry') - .returns(entry); + .stub(videoIframe.implementation_, 'postIntersection_') + .callsFake((id) => { + videoIframe.implementation_.intersectionCallback_(id, [entry]); + }); return entry; } @@ -290,7 +292,7 @@ describes.realWin( const expectedResponseMessage = { id, - args: stubIntersectionEntry(videoIframe, time, intersectionRatio), + args: stubPostIntersection(videoIframe, time, intersectionRatio), }; videoIframe.implementation_.onMessage_(message); @@ -311,7 +313,7 @@ describes.realWin( const postMessage = stubPostMessage(videoIframe); - stubIntersectionEntry(videoIframe, time, intersectionRatio); + stubPostIntersection(videoIframe, time, intersectionRatio); acceptMockedMessages(videoIframe); From f715271cd7e23a721885a35775ea6b5ac1b9393a Mon Sep 17 00:00:00 2001 From: Jake Fried Date: Fri, 8 Jan 2021 16:35:29 -0500 Subject: [PATCH 2/4] update to use new helper --- .../amp-video-iframe/0.1/amp-video-iframe.js | 33 ++++++++----------- .../0.1/test/test-amp-video-iframe.js | 16 ++++----- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js index 24bd84793e45..cd92ff93552d 100644 --- a/extensions/amp-video-iframe/0.1/amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/amp-video-iframe.js @@ -43,6 +43,7 @@ import { import {getData, listen} from '../../../src/event-helper'; import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl'; import {isLayoutSizeDefined} from '../../../src/layout'; +import {measureIntersection} from '../../../src/utils/intersection'; import {once} from '../../../src/utils/function'; /** @private @const */ @@ -126,6 +127,12 @@ class AmpVideoIframe extends AMP.BaseElement { * @private */ this.boundOnMessage_ = (e) => this.onMessage_(e); + + /** + * @param {!Element} element + * @return {!Promise} element + */ + this.measureIntersection = (element) => measureIntersection(element); } /** @override */ @@ -275,6 +282,7 @@ class AmpVideoIframe extends AMP.BaseElement { /** * @param {!Event} event + * @returns {Promise} * @private */ onMessage_(event) { @@ -302,8 +310,9 @@ class AmpVideoIframe extends AMP.BaseElement { if (methodReceived) { if (methodReceived == 'getIntersection') { - this.postIntersection_(messageId); - return; + return this.measureIntersection(this.element).then((intersection) => { + this.postIntersection_(messageId, intersection); + }); } userAssert(false, 'Unknown method `%s`.', methodReceived); return; @@ -360,27 +369,13 @@ class AmpVideoIframe extends AMP.BaseElement { } /** - * Creates an IntersectionObserver to fire a single intersection event. - * * @param {number} messageId + * @param {!IntersectionObserverEntry} intersection * @private */ - postIntersection_(messageId) { - const intersectionObserver = new IntersectionObserver((entries) => { - this.intersectionCallback_(messageId, entries); - intersectionObserver.disconnect(); - }); - intersectionObserver.observe(this.element); - } + postIntersection_(messageId, intersection) { + const {intersectionRatio, time} = intersection; - /** - * @param {number} messageId - * @param {Array} entries - * @private - */ - intersectionCallback_(messageId, entries) { - const lastEntry = entries[entries.length - 1]; - const {time, intersectionRatio} = lastEntry; // Only post ratio > 0 when in autoplay range to prevent internal autoplay // implementations that differ from ours. const postedRatio = diff --git a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js index f9371a03549a..4aeb78fb429f 100644 --- a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js @@ -106,13 +106,11 @@ describes.realWin( ); } - function stubPostIntersection(videoIframe, time, intersectionRatio) { + function stubMeasureIntersection(videoIframe, time, intersectionRatio) { const entry = {time, intersectionRatio}; env.sandbox - .stub(videoIframe.implementation_, 'postIntersection_') - .callsFake((id) => { - videoIframe.implementation_.intersectionCallback_(id, [entry]); - }); + .stub(videoIframe.implementation_, 'measureIntersection') + .returns(Promise.resolve(entry)); return entry; } @@ -292,10 +290,10 @@ describes.realWin( const expectedResponseMessage = { id, - args: stubPostIntersection(videoIframe, time, intersectionRatio), + args: stubMeasureIntersection(videoIframe, time, intersectionRatio), }; - videoIframe.implementation_.onMessage_(message); + await videoIframe.implementation_.onMessage_(message); expect(postMessage.withArgs(env.sandbox.match(expectedResponseMessage))) .to.have.been.calledOnce; @@ -313,7 +311,7 @@ describes.realWin( const postMessage = stubPostMessage(videoIframe); - stubPostIntersection(videoIframe, time, intersectionRatio); + stubMeasureIntersection(videoIframe, time, intersectionRatio); acceptMockedMessages(videoIframe); @@ -327,7 +325,7 @@ describes.realWin( }, }; - videoIframe.implementation_.onMessage_(message); + await videoIframe.implementation_.onMessage_(message); expect(postMessage.withArgs(env.sandbox.match(expectedResponseMessage))) .to.have.been.calledOnce; From 2141301263506948ed22f1b0d9679d88beaf1864 Mon Sep 17 00:00:00 2001 From: Jake Fried Date: Fri, 8 Jan 2021 17:45:03 -0500 Subject: [PATCH 3/4] lint --- extensions/amp-video-iframe/0.1/amp-video-iframe.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js index cd92ff93552d..4d8b19d584cf 100644 --- a/extensions/amp-video-iframe/0.1/amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/amp-video-iframe.js @@ -282,7 +282,7 @@ class AmpVideoIframe extends AMP.BaseElement { /** * @param {!Event} event - * @returns {Promise} + * @return {Promise} * @private */ onMessage_(event) { From 2dbf3fb16111569f7215b442620512a676334b7c Mon Sep 17 00:00:00 2001 From: Jake Fried Date: Tue, 12 Jan 2021 12:59:11 -0500 Subject: [PATCH 4/4] address Dima comment --- .../amp-video-iframe/0.1/amp-video-iframe.js | 10 ++------- .../0.1/test/test-amp-video-iframe.js | 22 ++++++++++--------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/extensions/amp-video-iframe/0.1/amp-video-iframe.js b/extensions/amp-video-iframe/0.1/amp-video-iframe.js index 4d8b19d584cf..eac329a4afe9 100644 --- a/extensions/amp-video-iframe/0.1/amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/amp-video-iframe.js @@ -127,12 +127,6 @@ class AmpVideoIframe extends AMP.BaseElement { * @private */ this.boundOnMessage_ = (e) => this.onMessage_(e); - - /** - * @param {!Element} element - * @return {!Promise} element - */ - this.measureIntersection = (element) => measureIntersection(element); } /** @override */ @@ -282,7 +276,7 @@ class AmpVideoIframe extends AMP.BaseElement { /** * @param {!Event} event - * @return {Promise} + * @return {!Promise|undefined} * @private */ onMessage_(event) { @@ -310,7 +304,7 @@ class AmpVideoIframe extends AMP.BaseElement { if (methodReceived) { if (methodReceived == 'getIntersection') { - return this.measureIntersection(this.element).then((intersection) => { + return measureIntersection(this.element).then((intersection) => { this.postIntersection_(messageId, intersection); }); } diff --git a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js index 4aeb78fb429f..324852a0ae98 100644 --- a/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js +++ b/extensions/amp-video-iframe/0.1/test/test-amp-video-iframe.js @@ -106,12 +106,16 @@ describes.realWin( ); } - function stubMeasureIntersection(videoIframe, time, intersectionRatio) { - const entry = {time, intersectionRatio}; - env.sandbox - .stub(videoIframe.implementation_, 'measureIntersection') - .returns(Promise.resolve(entry)); - return entry; + function stubMeasureIntersection(target, time, intersectionRatio) { + env.win.IntersectionObserver = (callback) => ({ + observe() { + Promise.resolve().then(() => { + callback([{target, time, intersectionRatio}]); + }); + }, + unobserve() {}, + disconnect() {}, + }); } describe('#layoutCallback', () => { @@ -288,10 +292,8 @@ describes.realWin( const message = getIntersectionMessage(id); - const expectedResponseMessage = { - id, - args: stubMeasureIntersection(videoIframe, time, intersectionRatio), - }; + stubMeasureIntersection(videoIframe, time, intersectionRatio); + const expectedResponseMessage = {id, args: {time, intersectionRatio}}; await videoIframe.implementation_.onMessage_(message);