Fix for build race condition on ensureLoaded (#32979)

ampproject · Mar 1, 2021 · abcf9a4 · abcf9a4
1 parent 6a74422
commit abcf9a4
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 25 deletions.
diff --git a/src/custom-element.js b/src/custom-element.js
@@ -602,26 +602,31 @@ function createBaseCustomElementClass(win) {
           return this.whenLoaded();
         }
 
+        // Very ugly! The "built" signal must be resolved from the Resource
+        // and not the element itself because the Resource has not correctly
+        // set its state for the downstream to process it correctly.
         const resource = this.getResource_();
-        if (resource.getState() == ResourceState.LAYOUT_COMPLETE) {
-          return;
-        }
-        if (
-          resource.getState() != ResourceState.LAYOUT_SCHEDULED ||
-          resource.isMeasureRequested()
-        ) {
-          resource.measure();
-        }
-        if (!resource.isDisplayed()) {
-          return;
-        }
-        this.getResources().scheduleLayoutOrPreload(
-          resource,
-          /* layout */ true,
-          opt_parentPriority,
-          /* forceOutsideViewport */ true
-        );
-        return this.whenLoaded();
+        return resource.whenBuilt().then(() => {
+          if (resource.getState() == ResourceState.LAYOUT_COMPLETE) {
+            return;
+          }
+          if (
+            resource.getState() != ResourceState.LAYOUT_SCHEDULED ||
+            resource.isMeasureRequested()
+          ) {
+            resource.measure();
+          }
+          if (!resource.isDisplayed()) {
+            return;
+          }
+          this.getResources().scheduleLayoutOrPreload(
+            resource,
+            /* layout */ true,
+            opt_parentPriority,
+            /* forceOutsideViewport */ true
+          );
+          return this.whenLoaded();
+        });
       });
     }
 

diff --git a/test/unit/test-custom-element.js b/test/unit/test-custom-element.js
@@ -1584,7 +1584,8 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
             .once();
 
           const promise = element.ensureLoaded(parentPriority);
-          await element.buildInternal();
+          await resource.build();
+          await resource.whenBuilt();
           await element.layoutCallback();
 
           await promise;
@@ -1599,7 +1600,8 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
           container.appendChild(element);
           const resource = element.getResource_();
 
-          await element.buildInternal();
+          await resource.build();
+          await resource.whenBuilt();
           expect(element.isBuilt()).to.be.true;
 
           const parentPriority = 1;
@@ -1626,7 +1628,8 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
           container.appendChild(element);
           const resource = element.getResource_();
 
-          await element.buildInternal();
+          await resource.build();
+          await resource.whenBuilt();
           resource.measure();
           resource.layoutScheduled(Date.now());
           await resource.startLayout();
@@ -1644,7 +1647,8 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
           container.appendChild(element);
           const resource = element.getResource_();
 
-          await element.buildInternal();
+          await resource.build();
+          await resource.whenBuilt();
           resource.measure();
           resource.layoutScheduled(Date.now());
           const layoutCallbackStub = env.sandbox.stub(
@@ -1678,7 +1682,10 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
           const element = new ElementClass();
           element.setAttribute('layout', 'nodisplay');
           container.appendChild(element);
-          await element.buildInternal();
+          const resource = element.getResource_();
+
+          await resource.build();
+          await resource.whenBuilt();
 
           resourcesMock.expects('scheduleLayoutOrPreload').never();
 
@@ -1690,7 +1697,9 @@ describes.realWin('CustomElement', {amp: true}, (env) => {
           element.setAttribute('layout', 'nodisplay');
           container.appendChild(element);
           const resource = element.getResource_();
-          await element.buildInternal();
+
+          await resource.build();
+          await resource.whenBuilt();
 
           const measureSpy = env.sandbox.spy(resource, 'measure');
           resourcesMock.expects('scheduleLayoutOrPreload').never();