🐛 Update contentHeight algorithms (ampproject#20035)

* Update contentHeight algorithms

* Use getBoundingClientRect.height

* Fix tests

* comment

* Fix presubmit errors

* Fix tests

src/service/viewport/viewport-binding-ios-embed-sd.js

@@ -396,15 +396,17 @@ export class ViewportBindingIosEmbedShadowRoot_ {
 
   /** @override */
   getContentHeight() {
-    // The reparented body inside scroller will have the correct content height.
-    // Body is overflow: hidden so that the scrollHeight include the margins of
-    // body's first and last child.
-    // Body height doesn't include paddingTop on the parent, so we add on the
-    // position of the body from the top of the viewport and subtract the
-    // scrollTop (as position relative to the viewport changes as you scroll).
-    return this.wrapper_./*OK*/scrollHeight
+    // Don't use scrollHeight, since it returns `MAX(viewport_height,
+    // document_height)`, even though we only want the latter. Also, it doesn't
+    // account for margins
+    const scrollingElement = this.wrapper_;
+    const rect = scrollingElement./*OK*/getBoundingClientRect();
+    const style = computedStyle(this.win, scrollingElement);
+    return rect.height
         + this.paddingTop_
-        + this.getBorderTop();
+        + this.getBorderTop()
+        + parseInt(style.marginTop, 10)
+        + parseInt(style.marginBottom, 10);
   }
 
   /** @override */

src/service/viewport/viewport-binding-ios-embed-wrapper.js

@@ -17,10 +17,10 @@
 import {Observable} from '../../observable';
 import {Services} from '../../services';
 import {ViewportBindingDef} from './viewport-binding-def';
+import {computedStyle, px, setImportantStyles} from '../../style';
 import {dev} from '../../log';
 import {isExperimentOn} from '../../experiments';
 import {layoutRectLtwh} from '../../layout-rect';
-import {px, setImportantStyles} from '../../style';
 import {waitForBody} from '../../dom';
 import {whenDocumentReady} from '../../document-ready';
 
@@ -76,6 +76,9 @@ export class ViewportBindingIosEmbedWrapper_ {
     /** @private @const {boolean} */
     this.useLayers_ = isExperimentOn(this.win, 'layers');
 
+    /** @private {number} */
+    this.paddingTop_ = 0;
+
     // Setup UI.
     /** @private {boolean} */
     this.setupDone_ = false;
@@ -165,6 +168,7 @@ export class ViewportBindingIosEmbedWrapper_ {
 
   /** @override */
   updatePaddingTop(paddingTop) {
+    this.paddingTop_ = paddingTop;
     setImportantStyles(this.wrapper_, {
       'padding-top': px(paddingTop),
     });
@@ -234,14 +238,16 @@ export class ViewportBindingIosEmbedWrapper_ {
 
   /** @override */
   getContentHeight() {
-    // The reparented body inside wrapper will have the correct content height.
-    // Body is overflow: hidden so that the scrollHeight include the margins of
-    // body's first and last child.
-    // Body height doesn't include paddingTop on the parent, so we add on the
-    // position of the body from the top of the viewport and subtract the
-    // scrollTop (as position relative to the viewport changes as you scroll).
-    const rect = this.win.document.body./*OK*/getBoundingClientRect();
-    return rect.height + rect.top + this.getScrollTop();
+    // Don't use scrollHeight, since it returns `MAX(viewport_height,
+    // document_height)`, even though we only want the latter. Also, it doesn't
+    // account for margins
+    const scrollingElement = this.win.document.body;
+    const rect = scrollingElement./*OK*/getBoundingClientRect();
+    const style = computedStyle(this.win, scrollingElement);
+    return rect.height
+        + this.paddingTop_
+        + parseInt(style.marginTop, 10)
+        + parseInt(style.marginBottom, 10);
   }
 
   /** @override */

src/service/viewport/viewport-binding-natural.js

@@ -17,10 +17,10 @@
 import {Observable} from '../../observable';
 import {Services} from '../../services';
 import {ViewportBindingDef} from './viewport-binding-def';
+import {computedStyle, px, setImportantStyles} from '../../style';
 import {dev} from '../../log';
 import {isExperimentOn} from '../../experiments';
 import {layoutRectLtwh} from '../../layout-rect';
-import {px, setImportantStyles} from '../../style';
 
 
 const TAG_ = 'Viewport';
@@ -67,6 +67,9 @@ export class ViewportBindingNatural_ {
     /** @private @const {boolean} */
     this.useLayers_ = isExperimentOn(this.win, 'layers');
 
+    /** @private {number} */
+    this.paddingTop_ = 0;
+
     dev().fine(TAG_, 'initialized natural viewport');
   }
 
@@ -114,6 +117,7 @@ export class ViewportBindingNatural_ {
 
   /** @override */
   updatePaddingTop(paddingTop) {
+    this.paddingTop_ = paddingTop;
     setImportantStyles(this.win.document.documentElement, {
       'padding-top': px(paddingTop),
     });
@@ -195,14 +199,20 @@ export class ViewportBindingNatural_ {
 
   /** @override */
   getContentHeight() {
-    // The reparented body inside wrapper will have the correct content height.
-    // Body is overflow: hidden so that the scrollHeight include the margins of
-    // body's first and last child.
-    // Body height doesn't include paddingTop on the parent, so we add on the
-    // position of the body from the top of the viewport and subtract the
-    // scrollTop (as position relative to the viewport changes as you scroll).
-    const rect = this.win.document.body./*OK*/getBoundingClientRect();
-    return rect.height + rect.top + this.getScrollTop();
+    // Don't use scrollHeight, since it returns `MAX(viewport_height,
+    // document_height)`, even though we only want the latter. Also, it doesn't
+    // account for margins
+    const scrollingElement = this.getScrollingElement();
+    const rect = scrollingElement./*OK*/getBoundingClientRect();
+    const style = computedStyle(this.win, scrollingElement);
+    let paddingTop = 0;
+    if (scrollingElement !== this.win.document.documentElement) {
+      paddingTop = this.paddingTop_;
+    }
+    return rect.height
+        + paddingTop
+        + parseInt(style.marginTop, 10)
+        + parseInt(style.marginBottom, 10);
   }
 
   /** @override */