AMP Video Players: use IntersectionObserver instead of viewportCallba…

…ck + scroll detection. (#30647) * amp-video: remove all traces of viewportCallback * devAssert * devAssert --> toWin
ampproject · Oct 27, 2020 · 467a57a · 467a57a
1 parent 75a06d6
commit 467a57a
Show file tree

Hide file tree

Showing 18 changed files with 40 additions and 136 deletions.
diff --git a/extensions/amp-3q-player/0.1/amp-3q-player.js b/extensions/amp-3q-player/0.1/amp-3q-player.js
@@ -129,11 +129,6 @@ class Amp3QPlayer extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   pauseCallback() {
     if (this.iframe_) {

diff --git a/extensions/amp-brightcove/0.1/amp-brightcove.js b/extensions/amp-brightcove/0.1/amp-brightcove.js
@@ -100,13 +100,6 @@ class AmpBrightcove extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {
-      visible,
-    });
-  }
-
   /** @override */
   buildCallback() {
     this.urlReplacements_ = Services.urlReplacementsForDoc(this.element);

diff --git a/extensions/amp-dailymotion/0.1/amp-dailymotion.js b/extensions/amp-dailymotion/0.1/amp-dailymotion.js
@@ -145,11 +145,6 @@ class AmpDailymotion extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   buildCallback() {
     this.videoid_ = userAssert(

diff --git a/extensions/amp-ima-video/0.1/amp-ima-video.js b/extensions/amp-ima-video/0.1/amp-ima-video.js
@@ -205,11 +205,6 @@ class AmpImaVideo extends AMP.BaseElement {
     });
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   unlayoutCallback() {
     if (this.iframe_) {

diff --git a/extensions/amp-mowplayer/0.1/amp-mowplayer.js b/extensions/amp-mowplayer/0.1/amp-mowplayer.js
@@ -94,11 +94,6 @@ class AmpMowplayer extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   buildCallback() {
     this.mediaid_ = userAssert(

diff --git a/extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js b/extensions/amp-nexxtv-player/0.1/amp-nexxtv-player.js
@@ -136,11 +136,6 @@ class AmpNexxtvPlayer extends AMP.BaseElement {
     return assertAbsoluteHttpOrHttpsUrl(src);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   layoutCallback() {
     const iframe = createFrameFor(this, this.getVideoIframeSrc_());

diff --git a/extensions/amp-ooyala-player/0.1/amp-ooyala-player.js b/extensions/amp-ooyala-player/0.1/amp-ooyala-player.js
@@ -165,11 +165,6 @@ class AmpOoyalaPlayer extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   pauseCallback() {
     // Only send pauseVideo command if the player is playing. Otherwise

diff --git a/extensions/amp-ooyala-player/0.1/test/test-amp-ooyala.js b/extensions/amp-ooyala-player/0.1/test/test-amp-ooyala.js
@@ -40,6 +40,7 @@ describes.realWin(
       opt_placeholder
     ) {
       const player = doc.createElement('amp-ooyala-player');
+      player.setAttribute('layout', 'fixed');
       if (embedCode) {
         player.setAttribute('data-embedcode', embedCode);
       }

diff --git a/extensions/amp-powr-player/0.1/amp-powr-player.js b/extensions/amp-powr-player/0.1/amp-powr-player.js
@@ -98,13 +98,6 @@ class AmpPowrPlayer extends AMP.BaseElement {
     return isLayoutSizeDefined(layout);
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {
-      visible,
-    });
-  }
-
   /** @override */
   buildCallback() {
     this.urlReplacements_ = Services.urlReplacementsForDoc(this.element);

diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js
@@ -294,11 +294,6 @@ class AmpVideo extends AMP.BaseElement {
     // while the video is playing).
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   layoutCallback() {
     this.video_ = dev().assertElement(this.video_);

diff --git a/extensions/amp-wistia-player/0.1/amp-wistia-player.js b/extensions/amp-wistia-player/0.1/amp-wistia-player.js
@@ -143,11 +143,6 @@ class AmpWistiaPlayer extends AMP.BaseElement {
     return true;
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   pauseCallback() {
     if (this.iframe_) {

diff --git a/extensions/amp-youtube/0.1/amp-youtube.js b/extensions/amp-youtube/0.1/amp-youtube.js
@@ -141,11 +141,6 @@ class AmpYoutube extends AMP.BaseElement {
     return 0.75;
   }
 
-  /** @override */
-  viewportCallback(visible) {
-    this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-  }
-
   /** @override */
   buildCallback() {
     this.videoid_ = this.getVideoId_();

diff --git a/extensions/amp-youtube/0.1/test/test-amp-youtube.js b/extensions/amp-youtube/0.1/test/test-amp-youtube.js
@@ -283,7 +283,7 @@ describes.realWin(
       );
     });
 
-    it('loads only sddefault when it exists if source is videoid', async () => {
+    it('loads only default when it exists if source is videoid', async () => {
       const yt = await getYt({'data-videoid': EXAMPLE_VIDEOID}, true, function (
         yt
       ) {

diff --git a/src/service/video-manager-impl.js b/src/service/video-manager-impl.js
@@ -39,6 +39,7 @@ import {VideoSessionManager} from './video-session-manager';
 import {VideoUtils, getInternalVideoElementFor} from '../utils/video';
 import {clamp} from '../utils/math';
 import {createCustomEvent, getData, listen, listenOnce} from '../event-helper';
+import {createViewportObserver} from '../viewport-observer';
 import {dev, devAssert, user, userAssert} from '../log';
 import {dict, map} from '../utils/object';
 import {getMode} from '../mode';
@@ -81,22 +82,19 @@ export class VideoManager {
       installAutoplayStylesForDoc(this.ampdoc)
     );
 
-    /** @private {!../service/viewport/viewport-interface.ViewportInterface} */
-    this.viewport_ = Services.viewportForDoc(this.ampdoc);
-
     /** @private {?Array<!VideoEntry>} */
     this.entries_ = null;
 
+    /** @private {IntersectionObserver} */
+    this.viewportObserver_ = null;
+
     /**
      * Keeps last found entry as a small optimization for multiple state calls
      * during one task.
      * @private {?VideoEntry}
      */
     this.lastFoundEntry_ = null;
 
-    /** @private {boolean} */
-    this.scrollListenerInstalled_ = false;
-
     /** @private @const */
     this.timer_ = Services.timerFor(ampdoc.win);
 
@@ -124,6 +122,8 @@ export class VideoManager {
   /** @override */
   dispose() {
     this.getAutoFullscreenManager_().dispose();
+    this.viewportObserver_.disconnect();
+    this.viewportObserver_ = null;
 
     if (!this.entries_) {
       return;
@@ -176,19 +176,37 @@ export class VideoManager {
     }
   }
 
+  // TODO(#30723): create unregister() for cleanup.
   /** @param {!../video-interface.VideoInterface} video */
   register(video) {
     devAssert(video);
+    const videoBE = /** @type {!AMP.BaseElement} */ (video);
 
     this.registerCommonActions_(video);
 
     if (!video.supportsPlatform()) {
       return;
     }
+    if (!this.viewportObserver_) {
+      const viewportCallback = (
+        /** @type {!Array<!IntersectionObserverEntry>} */ records
+      ) =>
+        records.forEach(({target, isIntersecting}) => {
+          this.getEntry_(target).updateVisibility(
+            /* isVisible */ isIntersecting
+          );
+        });
+      this.viewportObserver_ = createViewportObserver(
+        viewportCallback,
+        this.ampdoc.win,
+        /* threshold */ MIN_VISIBILITY_RATIO_FOR_AUTOPLAY
+      );
+    }
+    this.viewportObserver_.observe(videoBE.element);
+    listen(videoBE.element, VideoEvents.RELOAD, () => entry.videoLoaded());
 
     this.entries_ = this.entries_ || [];
     const entry = new VideoEntry(this, video);
-    this.maybeInstallVisibilityObserver_(entry);
     this.entries_.push(entry);
 
     const {element} = entry.video;
@@ -249,45 +267,6 @@ export class VideoManager {
     }
   }
 
-  /**
-   * Install the necessary listeners to be notified when a video becomes visible
-   * in the viewport.
-   *
-   * Visibility of a video is defined by being in the viewport AND having
-   * {@link MIN_VISIBILITY_RATIO_FOR_AUTOPLAY} of the video element visible.
-   *
-   * @param {VideoEntry} entry
-   * @private
-   */
-  maybeInstallVisibilityObserver_(entry) {
-    const {element} = entry.video;
-
-    listen(element, VideoEvents.VISIBILITY, (details) => {
-      const data = getData(details);
-      if (data && data['visible'] == true) {
-        entry.updateVisibility(/* opt_forceVisible */ true);
-      } else {
-        entry.updateVisibility();
-      }
-    });
-
-    listen(element, VideoEvents.RELOAD, () => {
-      entry.videoLoaded();
-    });
-
-    // TODO(aghassemi, #6425): Use IntersectionObserver
-    if (!this.scrollListenerInstalled_) {
-      const scrollListener = () => {
-        for (let i = 0; i < this.entries_.length; i++) {
-          this.entries_[i].updateVisibility();
-        }
-      };
-      this.viewport_.onScroll(scrollListener);
-      this.viewport_.onChanged(scrollListener);
-      this.scrollListenerInstalled_ = true;
-    }
-  }
-
   /**
    * Returns the entry in the video manager corresponding to the video or
    * element provided
@@ -622,7 +601,6 @@ class VideoEntry {
       this.manager_.registerForAutoFullscreen(this);
     }
 
-    this.updateVisibility();
     if (this.hasAutoplay) {
       this.autoplayVideoBuilt_();
     }
@@ -719,7 +697,6 @@ class VideoEntry {
 
     this.getAnalyticsPercentageTracker_().start();
 
-    this.updateVisibility();
     if (this.isVisible_) {
       // Handles the case when the video becomes visible before loading
       this.loadedVideoVisibilityChanged_();
@@ -948,25 +925,14 @@ class VideoEntry {
   }
 
   /**
-   * Called by all possible events that might change the visibility of the video
-   * such as scrolling or {@link ../video-interface.VideoEvents#VISIBILITY}.
-   * @param {?boolean=} opt_forceVisible
+   * Called by an IntersectionObserver.
+   * @param {boolean} isVisible
    * @package
    */
-  updateVisibility(opt_forceVisible) {
+  updateVisibility(isVisible) {
     const wasVisible = this.isVisible_;
-
-    if (opt_forceVisible) {
-      this.isVisible_ = true;
-    } else {
-      const {element} = this.video;
-      const ratio = element.getIntersectionChangeEntry().intersectionRatio;
-      this.isVisible_ =
-        (!isFiniteNumber(ratio) ? 0 : ratio) >=
-        MIN_VISIBILITY_RATIO_FOR_AUTOPLAY;
-    }
-
-    if (this.isVisible_ != wasVisible) {
+    this.isVisible_ = isVisible;
+    if (isVisible != wasVisible) {
       this.videoVisibilityChanged_();
     }
   }

diff --git a/src/viewport-observer.js b/src/viewport-observer.js
@@ -16,6 +16,7 @@
 import {devAssert} from '../src/log';
 import {getMode} from './mode';
 import {isIframed} from './dom';
+import {toWin} from '../src/types';
 
 /**
  * Returns an IntersectionObserver tracking the Viewport.
@@ -76,7 +77,7 @@ export function observeWithSharedInOb(element, viewportCallback) {
     );
   }
 
-  const win = element.ownerDocument.defaultView;
+  const win = toWin(element.ownerDocument.defaultView);
   let viewportObserver = viewportObservers.get(win);
   if (!viewportObserver) {
     viewportObservers.set(
@@ -93,7 +94,7 @@ export function observeWithSharedInOb(element, viewportCallback) {
  * @param {!Element} element
  */
 export function unobserveWithSharedInOb(element) {
-  const win = element.ownerDocument.defaultView;
+  const win = toWin(element.ownerDocument.defaultView);
   const viewportObserver = viewportObservers.get(win);
   if (viewportObserver) {
     viewportObserver.unobserve(element);

diff --git a/test/integration/test-video-manager.js b/test/integration/test-video-manager.js
@@ -301,9 +301,7 @@ describe
         },
       };
 
-      win = {
-        document: doc,
-      };
+      win = {document: doc};
 
       isLite = false;
 
@@ -439,11 +437,6 @@ function createFakeVideoPlayerClass(win) {
       });
     }
 
-    /** @override */
-    viewportCallback(visible) {
-      this.element.dispatchCustomEvent(VideoEvents.VISIBILITY, {visible});
-    }
-
     // VideoInterface Implementation. See ../src/video-interface.VideoInterface
 
     /**

diff --git a/test/integration/test-video-players-helper.js b/test/integration/test-video-players-helper.js
@@ -382,7 +382,7 @@ export function runVideoPlayerIntegrationTests(
         );
       });
 
-      it('should not play when not in view port initially', () => {
+      it('should not play when initially outside viewport', () => {
         return getVideoPlayer({outsideView: true, autoplay: true}).then((r) => {
           const timer = Services.timerFor(r.video.implementation_.win);
           const p = listenOncePromise(r.video, VideoEvents.PLAYING).then(() => {
@@ -551,7 +551,7 @@ export function runVideoPlayerIntegrationTests(
 
   function getVideoPlayer(options) {
     options = options || {};
-    const top = options.outsideView ? '100vh' : '0';
+    const top = options.outsideView ? '126vh' : '0';
     let fixture;
     return createFixtureIframe('test/fixtures/video-players.html', FRAME_HEIGHT)
       .then((f) => {