Throw cancellation errors where appropriate

ampproject · Oct 2, 2020 · ab3cabb · ab3cabb
1 parent 6a19ebf
commit ab3cabb
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 19 deletions.
diff --git a/src/custom-element.js b/src/custom-element.js
@@ -28,7 +28,12 @@ import {LayoutDelayMeter} from './layout-delay-meter';
 import {ResourceState} from './service/resource';
 import {Services} from './services';
 import {Signals} from './utils/signals';
-import {blockedByConsentError, isBlockedByConsent, reportError} from './error';
+import {
+  blockedByConsentError,
+  cancellation,
+  isBlockedByConsent,
+  reportError,
+} from './error';
 import {createLoaderElement} from '../src/loader.js';
 import {dev, devAssert, rethrowAsync, user, userAssert} from './log';
 import {getIntersectionChangeEntry} from './utils/intersection-observer-3p-host';
@@ -1153,6 +1158,10 @@ function createBaseCustomElementClass(win) {
     layoutCallback(signal) {
       assertNotTemplate(this);
       devAssert(this.isBuilt(), 'Must be built to receive viewport events');
+      if (signal.aborted) {
+        throw cancellation();
+      }
+
       this.dispatchCustomEventForTesting(AmpEvents.LOAD_START);
       const isLoadEvent = this.layoutCount_ == 0; // First layout is "load".
       this.signals_.reset(CommonSignals.UNLOAD);
@@ -1170,7 +1179,7 @@ function createBaseCustomElementClass(win) {
       return promise.then(
         () => {
           if (signal.aborted) {
-            return;
+            throw cancellation();
           }
           if (isLoadEvent) {
             this.signals_.signal(CommonSignals.LOAD_END);
@@ -1188,7 +1197,7 @@ function createBaseCustomElementClass(win) {
         },
         (reason) => {
           if (signal.aborted) {
-            return;
+            throw cancellation();
           }
           // add layoutCount_ by 1 despite load fails or not
           if (isLoadEvent) {

diff --git a/src/service/resource.js b/src/service/resource.js
@@ -17,9 +17,9 @@
 import {Deferred, tryResolve} from '../utils/promise';
 import {Layout} from '../layout';
 import {Services} from '../services';
+import {cancellation, isBlockedByConsent, reportError} from '../error';
 import {computedStyle, toggle} from '../style';
 import {dev, devAssert} from '../log';
-import {isBlockedByConsent, reportError} from '../error';
 import {
   layoutRectLtwh,
   layoutRectSizeEquals,
@@ -940,14 +940,10 @@ export class Resource {
     this.layoutCount_++;
     this.state_ = ResourceState.LAYOUT_SCHEDULED;
     this.abortController_ = new AbortController();
-    const signal = this.abortController_.signal;
+    const {signal} = this.abortController_;
 
     const promise = new Promise((resolve, reject) => {
       Services.vsyncFor(this.hostWin).mutate(() => {
-        if (signal.aborted) {
-          // The chained layoutCount_ will log the expected race error.
-          return;
-        }
         try {
           resolve(this.element.layoutCallback(signal));
         } catch (e) {
@@ -976,7 +972,7 @@ export class Resource {
       const err = dev().createError('layoutComplete race');
       err.associatedElement = this.element;
       dev().expectedError(TAG, err);
-      return;
+      throw cancellation();
     }
     if (this.loadPromiseResolve_) {
       this.loadPromiseResolve_();
@@ -990,7 +986,6 @@ export class Resource {
     this.lastLayoutError_ = opt_reason;
     if (success) {
       dev().fine(TAG, 'layout complete:', this.debugid);
-      return Promise.resolve(signal);
     } else {
       dev().fine(TAG, 'loading failed:', this.debugid, opt_reason);
       return Promise.reject(opt_reason);

diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js
@@ -1659,15 +1659,11 @@ export class ResourcesImpl {
   /**
    * @param {!./task-queue.TaskDef} task
    * @param {boolean} success
-   * @param {!AbortSignal|*} data
+   * @param {*=} opt_reason
    * @return {!Promise|undefined}
    * @private
    */
-  taskComplete_(task, success, data) {
-    // If we succeeded, then data is an AbortSignal.
-    if (success && data.aborted) {
-      return;
-    }
+  taskComplete_(task, success, opt_reason) {
     this.totalLayoutCount_++;
     if (task.resource.isInViewport() && this.firstVisibleTime_ >= 0) {
       this.slowLayoutCount_++;
@@ -1676,8 +1672,14 @@ export class ResourcesImpl {
     this.exec_.dequeue(task);
     this.schedulePass(POST_TASK_PASS_DELAY_);
     if (!success) {
-      dev().info(TAG_, 'task failed:', task.id, task.resource.debugid, data);
-      return Promise.reject(data);
+      dev().info(
+        TAG_,
+        'task failed:',
+        task.id,
+        task.resource.debugid,
+        opt_reason
+      );
+      return Promise.reject(opt_reason);
     }
   }