Skip to content

Commit

Permalink
Resources: Don't become ready for layout if already scheduled (#29389)
Browse files Browse the repository at this point in the history
* Only go to READY_FOR_LAYOUT if already LAYOUT_COMPLETE

* Make cases more clear

* Add test case
  • Loading branch information
jridgewell authored Jul 21, 2020
1 parent ef87a55 commit 7f59ffb
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
23 changes: 15 additions & 8 deletions src/service/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {Layout} from '../layout';
import {Services} from '../services';
import {computedStyle, toggle} from '../style';
import {dev, devAssert} from '../log';
import {isBlockedByConsent} from '../error';
import {isBlockedByConsent, reportError} from '../error';
import {
layoutRectLtwh,
layoutRectSizeEquals,
Expand Down Expand Up @@ -498,13 +498,19 @@ export class Resource {
oldBox.top != newBox.top ||
sizeChanges
) {
if (
this.element.isUpgraded() &&
this.state_ != ResourceState.NOT_BUILT &&
(this.state_ == ResourceState.NOT_LAID_OUT ||
this.element.isRelayoutNeeded())
) {
this.state_ = ResourceState.READY_FOR_LAYOUT;
if (this.element.isUpgraded()) {
if (this.state_ == ResourceState.NOT_LAID_OUT) {
// If the element isn't laid out yet, then we're now ready for layout.
this.state_ = ResourceState.READY_FOR_LAYOUT;
} else if (
(this.state_ == ResourceState.LAYOUT_COMPLETE ||
this.state_ == ResourceState.LAYOUT_FAILED) &&
this.element.isRelayoutNeeded()
) {
// If the element was already laid out and we need to relayout, then
// go back to ready for layout.
this.state_ = ResourceState.READY_FOR_LAYOUT;
}
}
}

Expand Down Expand Up @@ -902,6 +908,7 @@ export class Resource {
this.state_
);
err.associatedElement = this.element;
reportError(err);
return Promise.reject(err);
}

Expand Down
16 changes: 16 additions & 0 deletions test/unit/test-resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,22 @@ describes.realWin('Resource', {amp: true}, (env) => {
expect(resource.getLayoutBox().width).to.equal(111 + 10);
});

it('should not relayout if element has not completed layout', () => {
elementMock.expects('isUpgraded').returns(true).atLeast(1);
resource.state_ = ResourceState.LAYOUT_SCHEDULED;
resource.layoutBox_ = {left: 11, top: 12, width: 111, height: 222};

// Width changed.
elementMock
.expects('getBoundingClientRect')
.returns({left: 11, top: 12, width: 111 + 10, height: 222})
.once();
elementMock.expects('isRelayoutNeeded').returns(true).atLeast(0);
resource.measure();
expect(resource.getState()).to.equal(ResourceState.LAYOUT_SCHEDULED);
expect(resource.getLayoutBox().width).to.equal(111 + 10);
});

it('should calculate NOT fixed for non-displayed elements', () => {
elementMock.expects('isUpgraded').returns(true).atLeast(1);
elementMock
Expand Down

0 comments on commit 7f59ffb

Please sign in to comment.