Remove getPageLayoutBox, getLayoutBox, and getIntersectionChangeEntry…

… APIs from docking (#31749) * Remove getPageLayoutBox, getLayoutBox, and getIntersectionChangeEntry APIs from docking * lints
ampproject · Dec 28, 2020 · 46204c0 · 46204c0
1 parent 21b013e
commit 46204c0
Show file tree

Hide file tree

Showing 4 changed files with 119 additions and 76 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -1205,7 +1205,6 @@ const forbiddenTermsSrcInclusive = {
       'src/base-element.js',
       'src/custom-element.js',
       'src/service/resource.js',
-      'extensions/amp-video-docking/0.1/amp-video-docking.js',
     ],
   },
   '\\.getIntersectionElementLayoutBox': {

diff --git a/extensions/amp-video-docking/0.1/amp-video-docking.js b/extensions/amp-video-docking/0.1/amp-video-docking.js
@@ -21,7 +21,8 @@ import {HtmlLiteralTagDef} from './html';
 import {
   LayoutRectDef,
   layoutRectEquals,
-  moveLayoutRect,
+  layoutRectFromDomRect,
+  rectIntersection,
 } from '../../../src/layout-rect';
 import {
   PlayingStates,
@@ -140,6 +141,7 @@ let DockedDef;
  * @struct @typedef {{
  *   type: DockTargetType,
  *   rect: !LayoutRectDef,
+ *   slot: (!Element|undefined),
  * }}
  */
 let DockTargetDef;
@@ -450,7 +452,7 @@ export class VideoDocking {
     const slot = this.getSlot_();
     if (slot) {
       // Match slot's top edge to tie transition to element.
-      return slot.getPageLayoutBox().top;
+      return slot./*OK*/ getBoundingClientRect().top;
     }
     return 0;
   }
@@ -461,8 +463,8 @@ export class VideoDocking {
 
     const {element} = video;
     const fidelity = PositionObserverFidelity.HIGH;
-    this.getPositionObserver_().observe(element, fidelity, () =>
-      this.updateOnPositionChange_(video)
+    this.getPositionObserver_().observe(element, fidelity, ({positionRect}) =>
+      this.updateOnPositionChange_(video, positionRect)
     );
     this.observed_.push(video);
   }
@@ -594,8 +596,9 @@ export class VideoDocking {
     }
 
     const {element} = video;
-    const {intersectionRect} = element.getIntersectionChangeEntry();
-    if (!isSizedRect(intersectionRect)) {
+    const clientRect = element./*OK*/ getBoundingClientRect();
+    const intersectionRect = rectIntersection(clientRect, this.viewportRect_);
+    if (!intersectionRect || !isSizedRect(intersectionRect)) {
       return null;
     }
     if (intersectionRect.top > this.getTopBoundary_()) {
@@ -610,8 +613,8 @@ export class VideoDocking {
    * @private
    */
   getScrollAdjustedRect_(element) {
-    const dy = -this.viewport_.getScrollTop();
-    return moveLayoutRect(element.getLayoutBox(), /* dx */ 0, dy);
+    element = element.element || element;
+    return layoutRectFromDomRect(element./*OK*/ getBoundingClientRect());
   }
 
   /**
@@ -643,16 +646,17 @@ export class VideoDocking {
 
   /**
    * @param {!VideoOrBaseElementDef} video
+   * @param {!LayoutRectDef=} opt_clientRect
    * @private
    */
-  updateOnPositionChange_(video) {
+  updateOnPositionChange_(video, opt_clientRect) {
     if (this.isTransitioning_) {
       return;
     }
     if (this.scrollDirection_ == DirectionY.TOP) {
       const target = this.getTargetFor_(video);
       if (target) {
-        this.dockOnPositionChange_(video, target);
+        this.dockOnPositionChange_(video, target, opt_clientRect);
       }
     } else if (this.scrollDirection_ == DirectionY.BOTTOM) {
       if (!this.currentlyDocked_) {
@@ -687,7 +691,7 @@ export class VideoDocking {
    * @private
    */
   isValidSize_(video) {
-    const {width, height} = video.getLayoutBox();
+    const {width, height} = video.element./*OK*/ getBoundingClientRect();
     if (width / height < 1 - FLOAT_TOLERANCE) {
       complainAboutPortrait(video.element);
       return false;
@@ -714,9 +718,10 @@ export class VideoDocking {
   /**
    * @param {!VideoOrBaseElementDef} video
    * @param {!DockTargetDef} target
+   * @param {!LayoutRectDef=} opt_clientRect
    * @private
    */
-  dockOnPositionChange_(video, target) {
+  dockOnPositionChange_(video, target, opt_clientRect) {
     if (this.ignoreDueToDismissal_(video)) {
       return;
     }
@@ -725,43 +730,46 @@ export class VideoDocking {
       return;
     }
 
-    const currentlyDocked = this.currentlyDocked_;
-
-    if (
-      currentlyDocked &&
-      currentlyDocked.step >= 0 &&
-      layoutRectEquals(currentlyDocked.target.rect, target.rect)
-    ) {
-      return;
-    }
-
-    this.dockInTransferLayerStep_(video, target);
+    this.dockInTransferLayerStep_(
+      video,
+      target,
+      /* step */ 0.1,
+      opt_clientRect
+    );
   }
 
   /**
    * @param {!VideoOrBaseElementDef} video
    * @param {!DockTargetDef} target
-   * @param {number=} opt_step
+   * @param {number} step
+   * @param {!LayoutRectDef=} opt_clientRect
    * @return {!Promise}
    * @private
    */
-  dockInTransferLayerStep_(video, target, opt_step) {
+  dockInTransferLayerStep_(video, target, step, opt_clientRect) {
     // Do this in a multi-step process due to a browser quirk in transferring
     // layers to GPU.
-    const step = opt_step || 0.1;
     // This cutoff is arbitrary and may be dependant on performance.
-    // TODO(alanorozco): Investigate.
     if (step > 0.3) {
-      return this.dock_(video, target, /* step */ 1);
+      return this.dock_(video, target, /* step */ 1, opt_clientRect);
     }
     const isTransferLayerStep = true;
-    return this.dock_(video, target, step, isTransferLayerStep).then(
+    return this.dock_(
+      video,
+      target,
+      step,
+      opt_clientRect,
+      isTransferLayerStep
+    ).then(
       () =>
         new Promise((resolve) => {
           this.raf_(() => {
-            this.dockInTransferLayerStep_(video, target, step + 0.1).then(
-              resolve
-            );
+            this.dockInTransferLayerStep_(
+              video,
+              target,
+              step + 0.1,
+              opt_clientRect
+            ).then(resolve);
           });
         })
     );
@@ -771,11 +779,12 @@ export class VideoDocking {
    * @param {!VideoOrBaseElementDef} video
    * @param {!DockTargetDef} target
    * @param {number} step
+   * @param {!LayoutRectDef=} opt_clientRect
    * @param {boolean=} opt_isTransferLayerStep
    * @return {!Promise}
    * @private
    */
-  dock_(video, target, step, opt_isTransferLayerStep) {
+  dock_(video, target, step, opt_clientRect, opt_isTransferLayerStep) {
     const {element} = video;
 
     // Component background is now visible, so hide the poster for the Android
@@ -784,7 +793,12 @@ export class VideoDocking {
     // TODO(alanorozco): Make docking agnostic to this workaround.
     this.removePosterForAndroidBug_(element);
 
-    const {x, y, scale, relativeX} = this.getDims_(video, target, step);
+    const {x, y, scale, relativeX} = this.getDims_(
+      video,
+      target,
+      step,
+      opt_clientRect
+    );
 
     video.hideControls();
 
@@ -799,7 +813,8 @@ export class VideoDocking {
       scale,
       step,
       transitionDurationMs,
-      relativeX
+      relativeX,
+      opt_clientRect
     ).then(() => {
       if (opt_isTransferLayerStep) {
         // Do not enable controls during transfer layer steps, which would
@@ -812,11 +827,14 @@ export class VideoDocking {
 
   /**
    * @param {!Actions} action
+   * @param {!DockTargetDef=} opt_target
    * @private
    */
-  trigger_(action) {
+  trigger_(action, opt_target) {
     const element = dev().assertElement(
-      this.getSlot_() || this.getDockedVideo_().element
+      (opt_target && opt_target.slot) ||
+        this.getSlot_() ||
+        this.getDockedVideo_().element
     );
 
     const trust = ActionTrust.LOW;
@@ -908,24 +926,34 @@ export class VideoDocking {
    * @param {number} step in [0..1]
    * @param {number} transitionDurationMs
    * @param {DirectionX=} opt_relativeX
+   * @param {!LayoutRectDef=} opt_clientRect
    * @return {!Promise}
    * @private
    */
-  placeAt_(video, x, y, scale, step, transitionDurationMs, opt_relativeX) {
+  placeAt_(
+    video,
+    x,
+    y,
+    scale,
+    step,
+    transitionDurationMs,
+    opt_relativeX,
+    opt_clientRect
+  ) {
     if (this.alreadyPlacedAt_(x, y, scale)) {
       return Promise.resolve();
     }
 
     this.isTransitioning_ = true;
 
-    const {width, height} = video.getLayoutBox();
+    const {element} = video;
+    const {width, height} =
+      opt_clientRect || video.element./*OK*/ getBoundingClientRect();
 
     this.placedAt_ = {x, y, scale};
 
     const transitionTiming = step > 0 ? 'ease-out' : 'ease-in';
 
-    const {element} = video;
-
     const internalElement = getInternalVideoElementFor(element);
     const shadowLayer = this.getShadowLayer_();
     const {overlay} = this.getControls_();
@@ -1171,7 +1199,7 @@ export class VideoDocking {
       return;
     }
     this.getControls_().setVideo(video, target.rect);
-    this.trigger_(Actions.DOCK);
+    this.trigger_(Actions.DOCK, target);
   }
 
   /**
@@ -1472,7 +1500,7 @@ export class VideoDocking {
   getCenterX_(offsetX) {
     const {target, step} = this.currentlyDocked_;
     const video = this.getDockedVideo_();
-    const {width} = video.getLayoutBox();
+    const {width} = video.element./*OK*/ getBoundingClientRect();
     const {x, scale} = this.getDims_(video, target, step);
     return x + offsetX + (width * scale) / 2;
   }
@@ -1520,11 +1548,12 @@ export class VideoDocking {
    * @param {!VideoOrBaseElementDef} video
    * @param {!DockTargetDef} target
    * @param {number} step in [0..1]
+   * @param {!LayoutRectDef} opt_clientRect
    * @return {{x: number, y: number, scale: number, relativeX: !DirectionX}}
    */
-  getDims_(video, target, step) {
+  getDims_(video, target, step, opt_clientRect) {
     return interpolatedBoxesTransform(
-      this.getScrollAdjustedRect_(video),
+      opt_clientRect || this.getScrollAdjustedRect_(video),
       target.rect,
       step
     );
@@ -1672,12 +1701,18 @@ export class VideoDocking {
    */
   getUsableTarget_(video) {
     const slot = this.getSlot_();
-    const inlineRect = video.getLayoutBox();
+    const inlineRect = layoutRectFromDomRect(
+      video.element./*OK*/ getBoundingClientRect()
+    );
 
     if (slot) {
       return {
         type: DockTargetType.SLOT,
-        rect: letterboxRect(inlineRect, slot.getPageLayoutBox()),
+        rect: letterboxRect(
+          inlineRect,
+          layoutRectFromDomRect(slot./*OK*/ getBoundingClientRect())
+        ),
+        slot,
       };
     }
 

diff --git a/extensions/amp-video-docking/0.1/math.js b/extensions/amp-video-docking/0.1/math.js
@@ -162,7 +162,8 @@ export function topCornerRect(
  * @param {!AmpElement} element
  * @return {boolean}
  */
-export const isVisibleBySize = (element) => isSizedRect(element.getLayoutBox());
+export const isVisibleBySize = (element) =>
+  isSizedRect(element./*OK*/ getBoundingClientRect());
 
 /**
  * @param {!LayoutRectDef|!ClientRect|!DOMRect} rect