diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js index 3e919cc0ce4e..520ebf587a0b 100644 --- a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js +++ b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js @@ -332,51 +332,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { this.inZIndexHoldBack_ = false; } - /** - * @return {number|boolean} render on idle configuration with false - * indicating disabled. - * @private - */ - getIdleRenderEnabled_() { - if (this.isIdleRender_) { - return this.isIdleRender_; - } - // Disable if publisher has indicated a non-default loading strategy. - if (this.element.getAttribute('data-loading-strategy')) { - return false; - } - const expVal = this.postAdResponseExperimentFeatures['render-idle-vp']; - const vpRange = parseInt(expVal, 10); - if (expVal && isNaN(vpRange)) { - // holdback branch sends non-numeric value. - return false; - } - return vpRange || 12; - } - - /** @override */ - idleRenderOutsideViewport() { - const vpRange = this.getIdleRenderEnabled_(); - if (vpRange === false) { - return vpRange; - } - const renderOutsideViewport = this.renderOutsideViewport(); - // False will occur when throttle in effect. - if (typeof renderOutsideViewport === 'boolean') { - return renderOutsideViewport; - } - this.isIdleRender_ = true; - // NOTE(keithwrightbos): handle race condition where previous - // idleRenderOutsideViewport marked slot as idle render despite never - // being schedule due to being beyond viewport max offset. If slot - // comes within standard outside viewport range, then ensure throttling - // will not be applied. - this.getResource() - .whenWithinViewport(renderOutsideViewport) - .then(() => (this.isIdleRender_ = false)); - return vpRange; - } - /** @override */ isLayoutSupported(layout) { this.isFluidPrimaryRequest_ = layout == Layout.FLUID; diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js index d5497b6b0747..dea06639c134 100644 --- a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js +++ b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js @@ -1693,100 +1693,6 @@ describes.realWin( }); }); - describe('#idleRenderOutsideViewport', () => { - beforeEach(() => { - element = createElementWithAttributes(doc, 'amp-ad', { - 'width': '200', - 'height': '50', - 'type': 'doubleclick', - }); - impl = new AmpAdNetworkDoubleclickImpl(element); - env.sandbox - .stub(impl, 'getResource') - .returns({whenWithinViewport: () => Promise.resolve()}); - }); - - it('should use experiment value', () => { - impl.postAdResponseExperimentFeatures['render-idle-vp'] = '4'; - expect(impl.idleRenderOutsideViewport()).to.equal(4); - expect(impl.isIdleRender_).to.be.true; - }); - - it('should return false if using loading strategy', () => { - impl.postAdResponseExperimentFeatures['render-idle-vp'] = '4'; - impl.element.setAttribute( - 'data-loading-strategy', - 'prefer-viewability-over-views' - ); - expect(impl.idleRenderOutsideViewport()).to.be.false; - expect(impl.isIdleRender_).to.be.false; - }); - - it('should return false if invalid experiment value', () => { - impl.postAdResponseExperimentFeatures['render-idle-vp'] = 'abc'; - expect(impl.idleRenderOutsideViewport()).to.be.false; - }); - - it('should return 12 if no experiment header', () => { - expect(impl.idleRenderOutsideViewport()).to.equal(12); - }); - - it('should return renderOutsideViewport boolean', () => { - env.sandbox.stub(impl, 'renderOutsideViewport').returns(false); - expect(impl.idleRenderOutsideViewport()).to.be.false; - }); - }); - - describe('idle renderNonAmpCreative', () => { - beforeEach(() => { - element = createElementWithAttributes(doc, 'amp-ad', { - 'width': '200', - 'height': '50', - 'type': 'doubleclick', - }); - impl = new AmpAdNetworkDoubleclickImpl(element); - impl.postAdResponseExperimentFeatures['render-idle-vp'] = '4'; - impl.postAdResponseExperimentFeatures['render-idle-throttle'] = 'true'; - env.sandbox - .stub(AmpA4A.prototype, 'renderNonAmpCreative') - .returns(Promise.resolve()); - }); - - // TODO(jeffkaufman, #13422): this test was silently failing - it.skip('should throttle if idle render and non-AMP creative', () => { - impl.win['3pla'] = 1; - const startTime = Date.now(); - return impl.renderNonAmpCreative().then(() => { - expect(Date.now() - startTime).to.be.at.least(1000); - }); - }); - - it('should NOT throttle if idle experiment not enabled', () => { - impl.win['3pla'] = 1; - delete impl.postAdResponseExperimentFeatures['render-idle-vp']; - const startTime = Date.now(); - return impl.renderNonAmpCreative().then(() => { - expect(Date.now() - startTime).to.be.at.most(50); - }); - }); - - it('should NOT throttle if experiment throttle not enabled', () => { - impl.win['3pla'] = 1; - const startTime = Date.now(); - return impl.renderNonAmpCreative().then(() => { - expect(Date.now() - startTime).to.be.at.most(50); - }); - }); - - it('should NOT throttle if idle render and no previous', () => { - impl.win['3pla'] = 0; - const startTime = Date.now(); - return impl.renderNonAmpCreative().then(() => { - expect(Date.now() - startTime).to.be.at.most(50); - }); - }); - }); - describe('#preconnect', () => { beforeEach(() => { element = createElementWithAttributes(doc, 'amp-ad', { diff --git a/src/base-element.js b/src/base-element.js index a250f0086726..b9515b0b0af8 100644 --- a/src/base-element.js +++ b/src/base-element.js @@ -429,17 +429,6 @@ export class BaseElement { return getMode(this.win).runtime == 'inabox' || 3; } - /** - * Allows for rendering outside of the constraint set by renderOutsideViewport - * so long task scheduler is idle. Integer values less than those returned - * by renderOutsideViewport have no effect. Subclasses can override (default - * is disabled). - * @return {boolean|number} - */ - idleRenderOutsideViewport() { - return false; - } - /** * Subclasses can override this method to opt-in into receiving additional * {@link layoutCallback} calls. Note that this method is not consulted for diff --git a/src/custom-element.js b/src/custom-element.js index 68f29d788a82..78ebdc2bc712 100644 --- a/src/custom-element.js +++ b/src/custom-element.js @@ -1021,16 +1021,6 @@ function createBaseCustomElementClass(win) { return this.implementation_.renderOutsideViewport(); } - /** - * Whether the element should render outside of renderOutsideViewport when - * the scheduler is idle. - * @return {boolean|number} - * @final - */ - idleRenderOutsideViewport() { - return this.implementation_.idleRenderOutsideViewport(); - } - /** * Returns a previously measured layout box adjusted to the viewport. This * mainly affects fixed-position elements that are adjusted to be always diff --git a/src/service/resource.js b/src/service/resource.js index 7cfffa5749bf..7635d2989654 100644 --- a/src/service/resource.js +++ b/src/service/resource.js @@ -844,15 +844,6 @@ export class Resource { ); } - /** - * Whether this is allowed to render when scheduler is idle but not in - * viewport. - * @return {boolean} - */ - idleRenderOutsideViewport() { - return this.isWithinViewportRatio(this.element.idleRenderOutsideViewport()); - } - /** * Sets the resource's state to LAYOUT_SCHEDULED. * @param {number} scheduleTime The time at which layout was scheduled. diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index b8b8c67eb47a..7acf88fd281c 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -1348,27 +1348,8 @@ export class ResourcesImpl { this.queue_.getSize() == 0 && now > this.exec_.getLastDequeueTime() + 5000 ) { - // Phase 5: Idle Render Outside Viewport layout: layout up to 4 items - // with idleRenderOutsideViewport true let idleScheduledCount = 0; - for ( - let i = 0; - i < this.resources_.length && idleScheduledCount < 4; - i++ - ) { - const r = this.resources_[i]; - if ( - r.getState() == ResourceState.READY_FOR_LAYOUT && - !r.hasOwner() && - r.isDisplayed() && - r.idleRenderOutsideViewport() - ) { - dev().fine(TAG_, 'idleRenderOutsideViewport layout:', r.debugid); - this.scheduleLayoutOrPreload(r, /* layout */ false); - idleScheduledCount++; - } - } - // Phase 6: Idle layout: layout more if we are otherwise not doing much. + // Phase 5: Idle layout: layout more if we are otherwise not doing much. // TODO(dvoytenko): document/estimate IDLE timeouts and other constants for ( let i = 0; @@ -1646,8 +1627,7 @@ export class ResourcesImpl { if ( !forceOutsideViewport && !resource.isInViewport() && - !resource.renderOutsideViewport() && - !resource.idleRenderOutsideViewport() + !resource.renderOutsideViewport() ) { return false; } diff --git a/test/unit/test-resource.js b/test/unit/test-resource.js index 9b1af57f6662..019929c3fa5a 100644 --- a/test/unit/test-resource.js +++ b/test/unit/test-resource.js @@ -990,59 +990,6 @@ describes.realWin('Resource', {amp: true}, (env) => { }); }); -describe('Resource idleRenderOutsideViewport', () => { - let element; - let resources; - let resource; - let idleRenderOutsideViewport; - let isWithinViewportRatio; - - beforeEach(() => { - idleRenderOutsideViewport = window.sandbox.stub(); - element = { - idleRenderOutsideViewport, - ownerDocument: {defaultView: window}, - tagName: 'AMP-AD', - hasAttribute: () => false, - isBuilt: () => false, - isBuilding: () => false, - isUpgraded: () => false, - prerenderAllowed: () => false, - renderOutsideViewport: () => true, - build: () => false, - getBoundingClientRect: () => null, - updateLayoutBox: () => {}, - isRelayoutNeeded: () => false, - layoutCallback: () => {}, - applySize: () => {}, - unlayoutOnPause: () => false, - unlayoutCallback: () => true, - pauseCallback: () => false, - resumeCallback: () => false, - viewportCallback: () => {}, - getLayoutPriority: () => LayoutPriority.CONTENT, - }; - resources = new ResourcesImpl(new AmpDocSingle(window)); - resource = new Resource(1, element, resources); - isWithinViewportRatio = window.sandbox.stub( - resource, - 'isWithinViewportRatio' - ); - }); - - it('should return true if isWithinViewportRatio', () => { - idleRenderOutsideViewport.returns(5); - isWithinViewportRatio.withArgs(5).returns(true); - expect(resource.idleRenderOutsideViewport()).to.equal(true); - }); - - it('should return false for false element idleRenderOutsideViewport', () => { - idleRenderOutsideViewport.returns(false); - isWithinViewportRatio.withArgs(false).returns(false); - expect(resource.idleRenderOutsideViewport()).to.equal(false); - }); -}); - describes.realWin('Resource renderOutsideViewport', {amp: true}, (env) => { let element; let resources; diff --git a/test/unit/test-resources.js b/test/unit/test-resources.js index d95223e96091..af53fe72767a 100644 --- a/test/unit/test-resources.js +++ b/test/unit/test-resources.js @@ -336,7 +336,6 @@ describe('Resources', () => { isInViewport: () => false, prerenderAllowed: () => true, renderOutsideViewport: () => false, - idleRenderOutsideViewport: () => false, startLayout: () => {}, applySizesAndMediaQuery: () => {}, }; @@ -356,7 +355,6 @@ describe('Resources', () => { isInViewport: () => false, prerenderAllowed: () => true, renderOutsideViewport: () => false, - idleRenderOutsideViewport: () => false, getLayoutPriority: () => LayoutPriority.METADATA, startLayout: () => {}, layoutScheduled: () => {}, @@ -385,31 +383,6 @@ describe('Resources', () => { isInViewport: () => false, prerenderAllowed: () => true, renderOutsideViewport: () => true, - idleRenderOutsideViewport: () => false, - getLayoutPriority: () => LayoutPriority.METADATA, - startLayout: () => {}, - layoutScheduled: () => {}, - getTaskId: () => 'resource#L', - applySizesAndMediaQuery: () => {}, - }; - resources.scheduleLayoutOrPreload(resource, true); - expect(resources.queue_.getSize()).to.equal(1); - expect(resources.queue_.tasks_[0].forceOutsideViewport).to.be.false; - } - ); - - it( - 'should schedule idleRenderOutsideViewport resource when' + - ' resource is not visible', - () => { - const resource = { - getState: () => ResourceState.READY_FOR_LAYOUT, - isDisplayed: () => true, - isFixed: () => false, - isInViewport: () => false, - prerenderAllowed: () => true, - renderOutsideViewport: () => false, - idleRenderOutsideViewport: () => true, getLayoutPriority: () => LayoutPriority.METADATA, startLayout: () => {}, layoutScheduled: () => {}, @@ -612,8 +585,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { element.getLayoutPriority = () => LayoutPriority.CONTENT; element.dispatchCustomEvent = () => {}; element.getLayout = () => 'fixed'; - - element.idleRenderOutsideViewport = () => true; element.isInViewport = () => false; element.getAttribute = () => null; element.hasAttribute = () => false; @@ -1001,7 +972,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { const layoutCanceledSpy = sandbox.spy(resource1, 'layoutCanceled'); sandbox.stub(resource1, 'isInViewport').returns(false); sandbox.stub(resource1, 'renderOutsideViewport').returns(false); - sandbox.stub(resource1, 'idleRenderOutsideViewport').returns(false); resources.work_(); expect(resources.exec_.getSize()).to.equal(0); expect(measureSpy).to.be.calledOnce; @@ -1022,7 +992,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { const measureSpy = sandbox.spy(resource1, 'measure'); sandbox.stub(resource1, 'isInViewport').returns(false); sandbox.stub(resource1, 'renderOutsideViewport').returns(false); - sandbox.stub(resource1, 'idleRenderOutsideViewport').returns(false); resources.work_(); expect(resources.exec_.getSize()).to.equal(1); expect(measureSpy).to.be.calledOnce; @@ -1113,7 +1082,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { .returns(true) .onSecondCall() .returns(false); - resource2.element.idleRenderOutsideViewport = () => false; resource1.state_ = ResourceState.NOT_BUILT; resource1.build = sandbox.spy(); @@ -1137,7 +1105,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { .returns(false) .onSecondCall() .returns(true); - resource2.element.idleRenderOutsideViewport = () => false; resource1.state_ = ResourceState.NOT_BUILT; resource1.build = sandbox.spy(); @@ -1158,7 +1125,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { resource1.prerenderAllowed = () => false; resource1.state_ = ResourceState.NOT_BUILT; resource1.build = sandbox.spy(); - resource2.element.idleRenderOutsideViewport = () => false; resources.discoverWork_(); @@ -1172,7 +1138,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { resources.buildAttemptsCount_ = 21; // quota is 20 resource1.element.isBuilt = () => false; - resource1.element.idleRenderOutsideViewport = () => true; resource1.prerenderAllowed = () => true; resource1.isBuildRenderBlocking = () => false; resource1.state_ = ResourceState.NOT_BUILT; @@ -1189,7 +1154,6 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { resources.buildAttemptsCount_ = 21; // quota is 20 resource1.element.isBuilt = () => false; - resource1.element.idleRenderOutsideViewport = () => true; resource1.prerenderAllowed = () => true; resource1.isBuildRenderBlocking = () => true; resource1.state_ = ResourceState.NOT_BUILT; @@ -1199,29 +1163,12 @@ describes.realWin('Resources discoverWork', {amp: true}, (env) => { expect(resource1.build).to.be.called; }); - it('should layout resource if outside viewport but idle', () => { - const schedulePassStub = sandbox.stub(resources, 'schedulePass'); - resources.documentReady_ = true; - sandbox.stub(resource1.element, 'nextSibling').returns({}); - resource1.element.isBuilt = () => true; - resource1.element.renderOutsideViewport = () => false; - resource1.element.idleRenderOutsideViewport = () => true; - resource2.element.renderOutsideViewport = () => false; - resource2.element.idleRenderOutsideViewport = () => false; - resource1.state_ = ResourceState.READY_FOR_LAYOUT; - - resources.discoverWork_(); - - expect(schedulePassStub).to.be.calledOnce; - }); - it('should force build resources during discoverWork layout phase', () => { const buildResourceSpy = sandbox.spy(resources, 'buildResourceUnsafe_'); sandbox.stub(resources, 'schedule_'); resources.documentReady_ = true; // Emulates a resource not building. resource1.element.isBuilt = sandbox.stub().returns(false); - resource2.element.idleRenderOutsideViewport = () => false; resource1.state_ = ResourceState.NOT_BUILT; resource1.build = sandbox.spy();