lots and lots of tests

ampproject · Mar 18, 2021 · d248c1c · d248c1c
1 parent b8d0ed7
commit d248c1c
Show file tree

Hide file tree

Showing 8 changed files with 507 additions and 141 deletions.
diff --git a/builtins/amp-img.js b/builtins/amp-img.js
@@ -57,7 +57,7 @@ export class AmpImg extends BaseElement {
   }
 
   /** @override @nocollapse */
-  static load() {
+  static usesLoading() {
     return true;
   }
 
@@ -336,8 +336,6 @@ export class AmpImg extends BaseElement {
       removeElement(img);
       this.img_ = null;
     }
-
-    return true;
   }
 
   /** @override */

diff --git a/src/base-element.js b/src/base-element.js
@@ -533,6 +533,9 @@ export class BaseElement {
    * times for resource management. The unmount should reverse the changes
    * made by the mount. See `unmountCallback` for more info.
    *
+   * If this callback returns a promise, the `readyState` becomes "complete"
+   * after the promise is resolved.
+   *
    * @param {!AbortSignal=} opt_abortSignal
    * @return {?Promise|undefined}
    */

diff --git a/src/custom-element.js b/src/custom-element.js
@@ -61,6 +61,7 @@ const UpgradeState = {
 };
 
 const NO_BUBBLES = {bubbles: false};
+const RETURN_TRUE = () => true;
 
 /**
  * Caches whether the template tag is supported to avoid memory allocations.
@@ -541,7 +542,9 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
           this.classList.remove('amp-notbuilt');
           this.signals_.signal(CommonSignals.BUILT);
 
-          if (!this.V1()) {
+          if (this.V1()) {
+            this.setReadyStateInternal(ReadyState.MOUNTING);
+          } else {
             this.setReadyStateInternal(ReadyState.LOADING);
             this.preconnect(/* onLayout */ false);
           }
@@ -633,15 +636,19 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
               : ReadyState.MOUNTING
           );
           this.mounted_ = true;
-          return this.impl_.mountCallback(signal);
+          const result = this.impl_.mountCallback(signal);
+          // The promise supports the V0 format for easy migration. If the
+          // `mountCallback` returns a promise, the assumption is that the
+          // element has finished loading when the promise completes.
+          return result ? result.then(RETURN_TRUE) : false;
         })
-        .then(() => {
+        .then((hasLoaded) => {
           this.mountAbortController_ = null;
           if (signal.aborted) {
             throw cancellation();
           }
           this.signals_.signal(CommonSignals.MOUNTED);
-          if (!this.implClass_.usesLoading(this)) {
+          if (!this.implClass_.usesLoading(this) || hasLoaded) {
             this.setReadyStateInternal(ReadyState.COMPLETE);
           }
         })
@@ -686,9 +693,6 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
         if (signal.aborted) {
           throw cancellation();
         }
-        if (this.mountPromise_) {
-          return this.mountPromise_;
-        }
         const scheduler = getSchedulerForDoc(this.getAmpDoc());
         scheduler.scheduleAsap(this);
         return this.whenMounted();
@@ -763,7 +767,7 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
      * @final
      */
     ensureLoaded(opt_parentPriority) {
-      return this.build().then(() => {
+      return this.mount().then(() => {
         if (this.V1()) {
           if (this.implClass_.usesLoading(this)) {
             this.impl_.ensureLoaded();
@@ -799,23 +803,6 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
       });
     }
 
-    /**
-     * Pauses the element.
-     */
-    pause() {
-      if (!this.isBuilt()) {
-        // Not built yet.
-        return;
-      }
-
-      this.impl_.pauseCallback();
-
-      // Legacy unlayoutOnPause support.
-      if (!this.V1() && this.impl_.unlayoutOnPause()) {
-        this.getResource_().unlayout();
-      }
-    }
-
     /**
      * Update the internal ready state.
      *
@@ -1354,8 +1341,6 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
         this.impl_.detachedCallback();
       }
       if (this.V1()) {
-        const scheduler = getSchedulerForDoc(this.getAmpDoc());
-        scheduler.unschedule(this);
         this.unmount();
       }
       this.toggleLoading(false);
@@ -1632,19 +1617,38 @@ function createBaseCustomElementClass(win, elementConnectedCallback) {
       );
     }
 
+    /**
+     * Pauses the element.
+     *
+     * @package @final
+     */
+    pause() {
+      if (!this.isBuilt()) {
+        // Not built yet.
+        return;
+      }
+
+      this.impl_.pauseCallback();
+
+      // Legacy unlayoutOnPause support.
+      if (!this.V1() && this.impl_.unlayoutOnPause()) {
+        this.getResource_().unlayout();
+      }
+    }
+
     /**
      * Requests the resource to resume its activity when the document returns
      * from an inactive state. The scope is up to the actual component. Among
      * other things the active playback of video or audio content may be
      * resumed.
      *
      * @package @final
-     * TODO(#31915): remove once V1 migration is complete.
      */
-    resumeCallback() {
-      if (this.isBuilt()) {
-        this.impl_.resumeCallback();
+    resume() {
+      if (!this.isBuilt()) {
+        return;
       }
+      this.impl_.resumeCallback();
     }
 
     /**

diff --git a/src/service/resource.js b/src/service/resource.js
@@ -1103,7 +1103,7 @@ export class Resource {
    * Calls element's resumeCallback callback.
    */
   resume() {
-    this.element.resumeCallback();
+    this.element.resume();
   }
 
   /**

diff --git a/test/unit/test-amp-img-v1.js b/test/unit/test-amp-img-v1.js
@@ -111,6 +111,42 @@ describes.realWin('amp-img V1', {amp: true}, (env) => {
     expect(togglePlaceholderSpy).to.be.calledOnce.calledWith(false);
   });
 
+  it('should unload the img on unmount before loaded', async () => {
+    const ampImg = await getImg({
+      src: '/examples/img/sample.jpg',
+      width: 300,
+      height: 200,
+      alt: 'An image',
+      title: 'Image title',
+      referrerpolicy: 'origin',
+    });
+    const iniImg = ampImg.querySelector('img');
+    expect(iniImg).to.exist;
+
+    ampImg.unmount();
+    expect(ampImg.querySelector('img')).to.not.exist;
+    expect(iniImg.src).to.contain('data:image/gif;');
+  });
+
+  it('should NOT unload the img on unmount after loaded', async () => {
+    const ampImg = await getImg({
+      src: '/examples/img/sample.jpg',
+      width: 300,
+      height: 200,
+      alt: 'An image',
+      title: 'Image title',
+      referrerpolicy: 'origin',
+    });
+    const iniImg = ampImg.querySelector('img');
+    expect(iniImg).to.exist;
+
+    Object.defineProperty(iniImg, 'complete', {value: true});
+
+    ampImg.unmount();
+    expect(ampImg.querySelector('img')).to.equal(iniImg);
+    expect(iniImg.src).to.not.contain('data:image/gif;');
+  });
+
   it('should set eager loading on ensureLoaded', async () => {
     const ampImg = await getImg({
       src: '/examples/img/sample.jpg',