Ensure that an element is measured when resize is scheduled (ampproje…

…ct#5688)

src/service/resource.js

@@ -305,7 +305,8 @@ export class Resource {
   }
 
   /**
-   * Measures the resource's boundaries. Only allowed for upgraded elements.
+   * Measures the resource's boundaries. An upgraded element will be
+   * transitioned to the "ready for layout" state.
    */
   measure() {
     this.isMeasureRequested_ = false;

src/service/resources-impl.js

@@ -1302,6 +1302,31 @@ export class Resources {
    */
   scheduleChangeSize_(resource, newHeight, newWidth, force,
       opt_callback) {
+    if (resource.hasBeenMeasured()) {
+      this.completeScheduleChangeSize_(resource, newHeight, newWidth, force,
+          opt_callback);
+    } else {
+      // This is a rare case since most of times the element itself schedules
+      // resize requests. However, this case is possible when another element
+      // requests resize of a controlled element.
+      this.vsync_.measure(() => {
+        resource.measure();
+        this.completeScheduleChangeSize_(resource, newHeight, newWidth, force,
+            opt_callback);
+      });
+    }
+  }
+
+  /**
+   * @param {!Resource} resource
+   * @param {number|undefined} newHeight
+   * @param {number|undefined} newWidth
+   * @param {boolean} force
+   * @param {function(boolean)=} opt_callback A callback function
+   * @private
+   */
+  completeScheduleChangeSize_(resource, newHeight, newWidth, force,
+      opt_callback) {
     resource.resetPendingChangeSize();
     const layoutBox = resource.getLayoutBox();
     if ((newHeight === undefined || newHeight == layoutBox.height) &&

test/functional/test-resources.js

@@ -32,6 +32,7 @@ describe('Resources', () => {
     sandbox = sinon.sandbox.create();
     clock = sandbox.useFakeTimers();
     resources = new Resources(new AmpDocSingle(window));
+    resources.isRuntimeOn_ = false;
   });
 
   afterEach(() => {
@@ -334,6 +335,7 @@ describe('Resources pause/resume/unlayout scheduling', () => {
   beforeEach(() => {
     sandbox = sinon.sandbox.create();
     resources = new Resources(new AmpDocSingle(window));
+    resources.isRuntimeOn_ = false;
     const parentTuple = createElementWithResource(1);
     parent = parentTuple[0];
     child0 = document.createElement('div');
@@ -523,6 +525,7 @@ describe('Resources schedulePreload', () => {
   beforeEach(() => {
     sandbox = sinon.sandbox.create();
     resources = new Resources(new AmpDocSingle(window));
+    resources.isRuntimeOn_ = false;
     const parentTuple = createElementWithResource(1);
     parent = parentTuple[0];
     placeholder = document.createElement('div');
@@ -950,7 +953,8 @@ describe('Resources changeSize', () => {
     const resource = new Resource(id, createElement(rect), resources);
     resource.element['__AMP__RESOURCE'] = resource;
     resource.state_ = ResourceState.READY_FOR_LAYOUT;
-    resource.layoutBox_ = rect;
+    resource.layoutBox_ = layoutRectLtwh(
+        rect.left, rect.top, rect.width, rect.height);
     resource.changeSize = sandbox.spy();
     return resource;
   }
@@ -1058,6 +1062,31 @@ describe('Resources changeSize', () => {
     expect(resources.relayoutTop_).to.equal(resource1.layoutBox_.top);
   });
 
+  it('should measure non-measured elements', () => {
+    resource1.layoutBox_.top = -10000;
+    resource1.measure = sandbox.spy();
+    resource2.measure = sandbox.spy();
+
+    resources.scheduleChangeSize_(resource1, 111, 200, true);
+    resources.scheduleChangeSize_(resource2, 111, 222, true);
+    expect(resource1.hasBeenMeasured()).to.be.false;
+    expect(resource2.hasBeenMeasured()).to.be.true;
+
+    // Not yet scheduled, will wait until vsync.
+    expect(resource1.measure).to.not.be.called;
+
+    // Scheduling is done after vsync.
+    resources.vsync_.runScheduledTasks_();
+    expect(resource1.measure).to.be.calledOnce;
+    expect(resource2.measure).to.not.be.called;
+
+    // Notice that the `resource2` was scheduled first since it didn't
+    // require vsync.
+    expect(resources.requestsChangeSize_).to.have.length(2);
+    expect(resources.requestsChangeSize_[0].resource).to.equal(resource2);
+    expect(resources.requestsChangeSize_[1].resource).to.equal(resource1);
+  });
+
   describe('attemptChangeSize rules wrt viewport', () => {
     let overflowCallbackSpy;
     let vsyncSpy;
@@ -1326,6 +1355,7 @@ describe('Resources mutateElement and collapse', () => {
   beforeEach(() => {
     sandbox = sinon.sandbox.create();
     resources = new Resources(new AmpDocSingle(window));
+    resources.isRuntimeOn_ = false;
     viewportMock = sandbox.mock(resources.viewport_);
     resources.vsync_ = {
       mutate: callback => callback(),