From e876e881f23b31254bc1491734295c9a95761041 Mon Sep 17 00:00:00 2001 From: lannka Date: Mon, 26 Aug 2019 16:20:24 -0700 Subject: [PATCH 1/9] Move more methods from resources-impl to owners-impl --- extensions/amp-a4a/0.1/amp-a4a.js | 2 +- .../amp-access/0.1/test/test-amp-access.js | 6 +- extensions/amp-animation/0.1/amp-animation.js | 2 +- .../0.1/test/test-web-animations.js | 14 +- .../0.1/web-animation-service.js | 4 +- .../amp-animation/0.1/web-animations.js | 8 +- .../amp-story/1.0/test/test-amp-story-page.js | 14 +- src/service/owners-impl.js | 174 +++++++++++++----- src/service/resources-impl.js | 124 ++----------- test/unit/test-custom-element.js | 10 +- test/unit/test-owners.js | 10 +- test/unit/test-resources.js | 34 ++-- test/unit/test-standard-actions.js | 2 +- 13 files changed, 196 insertions(+), 208 deletions(-) diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js index 891e0d6fdc79..3316a27bff6c 100644 --- a/extensions/amp-a4a/0.1/amp-a4a.js +++ b/extensions/amp-a4a/0.1/amp-a4a.js @@ -1000,7 +1000,7 @@ export class AmpA4A extends AMP.BaseElement { Services.viewerForDoc(this.getAmpDoc()) .whenNextVisible() .then(() => { - Services.resourcesForDoc(this.getAmpDoc())./*OK*/ requireLayout( + Services.ownersForDoc(this.getAmpDoc())./*OK*/ requireLayout( this.element ); }); diff --git a/extensions/amp-access/0.1/test/test-amp-access.js b/extensions/amp-access/0.1/test/test-amp-access.js index 07368414f6f5..ccfd1931ed6f 100644 --- a/extensions/amp-access/0.1/test/test-amp-access.js +++ b/extensions/amp-access/0.1/test/test-amp-access.js @@ -320,7 +320,7 @@ describes.fakeWin( adapterMock = sandbox.mock(adapter); sandbox - .stub(service.resources_, 'mutateElement') + .stub(service.owners_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); @@ -587,7 +587,7 @@ describes.fakeWin( service = new AccessService(ampdoc); mutateElementStub = sandbox - .stub(service.resources_, 'mutateElement') + .stub(service.owners_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); @@ -1778,7 +1778,7 @@ describes.fakeWin( adapterDonutsMock = sandbox.mock(adapterDonuts); sandbox - .stub(service.resources_, 'mutateElement') + .stub(service.owners_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); diff --git a/extensions/amp-animation/0.1/amp-animation.js b/extensions/amp-animation/0.1/amp-animation.js index 7f77c9685cd5..99c018a3f806 100644 --- a/extensions/amp-animation/0.1/amp-animation.js +++ b/extensions/amp-animation/0.1/amp-animation.js @@ -502,7 +502,7 @@ export class AmpAnimation extends AMP.BaseElement { this.getRootNode_(), baseUrl, this.getVsync(), - this.element.getResources() + Services.ownersForDoc(this.element.getAmpDoc()) ); return builder.createRunner(configJson, args, opt_positionObserverData); }); diff --git a/extensions/amp-animation/0.1/test/test-web-animations.js b/extensions/amp-animation/0.1/test/test-web-animations.js index f760aa4f0162..ab550ba71a8e 100644 --- a/extensions/amp-animation/0.1/test/test-web-animations.js +++ b/extensions/amp-animation/0.1/test/test-web-animations.js @@ -24,6 +24,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { let win, doc; let vsync; let resources; + let owners; let requireLayoutSpy; let target1, target2; let warnStub; @@ -59,6 +60,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { return Promise.resolve(callback()); }); resources = win.__AMP_SERVICES.resources.obj; + owners = win.__AMP_SERVICES.owners.obj; requireLayoutSpy = sandbox.spy(resources, 'requireLayout'); target1 = doc.createElement('div'); @@ -82,7 +84,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { doc, 'https://acme.org/', /* vsync */ null, - /* resources */ null + /* owners */ null ); sandbox.stub(builder, 'requireLayout'); const scanner = builder.createScanner_([]); @@ -796,7 +798,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { doc, 'https://acme.org/', vsync, - /* resources */ null + /* owners */ null ); const cssContext = builder.css_; expect(cssContext.supports('supported: 0')).to.be.false; @@ -975,7 +977,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { doc, 'https://acme.org/', vsync, - /* resources */ null + /* owners */ null ); sandbox.stub(builder, 'requireLayout'); const spec = {target: target1, delay: 101, keyframes: {}}; @@ -1010,7 +1012,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { animation2.id = 'animation2'; doc.body.appendChild(animation2); - builder = new Builder(win, doc, 'https://acme.org/', vsync, resources); + builder = new Builder(win, doc, 'https://acme.org/', vsync, owners); sandbox.stub(builder, 'requireLayout'); scanner = builder.createScanner_([]); }); @@ -1255,7 +1257,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { doc, 'https://acme.org/', /* vsync */ null, - /* resources */ null + /* owners */ null ); css = builder.css_; parseSpy = sandbox.spy(css, 'resolveAsNode_'); @@ -1562,7 +1564,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { doc, 'https://acme.org/', vsync, - resources + owners ); return builder.createRunner(spec); } diff --git a/extensions/amp-animation/0.1/web-animation-service.js b/extensions/amp-animation/0.1/web-animation-service.js index 2eab3785da3c..ddd284975939 100644 --- a/extensions/amp-animation/0.1/web-animation-service.js +++ b/extensions/amp-animation/0.1/web-animation-service.js @@ -29,7 +29,7 @@ export class WebAnimationService { this.vsync_ = Services.vsyncFor(ampdoc.win); /** @private @const */ - this.resources_ = Services.resourcesForDoc(ampdoc); + this.owners_ = Services.ownersForDoc(ampdoc); } /** @@ -43,7 +43,7 @@ export class WebAnimationService { this.ampdoc_.getRootNode(), this.ampdoc_.getUrl(), this.vsync_, - this.resources_ + this.owners_ ); } } diff --git a/extensions/amp-animation/0.1/web-animations.js b/extensions/amp-animation/0.1/web-animations.js index 34079e1b12b5..56dfc5590162 100644 --- a/extensions/amp-animation/0.1/web-animations.js +++ b/extensions/amp-animation/0.1/web-animations.js @@ -151,9 +151,9 @@ export class Builder { * @param {!Document|!ShadowRoot} rootNode * @param {string} baseUrl * @param {!../../../src/service/vsync-impl.Vsync} vsync - * @param {!../../../src/service/resources-impl.ResourcesDef} resources + * @param {!../../../src/service/owners-impl.OwnersDef} owners */ - constructor(win, rootNode, baseUrl, vsync, resources) { + constructor(win, rootNode, baseUrl, vsync, owners) { /** @const @private */ this.win_ = win; @@ -164,7 +164,7 @@ export class Builder { this.vsync_ = vsync; /** @const @private */ - this.resources_ = resources; + this.owners_ = owners; /** @const @private {!Array} */ this.targets_ = []; @@ -231,7 +231,7 @@ export class Builder { requireLayout(target) { if (!this.targets_.includes(target)) { this.targets_.push(target); - this.loaders_.push(this.resources_.requireLayout(target)); + this.loaders_.push(this.owners_.requireLayout(target)); } } diff --git a/extensions/amp-story/1.0/test/test-amp-story-page.js b/extensions/amp-story/1.0/test/test-amp-story-page.js index ba911fcbad63..91fa2ca7150a 100644 --- a/extensions/amp-story/1.0/test/test-amp-story-page.js +++ b/extensions/amp-story/1.0/test/test-amp-story-page.js @@ -150,7 +150,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should perform media operations when state becomes active', done => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); sandbox.stub(page, 'loadPromise').returns(Promise.resolve()); @@ -209,7 +209,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { let mediaPoolMock; sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); page.buildCallback(); @@ -276,7 +276,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should pause/rewind media when state becomes not active', done => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); const videoEl = win.document.createElement('video'); @@ -321,7 +321,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should pause media when state becomes paused', done => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); const videoEl = win.document.createElement('video'); videoEl.setAttribute('src', 'https://example.com/video.mp3'); @@ -457,7 +457,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should start tracking media performance when entering the page', () => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = true; const startMeasuringStub = sandbox.stub( @@ -479,7 +479,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should stop tracking media performance when leaving the page', () => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = true; const stopMeasuringStub = sandbox.stub( @@ -502,7 +502,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should not start tracking media performance if tracking is off', () => { sandbox - .stub(page.resources_, 'getResourceForElement') + .stub(page.owners_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = false; const startMeasuringStub = sandbox.stub( diff --git a/src/service/owners-impl.js b/src/service/owners-impl.js index 9d4fc12e91f3..b874f6679355 100644 --- a/src/service/owners-impl.js +++ b/src/service/owners-impl.js @@ -107,6 +107,15 @@ export class OwnersDef { * @param {boolean} inLocalViewport */ updateInViewport(parentElement, subElements, inLocalViewport) {} + + /** + * Requires the layout of the specified element or top-level sub-elements + * within. + * @param {!Element} element + * @param {number=} opt_parentPriority + * @return {!Promise} + */ + requireLayout(element, opt_parentPriority) {} } /** @@ -150,13 +159,9 @@ export class Owners { const parentResource = this.resources_.getResourceForElement(parentElement); subElements = elements(subElements); - this.resources_.findResourcesInElements( - parentResource, - subElements, - resource => { - resource.pause(); - } - ); + this.findResourcesInElements_(parentResource, subElements, resource => { + resource.pause(); + }); } /** @override */ @@ -164,13 +169,9 @@ export class Owners { const parentResource = this.resources_.getResourceForElement(parentElement); subElements = elements(subElements); - this.resources_.findResourcesInElements( - parentResource, - subElements, - resource => { - resource.resume(); - } - ); + this.findResourcesInElements_(parentResource, subElements, resource => { + resource.resume(); + }); } /** @override */ @@ -178,13 +179,9 @@ export class Owners { const parentResource = this.resources_.getResourceForElement(parentElement); subElements = elements(subElements); - this.resources_.findResourcesInElements( - parentResource, - subElements, - resource => { - resource.unlayout(); - } - ); + this.findResourcesInElements_(parentResource, subElements, resource => { + resource.unlayout(); + }); } /** @override */ @@ -196,6 +193,83 @@ export class Owners { ); } + /** @override */ + requireLayout(element, opt_parentPriority) { + const promises = []; + this.discoverResourcesForElement_(element, resource => { + if (resource.getState() == ResourceState.LAYOUT_COMPLETE) { + return; + } + if (resource.getState() != ResourceState.LAYOUT_SCHEDULED) { + promises.push( + resource.whenBuilt().then(() => { + resource.measure(); + if (!resource.isDisplayed()) { + return; + } + this.resources_.scheduleLayoutOrPreload( + resource, + /* layout */ true, + opt_parentPriority, + /* forceOutsideViewport */ true + ); + return resource.loadedOnce(); + }) + ); + } else if (resource.isDisplayed()) { + promises.push(resource.loadedOnce()); + } + }); + return Promise.all(promises); + } + + /** + * Finds resources within the parent resource's shallow subtree. + * @param {!Resource} parentResource + * @param {!Array} elements + * @param {function(!Resource)} callback + * @private + */ + findResourcesInElements_(parentResource, elements, callback) { + elements.forEach(element => { + devAssert(parentResource.element.contains(element)); + this.discoverResourcesForElement_(element, callback); + }); + } + + /** + * @param {!Element} element + * @param {function(!Resource)} callback + */ + discoverResourcesForElement_(element, callback) { + // Breadth-first search. + if (element.classList.contains('i-amphtml-element')) { + callback(this.resources_.getResourceForElement(element)); + // Also schedule amp-element that is a placeholder for the element. + const placeholder = element.getPlaceholder(); + if (placeholder) { + this.discoverResourcesForElement_(placeholder, callback); + } + } else { + const ampElements = element.getElementsByClassName('i-amphtml-element'); + const seen = []; + for (let i = 0; i < ampElements.length; i++) { + const ampElement = ampElements[i]; + let covered = false; + for (let j = 0; j < seen.length; j++) { + if (seen[j].contains(ampElement)) { + covered = true; + break; + } + } + if (!covered) { + seen.push(ampElement); + callback(this.resources_.getResourceForElement(ampElement)); + } + } + } + } + /** * Schedules layout or preload for the sub-resources of the specified * resource. @@ -205,27 +279,43 @@ export class Owners { * @private */ scheduleLayoutOrPreloadForSubresources_(parentResource, layout, subElements) { - this.resources_.findResourcesInElements( - parentResource, - subElements, - resource => { - if (resource.getState() === ResourceState.NOT_BUILT) { - resource.whenBuilt().then(() => { - this.resources_.measureAndTryScheduleLayout( - resource, - !layout, - parentResource.getLayoutPriority() - ); - }); - } else { - this.resources_.measureAndTryScheduleLayout( + this.findResourcesInElements_(parentResource, subElements, resource => { + if (resource.getState() === ResourceState.NOT_BUILT) { + resource.whenBuilt().then(() => { + this.measureAndTryScheduleLayout_( resource, !layout, parentResource.getLayoutPriority() ); - } + }); + } else { + this.measureAndTryScheduleLayout_( + resource, + !layout, + parentResource.getLayoutPriority() + ); } - ); + }); + } + + /** + * @param {!Resource} resource + * @param {boolean} isPreload + * @param {number=} opt_parentPriority + * @private + */ + measureAndTryScheduleLayout_(resource, isPreload, opt_parentPriority) { + resource.measure(); + if ( + resource.getState() === ResourceState.READY_FOR_LAYOUT && + resource.isDisplayed() + ) { + this.resources_.scheduleLayoutOrPreload( + resource, + !isPreload, + opt_parentPriority + ); + } } /** @@ -241,13 +331,9 @@ export class Owners { inLocalViewport ) { const inViewport = parentResource.isInViewport() && inLocalViewport; - this.resources_.findResourcesInElements( - parentResource, - subElements, - resource => { - resource.setInViewport(inViewport); - } - ); + this.findResourcesInElements_(parentResource, subElements, resource => { + resource.setInViewport(inViewport); + }); } } diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index c72e39ecf5f0..ffc8660748c9 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -265,21 +265,18 @@ export class ResourcesDef extends MutatorsDef { removeForChildWindow(childWin) {} /** - * Finds resources within the parent resource's shallow subtree. - * @param {!Resource} parentResource - * @param {!Array} elements - * @param {function(!Resource)} callback - * @package - */ - findResourcesInElements(parentResource, elements, callback) {} - - /** + * Schedules layout or preload for the specified resource. * @param {!Resource} resource - * @param {boolean} isPreload + * @param {boolean} layout * @param {number=} opt_parentPriority - * @package + * @param {boolean=} opt_forceOutsideViewport */ - measureAndTryScheduleLayout(resource, isPreload, opt_parentPriority) {} + scheduleLayoutOrPreload( + resource, + layout, + opt_parentPriority, + opt_forceOutsideViewport + ) {} /** * Schedules the work pass at the latest with the specified delay. @@ -306,15 +303,6 @@ export class ResourcesDef extends MutatorsDef { */ ampInitComplete() {} - /** - * Requires the layout of the specified element or top-level sub-elements - * within. - * @param {!Element} element - * @param {number=} opt_parentPriority - * @return {!Promise} - */ - requireLayout(element, opt_parentPriority) {} - /** * Updates the priority of the resource. If there are tasks currently * scheduled, their priority is updated as well. @@ -838,14 +826,6 @@ export class Resources { toRemove.forEach(r => this.removeResource_(r, /* disconnect */ true)); } - /** @override */ - findResourcesInElements(parentResource, elements, callback) { - elements.forEach(element => { - devAssert(parentResource.element.contains(element)); - this.discoverResourcesForElement_(element, callback); - }); - } - /** @override */ upgraded(element) { const resource = Resource.forElement(element); @@ -853,36 +833,6 @@ export class Resources { dev().fine(TAG_, 'element upgraded:', resource.debugid); } - /** @override */ - requireLayout(element, opt_parentPriority) { - const promises = []; - this.discoverResourcesForElement_(element, resource => { - if (resource.getState() == ResourceState.LAYOUT_COMPLETE) { - return; - } - if (resource.getState() != ResourceState.LAYOUT_SCHEDULED) { - promises.push( - resource.whenBuilt().then(() => { - resource.measure(); - if (!resource.isDisplayed()) { - return; - } - this.scheduleLayoutOrPreload_( - resource, - /* layout */ true, - opt_parentPriority, - /* forceOutsideViewport */ true - ); - return resource.loadedOnce(); - }) - ); - } else if (resource.isDisplayed()) { - promises.push(resource.loadedOnce()); - } - }); - return Promise.all(promises); - } - /** @override */ updateLayoutPriority(element, newLayoutPriority) { const resource = Resource.forElement(element); @@ -1079,17 +1029,6 @@ export class Resources { this.schedulePass(FOUR_FRAME_DELAY_); } - /** @override */ - measureAndTryScheduleLayout(resource, isPreload, opt_parentPriority) { - resource.measure(); - if ( - resource.getState() === ResourceState.READY_FOR_LAYOUT && - resource.isDisplayed() - ) { - this.scheduleLayoutOrPreload_(resource, !isPreload, opt_parentPriority); - } - } - /** @override */ schedulePass(opt_delay, opt_relayoutAll) { if (opt_relayoutAll) { @@ -1692,7 +1631,7 @@ export class Resources { for (let i = 0; i < this.resources_.length; i++) { const r = this.resources_[i]; // TODO(dvoytenko): This extra build has to be merged with the - // scheduleLayoutOrPreload_ method below. + // scheduleLayoutOrPreload method below. // Force build for all resources visible, measured, and in the viewport. if ( !r.isBuilt() && @@ -1715,7 +1654,7 @@ export class Resources { // layers. This is currently a short-term fix to the problem that // the fixed elements get incorrect top coord. if (r.isDisplayed() && r.overlaps(loadRect)) { - this.scheduleLayoutOrPreload_(r, /* layout */ true); + this.scheduleLayoutOrPreload(r, /* layout */ true); } } } @@ -1742,7 +1681,7 @@ export class Resources { r.idleRenderOutsideViewport() ) { dev().fine(TAG_, 'idleRenderOutsideViewport layout:', r.debugid); - this.scheduleLayoutOrPreload_(r, /* layout */ false); + this.scheduleLayoutOrPreload(r, /* layout */ false); idleScheduledCount++; } } @@ -1760,7 +1699,7 @@ export class Resources { r.isDisplayed() ) { dev().fine(TAG_, 'idle layout:', r.debugid); - this.scheduleLayoutOrPreload_(r, /* layout */ false); + this.scheduleLayoutOrPreload(r, /* layout */ false); idleScheduledCount++; } } @@ -2168,9 +2107,9 @@ export class Resources { * @param {boolean} layout * @param {number=} opt_parentPriority * @param {boolean=} opt_forceOutsideViewport - * @private + * @package */ - scheduleLayoutOrPreload_( + scheduleLayoutOrPreload( resource, layout, opt_parentPriority, @@ -2254,39 +2193,6 @@ export class Resources { task.resource.layoutScheduled(task.scheduleTime); } - /** - * @param {!Element} element - * @param {function(!Resource)} callback - */ - discoverResourcesForElement_(element, callback) { - // Breadth-first search. - if (element.classList.contains('i-amphtml-element')) { - callback(this.getResourceForElement(element)); - // Also schedule amp-element that is a placeholder for the element. - const placeholder = element.getPlaceholder(); - if (placeholder) { - this.discoverResourcesForElement_(placeholder, callback); - } - } else { - const ampElements = element.getElementsByClassName('i-amphtml-element'); - const seen = []; - for (let i = 0; i < ampElements.length; i++) { - const ampElement = ampElements[i]; - let covered = false; - for (let j = 0; j < seen.length; j++) { - if (seen[j].contains(ampElement)) { - covered = true; - break; - } - } - if (!covered) { - seen.push(ampElement); - callback(this.getResourceForElement(ampElement)); - } - } - } - } - /** * @return {!Promise} when first pass executed. */ diff --git a/test/unit/test-custom-element.js b/test/unit/test-custom-element.js index 43e69f9f943b..e80c2736eecf 100644 --- a/test/unit/test-custom-element.js +++ b/test/unit/test-custom-element.js @@ -157,7 +157,7 @@ describes.realWin('CustomElement', {amp: true}, env => { allowConsoleError(() => { expect(() => element.getAmpDoc()).to.throw(/no ampdoc yet/); }); - expect(element.resources_).to.be.null; + expect(element.owners_).to.be.null; allowConsoleError(() => { expect(() => element.getResources()).to.throw(/no resources yet/); }); @@ -166,7 +166,7 @@ describes.realWin('CustomElement', {amp: true}, env => { container.appendChild(element); expect(element.ampdoc_).to.be.ok; expect(element.getAmpDoc()).to.be.ok; - expect(element.resources_).to.be.ok; + expect(element.owners_).to.be.ok; expect(element.getResources()).to.be.ok; }); @@ -1050,7 +1050,7 @@ describes.realWin('CustomElement', {amp: true}, env => { element.setAttribute('layout', 'fill'); element.everAttached = true; element.ampdoc_ = env.ampdoc; - element.resources_ = resources; + element.owners_ = resources; resourcesMock .expects('upgraded') .withExactArgs(element) @@ -1838,7 +1838,7 @@ describes.realWin('CustomElement', {amp: true}, env => { element.layoutWidth_ = 300; element.layout_ = Layout.FIXED; element.setAttribute('layout', 'fixed'); - element.resources_ = resources; + element.owners_ = resources; vsync = Services.vsyncFor(win); sandbox.stub(vsync, 'run').callsFake(task => { if (task.measure) { @@ -2192,7 +2192,7 @@ describes.realWin('CustomElement Overflow Element', {amp: true}, env => { element = new ElementClass(); element.layoutWidth_ = 300; element.layout_ = Layout.FIXED; - element.resources_ = resources; + element.owners_ = resources; overflowElement = doc.createElement('div'); overflowElement.setAttribute('overflow', ''); element.appendChild(overflowElement); diff --git a/test/unit/test-owners.js b/test/unit/test-owners.js index f5191c66aa45..662beb03c670 100644 --- a/test/unit/test-owners.js +++ b/test/unit/test-owners.js @@ -179,10 +179,7 @@ describes.realWin( sandbox.stub(resource, 'isDisplayed').returns(true); sandbox.stub(resource, 'isInViewport').returns(true); const measureStub = sandbox.stub(resource, 'measure'); - const scheduleStub = sandbox.stub( - resources, - 'scheduleLayoutOrPreload_' - ); + const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); owners.scheduleLayout(parentElement, element); expect(measureStub).to.be.calledOnce; expect(scheduleStub).to.be.calledOnce; @@ -203,10 +200,7 @@ describes.realWin( const measureStub = sandbox.stub(resource, 'measure').callsFake(() => { resource.state_ = ResourceState.READY_FOR_LAYOUT; }); - const scheduleStub = sandbox.stub( - resources, - 'scheduleLayoutOrPreload_' - ); + const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); owners.scheduleLayout(parentElement, element); expect(measureStub).to.not.be.called; expect(scheduleStub).to.not.be.called; diff --git a/test/unit/test-resources.js b/test/unit/test-resources.js index af403216f43c..d79a20499ae8 100644 --- a/test/unit/test-resources.js +++ b/test/unit/test-resources.js @@ -302,7 +302,7 @@ describe('Resources', () => { sandbox .stub(resources.viewer_, 'getVisibilityState') .returns(VisibilityState.PRERENDER); - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(0); } ); @@ -325,7 +325,7 @@ describe('Resources', () => { sandbox .stub(resources.viewer_, 'getVisibilityState') .returns(VisibilityState.PRERENDER); - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].forceOutsideViewport).to.be.false; }); @@ -348,7 +348,7 @@ describe('Resources', () => { sandbox .stub(resources.viewer_, 'getVisibilityState') .returns(VisibilityState.HIDDEN); - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(0); }); @@ -367,7 +367,7 @@ describe('Resources', () => { startLayout: () => {}, applySizesAndMediaQuery: () => {}, }; - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(0); } ); @@ -390,7 +390,7 @@ describe('Resources', () => { getTaskId: () => 'resource#L', applySizesAndMediaQuery: () => {}, }; - resources.scheduleLayoutOrPreload_(resource, true, 0, /* force */ true); + resources.scheduleLayoutOrPreload(resource, true, 0, /* force */ true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].forceOutsideViewport).to.be.true; } @@ -414,7 +414,7 @@ describe('Resources', () => { getTaskId: () => 'resource#L', applySizesAndMediaQuery: () => {}, }; - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].forceOutsideViewport).to.be.false; } @@ -438,7 +438,7 @@ describe('Resources', () => { getTaskId: () => 'resource#L', applySizesAndMediaQuery: () => {}, }; - resources.scheduleLayoutOrPreload_(resource, true); + resources.scheduleLayoutOrPreload(resource, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].forceOutsideViewport).to.be.false; } @@ -766,7 +766,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { resource1 = createResource(1, layoutRectLtwh(10, 10, 100, 100)); resource2 = createResource(2, layoutRectLtwh(10, 1010, 100, 100)); - resources.resources_ = [resource1, resource2]; + resources.owners_ = [resource1, resource2]; resources.vsync_ = { mutate: callback => callback(), measurePromise: callback => Promise.resolve(callback()), @@ -993,7 +993,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should schedule resource for execution', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1005,7 +1005,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should record layout schedule time on the resource element', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); resources.work_(); expect(resource1.getState()).to.equal(ResourceState.LAYOUT_SCHEDULED); @@ -1013,7 +1013,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should not schedule resource execution outside viewport', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1030,7 +1030,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should force schedule resource execution outside viewport', () => { - resources.scheduleLayoutOrPreload_(resource1, true, 0, /* force */ true); + resources.scheduleLayoutOrPreload(resource1, true, 0, /* force */ true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1045,7 +1045,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should schedule resource prerender when doc in prerender mode', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1066,7 +1066,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should not schedule resource prerender', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1087,7 +1087,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { }); it('should schedule resource execution when doc is hidden', () => { - resources.scheduleLayoutOrPreload_(resource1, true); + resources.scheduleLayoutOrPreload(resource1, true); expect(resources.queue_.getSize()).to.equal(1); expect(resources.queue_.tasks_[0].resource).to.equal(resource1); @@ -1524,7 +1524,7 @@ describe('Resources changeSize', () => { resource1 = createResource(1, layoutRectLtwh(10, 10, 100, 100)); resource2 = createResource(2, layoutRectLtwh(10, 1010, 100, 100)); - resources.resources_ = [resource1, resource2]; + resources.owners_ = [resource1, resource2]; }); afterEach(() => { @@ -2914,7 +2914,7 @@ describes.realWin('Resources mutateElement and collapse', {amp: true}, env => { resource1 = createResource(1, layoutRectLtwh(10, 10, 100, 100)); resource2 = createResource(2, layoutRectLtwh(10, 1010, 100, 100)); - resources.resources_ = [resource1, resource2]; + resources.owners_ = [resource1, resource2]; resource1RequestMeasureStub = sandbox.stub(resource1, 'requestMeasure'); resource2RequestMeasureStub = sandbox.stub(resource2, 'requestMeasure'); diff --git a/test/unit/test-standard-actions.js b/test/unit/test-standard-actions.js index 0253b6c079f5..c9905dee257f 100644 --- a/test/unit/test-standard-actions.js +++ b/test/unit/test-standard-actions.js @@ -49,7 +49,7 @@ describes.sandboxed('StandardActions', {}, () => { function stubMutate(methodName) { return sandbox - .stub(standardActions.resources_, methodName) + .stub(standardActions.owners_, methodName) .callsFake((unusedElement, mutator) => mutator()); } From c99bc0ff5220466b7fd790ee4851df64f45ec652 Mon Sep 17 00:00:00 2001 From: lannka Date: Mon, 26 Aug 2019 16:23:01 -0700 Subject: [PATCH 2/9] lint --- extensions/amp-animation/0.1/test/test-web-animations.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/extensions/amp-animation/0.1/test/test-web-animations.js b/extensions/amp-animation/0.1/test/test-web-animations.js index ab550ba71a8e..15d291abceaa 100644 --- a/extensions/amp-animation/0.1/test/test-web-animations.js +++ b/extensions/amp-animation/0.1/test/test-web-animations.js @@ -1559,13 +1559,7 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { } function createRunner(spec) { - const builder = new Builder( - win, - doc, - 'https://acme.org/', - vsync, - owners - ); + const builder = new Builder(win, doc, 'https://acme.org/', vsync, owners); return builder.createRunner(spec); } From 08c2ae3837674c6192698ea6f7cd3bd1b93c4279 Mon Sep 17 00:00:00 2001 From: lannka Date: Mon, 26 Aug 2019 16:27:15 -0700 Subject: [PATCH 3/9] revert --- extensions/amp-access/0.1/test/test-amp-access.js | 6 +++--- .../amp-story/1.0/test/test-amp-story-page.js | 14 +++++++------- test/unit/test-custom-element.js | 10 +++++----- test/unit/test-standard-actions.js | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/extensions/amp-access/0.1/test/test-amp-access.js b/extensions/amp-access/0.1/test/test-amp-access.js index ccfd1931ed6f..07368414f6f5 100644 --- a/extensions/amp-access/0.1/test/test-amp-access.js +++ b/extensions/amp-access/0.1/test/test-amp-access.js @@ -320,7 +320,7 @@ describes.fakeWin( adapterMock = sandbox.mock(adapter); sandbox - .stub(service.owners_, 'mutateElement') + .stub(service.resources_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); @@ -587,7 +587,7 @@ describes.fakeWin( service = new AccessService(ampdoc); mutateElementStub = sandbox - .stub(service.owners_, 'mutateElement') + .stub(service.resources_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); @@ -1778,7 +1778,7 @@ describes.fakeWin( adapterDonutsMock = sandbox.mock(adapterDonuts); sandbox - .stub(service.owners_, 'mutateElement') + .stub(service.resources_, 'mutateElement') .callsFake((unusedElement, mutator) => { mutator(); return Promise.resolve(); diff --git a/extensions/amp-story/1.0/test/test-amp-story-page.js b/extensions/amp-story/1.0/test/test-amp-story-page.js index 91fa2ca7150a..ba911fcbad63 100644 --- a/extensions/amp-story/1.0/test/test-amp-story-page.js +++ b/extensions/amp-story/1.0/test/test-amp-story-page.js @@ -150,7 +150,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should perform media operations when state becomes active', done => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); sandbox.stub(page, 'loadPromise').returns(Promise.resolve()); @@ -209,7 +209,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { let mediaPoolMock; sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); page.buildCallback(); @@ -276,7 +276,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should pause/rewind media when state becomes not active', done => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); const videoEl = win.document.createElement('video'); @@ -321,7 +321,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should pause media when state becomes paused', done => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); const videoEl = win.document.createElement('video'); videoEl.setAttribute('src', 'https://example.com/video.mp3'); @@ -457,7 +457,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should start tracking media performance when entering the page', () => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = true; const startMeasuringStub = sandbox.stub( @@ -479,7 +479,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should stop tracking media performance when leaving the page', () => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = true; const stopMeasuringStub = sandbox.stub( @@ -502,7 +502,7 @@ describes.realWin('amp-story-page', {amp: true}, env => { it('should not start tracking media performance if tracking is off', () => { sandbox - .stub(page.owners_, 'getResourceForElement') + .stub(page.resources_, 'getResourceForElement') .returns({isDisplayed: () => true}); isPerformanceTrackingOn = false; const startMeasuringStub = sandbox.stub( diff --git a/test/unit/test-custom-element.js b/test/unit/test-custom-element.js index e80c2736eecf..43e69f9f943b 100644 --- a/test/unit/test-custom-element.js +++ b/test/unit/test-custom-element.js @@ -157,7 +157,7 @@ describes.realWin('CustomElement', {amp: true}, env => { allowConsoleError(() => { expect(() => element.getAmpDoc()).to.throw(/no ampdoc yet/); }); - expect(element.owners_).to.be.null; + expect(element.resources_).to.be.null; allowConsoleError(() => { expect(() => element.getResources()).to.throw(/no resources yet/); }); @@ -166,7 +166,7 @@ describes.realWin('CustomElement', {amp: true}, env => { container.appendChild(element); expect(element.ampdoc_).to.be.ok; expect(element.getAmpDoc()).to.be.ok; - expect(element.owners_).to.be.ok; + expect(element.resources_).to.be.ok; expect(element.getResources()).to.be.ok; }); @@ -1050,7 +1050,7 @@ describes.realWin('CustomElement', {amp: true}, env => { element.setAttribute('layout', 'fill'); element.everAttached = true; element.ampdoc_ = env.ampdoc; - element.owners_ = resources; + element.resources_ = resources; resourcesMock .expects('upgraded') .withExactArgs(element) @@ -1838,7 +1838,7 @@ describes.realWin('CustomElement', {amp: true}, env => { element.layoutWidth_ = 300; element.layout_ = Layout.FIXED; element.setAttribute('layout', 'fixed'); - element.owners_ = resources; + element.resources_ = resources; vsync = Services.vsyncFor(win); sandbox.stub(vsync, 'run').callsFake(task => { if (task.measure) { @@ -2192,7 +2192,7 @@ describes.realWin('CustomElement Overflow Element', {amp: true}, env => { element = new ElementClass(); element.layoutWidth_ = 300; element.layout_ = Layout.FIXED; - element.owners_ = resources; + element.resources_ = resources; overflowElement = doc.createElement('div'); overflowElement.setAttribute('overflow', ''); element.appendChild(overflowElement); diff --git a/test/unit/test-standard-actions.js b/test/unit/test-standard-actions.js index c9905dee257f..0253b6c079f5 100644 --- a/test/unit/test-standard-actions.js +++ b/test/unit/test-standard-actions.js @@ -49,7 +49,7 @@ describes.sandboxed('StandardActions', {}, () => { function stubMutate(methodName) { return sandbox - .stub(standardActions.owners_, methodName) + .stub(standardActions.resources_, methodName) .callsFake((unusedElement, mutator) => mutator()); } From b76c4160dad32637391343adb39a834896d7965d Mon Sep 17 00:00:00 2001 From: lannka Date: Wed, 28 Aug 2019 13:58:11 -0700 Subject: [PATCH 4/9] Fix tests --- build-system/tasks/presubmit-checks.js | 6 +----- .../amp-animation/0.1/test/test-web-animations.js | 9 +++++---- src/service/resources-impl.js | 10 ++-------- test/unit/test-resources.js | 10 +++++----- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index 224fa52f442e..fc63228cab95 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -611,14 +611,10 @@ const forbiddenTerms = { 'src/service/viewer-impl.js', ], }, - '\\.findResourcesInElements\\(': { + '\\.scheduleLayoutOrPreload\\(': { message: 'findResourcesInElements is a restricted API.', whitelist: ['src/service/owners-impl.js'], }, - '\\.measureAndTryScheduleLayout\\(': { - message: 'measureAndTryScheduleLayout is a restricted API.', - whitelist: ['src/service/owners-impl.js'], - }, '(win|Win)(dow)?(\\(\\))?\\.open\\W': { message: 'Use dom.openWindowDialog', whitelist: ['src/dom.js'], diff --git a/extensions/amp-animation/0.1/test/test-web-animations.js b/extensions/amp-animation/0.1/test/test-web-animations.js index 15d291abceaa..608983b6d762 100644 --- a/extensions/amp-animation/0.1/test/test-web-animations.js +++ b/extensions/amp-animation/0.1/test/test-web-animations.js @@ -15,6 +15,7 @@ */ import {Builder} from '../web-animations'; import {NativeWebAnimationRunner} from '../runners/native-web-animation-runner'; +import {Services} from '../../../../src/services'; import {WebAnimationPlayState} from '../web-animation-types'; import {isArray, isObject} from '../../../../src/types'; import {poll} from '../../../../testing/iframe'; @@ -55,13 +56,13 @@ describes.realWin('MeasureScanner', {amp: 1}, env => { }); warnStub = sandbox.stub(user(), 'warn'); - vsync = win.__AMP_SERVICES.vsync.obj; + vsync = Services.vsyncFor(win); sandbox.stub(vsync, 'measurePromise').callsFake(callback => { return Promise.resolve(callback()); }); - resources = win.__AMP_SERVICES.resources.obj; - owners = win.__AMP_SERVICES.owners.obj; - requireLayoutSpy = sandbox.spy(resources, 'requireLayout'); + resources = Services.resourcesForDoc(env.ampdoc); + owners = Services.ownersForDoc(env.ampdoc); + requireLayoutSpy = sandbox.spy(owners, 'requireLayout'); target1 = doc.createElement('div'); target1.id = 'target1'; diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index ffc8660748c9..318ca845b799 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -270,6 +270,7 @@ export class ResourcesDef extends MutatorsDef { * @param {boolean} layout * @param {number=} opt_parentPriority * @param {boolean=} opt_forceOutsideViewport + * @package */ scheduleLayoutOrPreload( resource, @@ -2101,14 +2102,7 @@ export class Resources { return true; } - /** - * Schedules layout or preload for the specified resource. - * @param {!Resource} resource - * @param {boolean} layout - * @param {number=} opt_parentPriority - * @param {boolean=} opt_forceOutsideViewport - * @package - */ + /** @override */ scheduleLayoutOrPreload( resource, layout, diff --git a/test/unit/test-resources.js b/test/unit/test-resources.js index d79a20499ae8..c1751cc8bca0 100644 --- a/test/unit/test-resources.js +++ b/test/unit/test-resources.js @@ -453,7 +453,7 @@ describe('Resources', () => { sandbox.stub(resource, 'isDisplayed').returns(true); const measureStub = sandbox.stub(resource, 'measure'); const scheduleStub = sandbox - .stub(resources, 'scheduleLayoutOrPreload_') + .stub(resources, 'scheduleLayoutOrPreload') .callsFake(() => resource.loadPromiseResolve_()); const promise = resources.requireLayout(resource.element); resource.build(); @@ -471,7 +471,7 @@ describe('Resources', () => { const resource = new Resource(1, element, resources); resource.layoutScheduled(); const measureSpy = sandbox.spy(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload_'); + const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); const promise = resources.requireLayout(resource.element); resource.build(); resource.loadPromiseResolve_(); @@ -489,7 +489,7 @@ describe('Resources', () => { const resource = new Resource(1, element, resources); sandbox.stub(resource, 'isDisplayed').returns(false); const measureStub = sandbox.stub(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload_'); + const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); const promise = resources.requireLayout(resource.element); resource.build(); return Promise.all([promise, resource.whenBuilt()]).then(() => { @@ -506,7 +506,7 @@ describe('Resources', () => { const resource = new Resource(1, element, resources); resource.layoutComplete_(true); const measureSpy = sandbox.spy(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload_'); + const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); const promise = resources.requireLayout(resource.element); resource.build(); return promise.then(() => { @@ -1116,7 +1116,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { .returns(VisibilityState.VISIBLE); viewportMock.expects('getRect').returns(layoutRectLtwh(0, 0, 300, 400)); const setInViewport = sandbox.spy(resource1, 'setInViewport'); - const schedule = sandbox.spy(resources, 'scheduleLayoutOrPreload_'); + const schedule = sandbox.spy(resources, 'scheduleLayoutOrPreload'); resources.discoverWork_(); From a8aa81a77275c6f28a602b4d23677e9eb45e67f0 Mon Sep 17 00:00:00 2001 From: lannka Date: Wed, 28 Aug 2019 14:33:51 -0700 Subject: [PATCH 5/9] presubmit --- build-system/tasks/presubmit-checks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index fc63228cab95..ece6c0b26dcd 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -612,8 +612,8 @@ const forbiddenTerms = { ], }, '\\.scheduleLayoutOrPreload\\(': { - message: 'findResourcesInElements is a restricted API.', - whitelist: ['src/service/owners-impl.js'], + message: 'scheduleLayoutOrPreload is a restricted API.', + whitelist: ['src/service/owners-impl.js', 'src/service/resources-impl.js'], }, '(win|Win)(dow)?(\\(\\))?\\.open\\W': { message: 'Use dom.openWindowDialog', From 9f6136ad7e62e45937d3fb28a755fd777182da0e Mon Sep 17 00:00:00 2001 From: lannka Date: Thu, 29 Aug 2019 09:14:39 -0700 Subject: [PATCH 6/9] fix test-resources.js --- test/unit/test-resources.js | 91 +------------------------------------ 1 file changed, 1 insertion(+), 90 deletions(-) diff --git a/test/unit/test-resources.js b/test/unit/test-resources.js index c1751cc8bca0..d1c0ad5a0068 100644 --- a/test/unit/test-resources.js +++ b/test/unit/test-resources.js @@ -48,24 +48,6 @@ describe('Resources', () => { resources.pass_.cancel(); }); - function createAmpElement() { - const element = document.createElement('div'); - element.classList.add('i-amphtml-element'); - const signals = new Signals(); - element.signals = () => signals; - element.whenBuilt = () => Promise.resolve(); - element.isBuilt = () => true; - element.build = () => Promise.resolve(); - element.isUpgraded = () => true; - element.updateLayoutBox = () => {}; - element.getPlaceholder = () => null; - element.getLayoutPriority = () => LayoutPriority.CONTENT; - element.dispatchCustomEvent = () => {}; - element.getLayout = () => 'fixed'; - document.body.appendChild(element); - return element; - } - it('should calculate correct calcTaskScore', () => { const viewportRect = layoutRectLtwh(0, 100, 300, 400); sandbox.stub(resources.viewport_, 'getRect').callsFake(() => viewportRect); @@ -444,77 +426,6 @@ describe('Resources', () => { } ); - it('should require layout for non-scheduled element', () => { - const element = createAmpElement(); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 100, 100)); - const resource = new Resource(1, element, resources); - sandbox.stub(resource, 'isDisplayed').returns(true); - const measureStub = sandbox.stub(resource, 'measure'); - const scheduleStub = sandbox - .stub(resources, 'scheduleLayoutOrPreload') - .callsFake(() => resource.loadPromiseResolve_()); - const promise = resources.requireLayout(resource.element); - resource.build(); - return Promise.all([promise, resource.whenBuilt()]).then(() => { - expect(scheduleStub).to.be.calledOnce; - expect(measureStub).to.be.calledOnce; - }); - }); - - it('should require layout for scheduled element', () => { - const element = createAmpElement(); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 100, 100)); - const resource = new Resource(1, element, resources); - resource.layoutScheduled(); - const measureSpy = sandbox.spy(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); - const promise = resources.requireLayout(resource.element); - resource.build(); - resource.loadPromiseResolve_(); - return Promise.all([promise, element.whenBuilt()]).then(() => { - expect(scheduleStub).to.not.be.called; - expect(measureSpy).to.not.be.called; - }); - }); - - it('should not require layout for undisplayed element', () => { - const element = createAmpElement(); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 0, 0)); - const resource = new Resource(1, element, resources); - sandbox.stub(resource, 'isDisplayed').returns(false); - const measureStub = sandbox.stub(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); - const promise = resources.requireLayout(resource.element); - resource.build(); - return Promise.all([promise, resource.whenBuilt()]).then(() => { - expect(scheduleStub).to.not.be.called; - expect(measureStub).to.be.calledOnce; - }); - }); - - it('should not require layout for already completed element', () => { - const element = createAmpElement(); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 0, 0)); - const resource = new Resource(1, element, resources); - resource.layoutComplete_(true); - const measureSpy = sandbox.spy(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); - const promise = resources.requireLayout(resource.element); - resource.build(); - return promise.then(() => { - expect(scheduleStub).to.not.be.called; - expect(measureSpy).to.not.be.called; - }); - }); - it('should update priority and schedule pass', () => { const element = document.createElement('div'); element.isBuilt = () => true; @@ -766,7 +677,7 @@ describes.realWin('Resources discoverWork', {amp: true}, env => { resource1 = createResource(1, layoutRectLtwh(10, 10, 100, 100)); resource2 = createResource(2, layoutRectLtwh(10, 1010, 100, 100)); - resources.owners_ = [resource1, resource2]; + resources.resources_ = [resource1, resource2]; resources.vsync_ = { mutate: callback => callback(), measurePromise: callback => Promise.resolve(callback()), From 67bd61afa408738ed9ea8f532bdcdc656a403198 Mon Sep 17 00:00:00 2001 From: lannka Date: Thu, 29 Aug 2019 10:55:22 -0700 Subject: [PATCH 7/9] refactor owners tests --- test/unit/test-owners.js | 289 +++++++++++++++------------------------ 1 file changed, 114 insertions(+), 175 deletions(-) diff --git a/test/unit/test-owners.js b/test/unit/test-owners.js index 662beb03c670..f2883259544f 100644 --- a/test/unit/test-owners.js +++ b/test/unit/test-owners.js @@ -14,15 +14,11 @@ * limitations under the License. */ -import {LayoutPriority} from '../../src/layout'; -import {Owners} from '../../src/service/owners-impl'; import {Resource, ResourceState} from '../../src/service/resource'; -import {Signals} from '../../src/utils/signals'; -import {layoutRectLtwh} from '../../src/layout-rect'; +import {Services} from '../../src/services'; -/*eslint "google-camelcase/google-camelcase": 0*/ describes.realWin( - 'Resources pause/resume/unlayout scheduling', + 'owners-impl', { amp: true, }, @@ -35,54 +31,54 @@ describes.realWin( let child0; let child1; let child2; + let sandbox; + let scheduleLayoutOrPreloadStub; beforeEach(() => { win = env.win; doc = win.document; - owners = new Owners(env.ampdoc); - resources = owners.resources_; + sandbox = env.sandbox; + owners = Services.ownersForDoc(env.ampdoc); + resources = Services.resourcesForDoc(env.ampdoc); resources.isRuntimeOn_ = false; - const parentTuple = createElementWithResource(1); - parent = parentTuple[0]; + parent = createElementWithResource(1); + doc.body.appendChild(parent); child0 = doc.createElement('div'); - child1 = createElementWithResource(2)[0]; - child2 = createElementWithResource(3)[0]; + child1 = createElementWithResource(2); + child2 = createElementWithResource(3); children = [child0, child1, child2]; children.forEach(child => { parent.appendChild(child); }); + scheduleLayoutOrPreloadStub = sandbox.stub( + resources, + 'scheduleLayoutOrPreload' + ); }); - function createAmpElement() { - const element = document.createElement('div'); - element.classList.add('i-amphtml-element'); - const signals = new Signals(); - element.signals = () => signals; - element.whenBuilt = () => Promise.resolve(); - element.isBuilt = () => true; - element.build = () => Promise.resolve(); - element.isUpgraded = () => true; - element.updateLayoutBox = () => {}; - element.getPlaceholder = () => null; - element.getLayoutPriority = () => LayoutPriority.CONTENT; - element.dispatchCustomEvent = () => {}; - element.getLayout = () => 'fixed'; - document.body.appendChild(element); - return element; - } - function createElement() { const element = env.createAmpElement('amp-test'); - sandbox.stub(element, 'isBuilt').callsFake(() => true); + element.ampdoc_ = env.ampdoc; + element.isUpgradedForTesting = true; + element.isBuiltForTesting = true; + sandbox.stub(element, 'isBuilt').returns(element.isUpgradedForTesting); + sandbox.stub(element, 'isUpgraded').returns(element.isBuiltForTesting); return element; } - function createElementWithResource(id) { + function createElementWithResource( + id, + state = ResourceState.LAYOUT_COMPLETE + ) { const element = createElement(); const resource = new Resource(id, element, resources); - resource.state_ = ResourceState.LAYOUT_COMPLETE; - resource.element['__AMP__RESOURCE'] = resource; - return [element, resource]; + resource.state_ = state; + sandbox.stub(resource, 'measure').callsFake(() => { + resource.state_ = ResourceState.READY_FOR_LAYOUT; + }); + resource.isDisplayed = () => true; + resource.isInViewport = () => true; + return element; } describe('schedulePause', () => { @@ -166,53 +162,32 @@ describes.realWin( }); describe('scheduleLayout', () => { - it('should schedule immediately when resource is READY_FOR_LAYOUT', () => { - const parentElement = createAmpElement(); - const element = createAmpElement(); - parentElement.appendChild(element); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 10, 10)); - new Resource(1, parentElement, resources); - const resource = new Resource(2, element, resources); - resource.state_ = ResourceState.READY_FOR_LAYOUT; - sandbox.stub(resource, 'isDisplayed').returns(true); - sandbox.stub(resource, 'isInViewport').returns(true); - const measureStub = sandbox.stub(resource, 'measure'); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); - owners.scheduleLayout(parentElement, element); - expect(measureStub).to.be.calledOnce; - expect(scheduleStub).to.be.calledOnce; + it('should schedule when resource is READY_FOR_LAYOUT', () => { + const resource1 = resources.getResourceForElement(child1); + resource1.state_ = ResourceState.READY_FOR_LAYOUT; + owners.scheduleLayout(parent, child1); + expect(scheduleLayoutOrPreloadStub).to.be.calledWith( + resource1, + true, + parent.getLayoutPriority() + ); }); - it('should schedule after build', () => { - const parentElement = createAmpElement(); - const element = createAmpElement(); - parentElement.appendChild(element); - sandbox - .stub(element, 'getBoundingClientRect') - .callsFake(() => layoutRectLtwh(0, 0, 10, 10)); - sandbox.stub(element, 'isBuilt').callsFake(() => false); - new Resource(1, parentElement, resources); - const resource = new Resource(2, element, resources); - resource.state_ = ResourceState.NOT_BUILT; - sandbox.stub(resource, 'isDisplayed').returns(true); - const measureStub = sandbox.stub(resource, 'measure').callsFake(() => { - resource.state_ = ResourceState.READY_FOR_LAYOUT; - }); - const scheduleStub = sandbox.stub(resources, 'scheduleLayoutOrPreload'); - owners.scheduleLayout(parentElement, element); - expect(measureStub).to.not.be.called; - expect(scheduleStub).to.not.be.called; - return resource - .build() - .then(() => { - return element.whenBuilt(); + it('should schedule after build', async () => { + child1.isBuiltForTesting = false; + const resource1 = resources.getResourceForElement(child1); + resource1.state_ = ResourceState.NOT_BUILT; + let buildResource; + sandbox.stub(resource1, 'whenBuilt').returns( + new Promise(resolve => { + buildResource = resolve; }) - .then(() => { - expect(measureStub).to.be.calledOnce; - expect(scheduleStub).to.be.calledOnce; - }); + ); + owners.scheduleLayout(parent, child1); + expect(scheduleLayoutOrPreloadStub).to.not.be.called; + buildResource(); + await Promise.resolve(); + expect(scheduleLayoutOrPreloadStub).to.be.calledOnce; }); }); @@ -243,103 +218,67 @@ describes.realWin( expect(stub2.called).to.be.true; }); }); - } -); -describes.realWin('Owners schedulePreload', {amp: true}, env => { - let win, doc; - let resources; - let owners; - let parent; - let children; - let child0; - let child1; - let child2; - - beforeEach(() => { - win = env.win; - doc = win.document; - owners = new Owners(env.ampdoc); - resources = owners.resources_; - resources.isRuntimeOn_ = false; - const parentTuple = createElementWithResource(1); - parent = parentTuple[0]; - child0 = doc.createElement('div'); - child1 = createElementWithResource(2)[0]; - child2 = createElementWithResource(3)[0]; - children = [child0, child1, child2]; - children.forEach(child => { - parent.appendChild(child); - }); - }); + describe('schedulePreload', () => { + beforeEach(() => { + [parent, child1, child2].forEach(element => { + resources.getResourceForElement(element).state_ = + ResourceState.READY_FOR_LAYOUT; + }); + }); - function createElement() { - const element = env.createAmpElement('amp-test'); - sandbox.stub(element, 'isBuilt').callsFake(() => true); - sandbox.stub(element, 'isUpgraded').callsFake(() => true); - return element; - } + it('should not throw with a single element', () => { + expect(() => { + owners.schedulePreload(parent, child1); + }).to.not.throw(); + }); - function createElementWithResource(id) { - const element = createElement(); - const resource = new Resource(id, element, resources); - resource.state_ = ResourceState.READY_FOR_LAYOUT; - resource.element['__AMP__RESOURCE'] = resource; - resource.measure = sandbox.spy(); - resource.isDisplayed = () => true; - resource.isInViewport = () => true; - return [element, resource]; - } + it('should not throw with an array of elements', () => { + expect(() => { + owners.schedulePreload(parent, [child1, child2]); + }).to.not.throw(); + }); + + it('should be ok with non amp children', () => { + expect(() => { + owners.schedulePreload(parent, children); + }).to.not.throw(); + }); - it('should not throw with a single element', () => { - expect(() => { - owners.schedulePreload(parent, child1); - }).to.not.throw(); - }); - - it('should not throw with an array of elements', () => { - expect(() => { - owners.schedulePreload(parent, [child1, child2]); - }).to.not.throw(); - }); - - it('should be ok with non amp children', () => { - expect(() => { - owners.schedulePreload(parent, children); - }).to.not.throw(); - }); - - it('should schedule on custom element with multiple children', () => { - const stub1 = sandbox.stub(resources, 'schedule_'); - owners.schedulePreload(parent, children); - expect(stub1.called).to.be.true; - expect(stub1.callCount).to.be.equal(2); - }); - - it('should schedule on nested custom element placeholder', () => { - const stub1 = sandbox.stub(resources, 'schedule_'); - - const placeholder1 = createElementWithResource(4)[0]; - child1.getPlaceholder = () => placeholder1; - - const placeholder2 = createElementWithResource(5)[0]; - child2.getPlaceholder = () => placeholder2; - - owners.schedulePreload(parent, children); - expect(stub1.called).to.be.true; - expect(stub1.callCount).to.be.equal(4); - }); - - it('should schedule amp-* placeholder inside non-amp element', () => { - const stub1 = sandbox.stub(resources, 'schedule_'); - - const insidePlaceholder1 = createElementWithResource(4)[0]; - const placeholder1 = doc.createElement('div'); - child0.getElementsByClassName = () => [insidePlaceholder1]; - child0.getPlaceholder = () => placeholder1; - - owners.schedulePreload(parent, children); - expect(stub1.called).to.be.true; - expect(stub1.callCount).to.be.equal(3); - }); -}); + it('should schedule on custom element with multiple children', () => { + owners.schedulePreload(parent, children); + expect(scheduleLayoutOrPreloadStub).to.be.calledTwice; + }); + + it('should schedule on nested custom element placeholder', () => { + const placeholder1 = createElementWithResource( + 4, + ResourceState.READY_FOR_LAYOUT + ); + child1.getPlaceholder = () => placeholder1; + + const placeholder2 = createElementWithResource( + 5, + ResourceState.READY_FOR_LAYOUT + ); + child2.getPlaceholder = () => placeholder2; + + owners.schedulePreload(parent, children); + expect(scheduleLayoutOrPreloadStub.callCount).to.equal(4); + }); + + it('should schedule amp-* placeholder inside non-amp element', () => { + const insidePlaceholder1 = createElementWithResource( + 4, + ResourceState.READY_FOR_LAYOUT + ); + const placeholder1 = doc.createElement('div'); + child0.getElementsByClassName = () => [insidePlaceholder1]; + child0.getPlaceholder = () => placeholder1; + + owners.schedulePreload(parent, children); + expect(scheduleLayoutOrPreloadStub).to.be.calledThrice; + }); + }); + } +); From 4a810510b29dc3c637ec03794732af3a603e57a3 Mon Sep 17 00:00:00 2001 From: lannka Date: Thu, 29 Aug 2019 13:00:09 -0700 Subject: [PATCH 8/9] refactor tests --- test/unit/test-owners.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/test-owners.js b/test/unit/test-owners.js index f2883259544f..872d673aadce 100644 --- a/test/unit/test-owners.js +++ b/test/unit/test-owners.js @@ -76,7 +76,10 @@ describes.realWin( sandbox.stub(resource, 'measure').callsFake(() => { resource.state_ = ResourceState.READY_FOR_LAYOUT; }); - resource.isDisplayed = () => true; + resource.isDisplayedForTesting = true; + sandbox + .stub(resource, 'isDisplayed') + .returns(resource.isDisplayedForTesting); resource.isInViewport = () => true; return element; } From a3cbae6b4c6d4e8ce8eb759f35756410a0068fba Mon Sep 17 00:00:00 2001 From: lannka Date: Thu, 29 Aug 2019 16:07:16 -0700 Subject: [PATCH 9/9] rewrite tests for requireLayout --- test/unit/test-owners.js | 203 ++++++++++++++++++++++++++++----------- 1 file changed, 148 insertions(+), 55 deletions(-) diff --git a/test/unit/test-owners.js b/test/unit/test-owners.js index 872d673aadce..87858ffe295a 100644 --- a/test/unit/test-owners.js +++ b/test/unit/test-owners.js @@ -28,9 +28,6 @@ describes.realWin( let owners; let parent; let children; - let child0; - let child1; - let child2; let sandbox; let scheduleLayoutOrPreloadStub; @@ -41,15 +38,24 @@ describes.realWin( owners = Services.ownersForDoc(env.ampdoc); resources = Services.resourcesForDoc(env.ampdoc); resources.isRuntimeOn_ = false; - parent = createElementWithResource(1); + // DOM tree (numbers are resource IDs): + // body -- 0 -- div -- 3 + // -- 4 + // -- 1 + // -- 2 + parent = createElementWithResource(0); doc.body.appendChild(parent); - child0 = doc.createElement('div'); - child1 = createElementWithResource(2); - child2 = createElementWithResource(3); - children = [child0, child1, child2]; - children.forEach(child => { - parent.appendChild(child); - }); + const child0 = doc.createElement('div'); + parent.appendChild(child0); + children = [child0]; + for (let i = 1; i <= 4; ++i) { + children[i] = createElementWithResource(i); + if (i <= 2) { + parent.appendChild(children[i]); + } else { + children[0].appendChild(children[i]); + } + } scheduleLayoutOrPreloadStub = sandbox.stub( resources, 'scheduleLayoutOrPreload' @@ -58,11 +64,7 @@ describes.realWin( function createElement() { const element = env.createAmpElement('amp-test'); - element.ampdoc_ = env.ampdoc; - element.isUpgradedForTesting = true; - element.isBuiltForTesting = true; - sandbox.stub(element, 'isBuilt').returns(element.isUpgradedForTesting); - sandbox.stub(element, 'isUpgraded').returns(element.isBuiltForTesting); + sandbox.stub(element, 'isUpgraded').returns(true); return element; } @@ -79,34 +81,48 @@ describes.realWin( resource.isDisplayedForTesting = true; sandbox .stub(resource, 'isDisplayed') - .returns(resource.isDisplayedForTesting); + .callsFake(() => resource.isDisplayedForTesting); resource.isInViewport = () => true; return element; } + function setAllResourceState(state) { + children.concat([parent]).forEach(element => { + setElementResourceState(element, state); + }); + } + + function setElementResourceState(element, state) { + const resource = resources.getResourceForElementOptional(element); + if (resource) { + resource.state_ = state; + } + return resource; + } + describe('schedulePause', () => { it('should not throw with a single element', () => { expect(() => { - owners.schedulePause(parent, child1); + owners.schedulePause(parent, children[1]); }).to.not.throw(); }); it('should not throw with an array of elements', () => { expect(() => { - owners.schedulePause(parent, [child1, child2]); + owners.schedulePause(parent, [children[1], children[2]]); }).to.not.throw(); }); it('should be ok with non amp children', () => { expect(() => { owners.schedulePause(parent, children); - owners.schedulePause(parent, child0); + owners.schedulePause(parent, children[0]); }).to.not.throw(); }); it('should call pauseCallback on custom element', () => { - const stub1 = sandbox.stub(child1, 'pauseCallback'); - const stub2 = sandbox.stub(child2, 'pauseCallback'); + const stub1 = sandbox.stub(children[1], 'pauseCallback'); + const stub2 = sandbox.stub(children[2], 'pauseCallback'); owners.schedulePause(parent, children); expect(stub1.calledOnce).to.be.true; @@ -114,9 +130,9 @@ describes.realWin( }); it('should call unlayoutCallback when unlayoutOnPause', () => { - const stub1 = sandbox.stub(child1, 'unlayoutCallback'); - const stub2 = sandbox.stub(child2, 'unlayoutCallback'); - sandbox.stub(child1, 'unlayoutOnPause').returns(true); + const stub1 = sandbox.stub(children[1], 'unlayoutCallback'); + const stub2 = sandbox.stub(children[2], 'unlayoutCallback'); + sandbox.stub(children[1], 'unlayoutOnPause').returns(true); owners.schedulePause(parent, children); expect(stub1.calledOnce).to.be.true; @@ -127,37 +143,37 @@ describes.realWin( describe('scheduleResume', () => { beforeEach(() => { // Pause one child. - owners.schedulePause(parent, child1); + owners.schedulePause(parent, children[1]); }); it('should not throw with a single element', () => { expect(() => { - owners.scheduleResume(parent, child1); + owners.scheduleResume(parent, children[1]); }).to.not.throw(); }); it('should not throw with an array of elements', () => { expect(() => { - owners.scheduleResume(parent, [child1, child2]); + owners.scheduleResume(parent, [children[1], children[2]]); }).to.not.throw(); }); it('should be ok with non amp children', () => { expect(() => { owners.scheduleResume(parent, children); - owners.scheduleResume(parent, child0); + owners.scheduleResume(parent, children[0]); }).to.not.throw(); }); it('should call resumeCallback on paused custom elements', () => { - const stub1 = sandbox.stub(child1, 'resumeCallback'); + const stub1 = sandbox.stub(children[1], 'resumeCallback'); owners.scheduleResume(parent, children); expect(stub1.calledOnce).to.be.true; }); it('should call resumeCallback on non-paused custom elements', () => { - const stub2 = sandbox.stub(child2, 'resumeCallback'); + const stub2 = sandbox.stub(children[2], 'resumeCallback'); owners.scheduleResume(parent, children); expect(stub2.calledOnce).to.be.true; @@ -166,9 +182,11 @@ describes.realWin( describe('scheduleLayout', () => { it('should schedule when resource is READY_FOR_LAYOUT', () => { - const resource1 = resources.getResourceForElement(child1); - resource1.state_ = ResourceState.READY_FOR_LAYOUT; - owners.scheduleLayout(parent, child1); + const resource1 = setElementResourceState( + children[1], + ResourceState.READY_FOR_LAYOUT + ); + owners.scheduleLayout(parent, children[1]); expect(scheduleLayoutOrPreloadStub).to.be.calledWith( resource1, true, @@ -177,33 +195,34 @@ describes.realWin( }); it('should schedule after build', async () => { - child1.isBuiltForTesting = false; - const resource1 = resources.getResourceForElement(child1); - resource1.state_ = ResourceState.NOT_BUILT; + const resource1 = setElementResourceState( + children[1], + ResourceState.NOT_BUILT + ); let buildResource; sandbox.stub(resource1, 'whenBuilt').returns( new Promise(resolve => { buildResource = resolve; }) ); - owners.scheduleLayout(parent, child1); + owners.scheduleLayout(parent, children[1]); expect(scheduleLayoutOrPreloadStub).to.not.be.called; buildResource(); await Promise.resolve(); - expect(scheduleLayoutOrPreloadStub).to.be.calledOnce; + expect(scheduleLayoutOrPreloadStub).to.be.calledWith(resource1, true); }); }); describe('scheduleUnlayout', () => { it('should not throw with a single element', () => { expect(() => { - owners.scheduleUnlayout(parent, child1); + owners.scheduleUnlayout(parent, children[1]); }).to.not.throw(); }); it('should not throw with an array of elements', () => { expect(() => { - owners.scheduleUnlayout(parent, [child1, child2]); + owners.scheduleUnlayout(parent, [children[1], children[2]]); }).to.not.throw(); }); @@ -214,8 +233,8 @@ describes.realWin( }); it('should schedule on custom element with multiple children', () => { - const stub1 = sandbox.stub(child1, 'unlayoutCallback'); - const stub2 = sandbox.stub(child2, 'unlayoutCallback'); + const stub1 = sandbox.stub(children[1], 'unlayoutCallback'); + const stub2 = sandbox.stub(children[2], 'unlayoutCallback'); owners.scheduleUnlayout(parent, children); expect(stub1.called).to.be.true; expect(stub2.called).to.be.true; @@ -224,21 +243,21 @@ describes.realWin( describe('schedulePreload', () => { beforeEach(() => { - [parent, child1, child2].forEach(element => { - resources.getResourceForElement(element).state_ = - ResourceState.READY_FOR_LAYOUT; + setAllResourceState(ResourceState.NOT_BUILT); + [parent, children[1], children[2]].forEach(element => { + setElementResourceState(element, ResourceState.READY_FOR_LAYOUT); }); }); it('should not throw with a single element', () => { expect(() => { - owners.schedulePreload(parent, child1); + owners.schedulePreload(parent, children[1]); }).to.not.throw(); }); it('should not throw with an array of elements', () => { expect(() => { - owners.schedulePreload(parent, [child1, child2]); + owners.schedulePreload(parent, [children[1], children[2]]); }).to.not.throw(); }); @@ -255,16 +274,16 @@ describes.realWin( it('should schedule on nested custom element placeholder', () => { const placeholder1 = createElementWithResource( - 4, + 10, ResourceState.READY_FOR_LAYOUT ); - child1.getPlaceholder = () => placeholder1; + children[1].getPlaceholder = () => placeholder1; const placeholder2 = createElementWithResource( - 5, + 11, ResourceState.READY_FOR_LAYOUT ); - child2.getPlaceholder = () => placeholder2; + children[2].getPlaceholder = () => placeholder2; owners.schedulePreload(parent, children); expect(scheduleLayoutOrPreloadStub.callCount).to.equal(4); @@ -272,16 +291,90 @@ describes.realWin( it('should schedule amp-* placeholder inside non-amp element', () => { const insidePlaceholder1 = createElementWithResource( - 4, + 10, ResourceState.READY_FOR_LAYOUT ); const placeholder1 = doc.createElement('div'); - child0.getElementsByClassName = () => [insidePlaceholder1]; - child0.getPlaceholder = () => placeholder1; + children[0].getElementsByClassName = () => [insidePlaceholder1]; + children[0].getPlaceholder = () => placeholder1; owners.schedulePreload(parent, children); expect(scheduleLayoutOrPreloadStub).to.be.calledThrice; }); }); + + describe('requireLayout', () => { + beforeEach(() => { + children.concat([parent]).forEach(element => { + const resource = resources.getResourceForElementOptional(element); + if (!resource) { + return; + } + sandbox.stub(resource, 'whenBuilt').returns(Promise.resolve()); + sandbox.stub(resource, 'loadedOnce').returns(Promise.resolve()); + }); + }); + + it('should layout AMP element itself', async () => { + setAllResourceState(ResourceState.READY_FOR_LAYOUT); + const scheduledElements = await owners.requireLayout(parent); + expect(scheduledElements).to.have.length(1); + expect(scheduleLayoutOrPreloadStub).to.be.calledOnce; + expect(scheduleLayoutOrPreloadStub).to.be.calledWith( + resources.getResourceForElement(parent), + true + ); + }); + + it("should layout non-AMP element's all AMP children", async () => { + setAllResourceState(ResourceState.READY_FOR_LAYOUT); + const scheduledElements = await owners.requireLayout(children[0]); + expect(scheduledElements).to.have.length(2); + expect(scheduleLayoutOrPreloadStub).to.be.calledTwice; + expect(scheduleLayoutOrPreloadStub).to.be.calledWith( + resources.getResourceForElement(children[3]), + true + ); + expect(scheduleLayoutOrPreloadStub).to.be.calledWith( + resources.getResourceForElement(children[4]), + true + ); + }); + + it('should layout element w/ state=LAYOUT_FAILED', async () => { + const resource = setElementResourceState( + parent, + ResourceState.LAYOUT_FAILED + ); + await owners.requireLayout(parent); + expect(scheduleLayoutOrPreloadStub).to.be.calledOnce; + expect(scheduleLayoutOrPreloadStub).to.be.calledWith(resource, true); + }); + + it('should not layout element w/ state=LAYOUT_COMPLETE', async () => { + setElementResourceState(parent, ResourceState.LAYOUT_COMPLETE); + const scheduledElements = await owners.requireLayout(parent); + expect(scheduledElements).to.have.length(0); + expect(scheduleLayoutOrPreloadStub).to.not.be.called; + }); + + it('should not double schedule element w/ state=LAYOUT_SCHEDULED', async () => { + setElementResourceState(parent, ResourceState.LAYOUT_SCHEDULED); + const scheduledElements = await owners.requireLayout(parent); + expect(scheduledElements).to.have.length(1); + expect(scheduleLayoutOrPreloadStub).to.not.be.called; + }); + + it('should not require layout for undisplayed element', async () => { + const resource = setElementResourceState( + parent, + ResourceState.READY_FOR_LAYOUT + ); + resource.isDisplayedForTesting = false; + const scheduledElements = await owners.requireLayout(parent); + expect(scheduledElements).to.have.length(1); + expect(scheduleLayoutOrPreloadStub).to.not.be.called; + }); + }); } );