diff --git a/src/service/performance-impl.js b/src/service/performance-impl.js index f36a8d03e9ec..6faa6804ceb4 100644 --- a/src/service/performance-impl.js +++ b/src/service/performance-impl.js @@ -107,12 +107,10 @@ export class Performance { this.shiftScoresTicked_ = 0; /** - * The sum of all layout shift fractions triggered on the page from the - * Layout Instability API. - * - * @private {number} + * The collection of layout shift events from the Layout Instability API. + * @private {Array} */ - this.aggregateShiftScore_ = 0; + this.layoutShifts_ = []; const supportedEntryTypes = (this.win.PerformanceObserver && @@ -241,11 +239,6 @@ export class Performance { this.ampdoc_.whenFirstVisible().then(() => { this.tick(TickLabel.ON_FIRST_VISIBLE); - // TODO(#33207): Remove after data collection - this.tick( - TickLabel.CUMULATIVE_LAYOUT_SHIFT_BEFORE_VISIBLE, - this.aggregateShiftScore_ - ); this.flush(); }); @@ -355,13 +348,6 @@ export class Performance { this.tickDelta(TickLabel.FIRST_CONTENTFUL_PAINT, value); this.tickSinceVisible(TickLabel.FIRST_CONTENTFUL_PAINT_VISIBLE, value); recordedFirstContentfulPaint = true; - - // TODO(#33207): remove after data collection - // On the first contentful paint, report cumulative CLS score - this.tickDelta( - TickLabel.CUMULATIVE_LAYOUT_SHIFT_BEFORE_FCP, - this.aggregateShiftScore_ - ); } else if ( entry.entryType === 'first-input' && !recordedFirstInputDelay @@ -372,8 +358,9 @@ export class Performance { } else if (entry.entryType === 'layout-shift') { // Ignore layout shift that occurs within 500ms of user input, as it is // likely in response to the user's action. - if (!entry.hadRecentInput) { - this.aggregateShiftScore_ += entry.value; + // 1000 here is a magic number to prevent unbounded growth. We don't expect it to be reached. + if (!entry.hadRecentInput && this.layoutShifts_.length < 1000) { + this.layoutShifts_.push(entry); } } else if (entry.entryType === 'largest-contentful-paint') { if (entry.loadTime) { @@ -524,18 +511,35 @@ export class Performance { * amount of visibility into this metric. */ tickLayoutShiftScore_() { + const cls = this.layoutShifts_.reduce((sum, entry) => sum + entry.value, 0); + const fcp = this.metrics_.get(TickLabel.FIRST_CONTENTFUL_PAINT) ?? 0; // fallback to 0, so that we never overcount. + const ofv = this.metrics_.get(TickLabel.ON_FIRST_VISIBLE) ?? 0; + + // TODO(#33207): Remove after data collection + const clsBeforeFCP = this.layoutShifts_.reduce((sum, entry) => { + if (entry.startTime < fcp) { + return sum + entry.value; + } + return sum; + }, 0); + const clsBeforeOFV = this.layoutShifts_.reduce((sum, entry) => { + if (entry.startTime < ofv) { + return sum + entry.value; + } + return sum; + }, 0); + if (this.shiftScoresTicked_ === 0) { + this.tick(TickLabel.CUMULATIVE_LAYOUT_SHIFT_BEFORE_VISIBLE, clsBeforeOFV); this.tickDelta( - TickLabel.CUMULATIVE_LAYOUT_SHIFT, - this.aggregateShiftScore_ + TickLabel.CUMULATIVE_LAYOUT_SHIFT_BEFORE_FCP, + clsBeforeFCP ); + this.tickDelta(TickLabel.CUMULATIVE_LAYOUT_SHIFT, cls); this.flush(); this.shiftScoresTicked_ = 1; } else if (this.shiftScoresTicked_ === 1) { - this.tickDelta( - TickLabel.CUMULATIVE_LAYOUT_SHIFT_2, - this.aggregateShiftScore_ - ); + this.tickDelta(TickLabel.CUMULATIVE_LAYOUT_SHIFT_2, cls); this.flush(); this.shiftScoresTicked_ = 2; diff --git a/test/unit/test-performance.js b/test/unit/test-performance.js index 1358a5f9a299..ea72dbc02479 100644 --- a/test/unit/test-performance.js +++ b/test/unit/test-performance.js @@ -300,7 +300,7 @@ describes.realWin('performance', {amp: true}, (env) => { ]).then(() => { expect(flushSpy).to.have.callCount(4); expect(perf.isMessagingReady_).to.be.false; - const count = 6; + const count = 5; expect(perf.events_.length).to.equal(count); }); }); @@ -653,15 +653,13 @@ describes.realWin('performance', {amp: true}, (env) => { const initialCount = tickSpy.callCount; return ampdoc.whenFirstVisible().then(() => { clock.tick(400); - // TODO(#33207): reduce call counts by 1 after data collection - expect(tickSpy).to.have.callCount(initialCount + 2); - expect(tickSpy.withArgs('cls-ofv')).to.be.calledOnce; + expect(tickSpy).to.have.callCount(initialCount + 1); whenViewportLayoutCompleteResolve(); return perf.whenViewportLayoutComplete_().then(() => { - expect(tickSpy).to.have.callCount(initialCount + 2); + expect(tickSpy).to.have.callCount(initialCount + 1); expect(tickSpy.withArgs('ofv')).to.be.calledOnce; return whenFirstVisiblePromise.then(() => { - expect(tickSpy).to.have.callCount(initialCount + 3); + expect(tickSpy).to.have.callCount(initialCount + 2); expect(tickSpy.withArgs('pc')).to.be.calledOnce; expect(Number(tickSpy.withArgs('pc').args[0][1])).to.equal(400); }); @@ -851,6 +849,8 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { let windowEventListeners; let performanceObserver; let viewerVisibilityState; + let whenFirstVisiblePromise; + let whenFirstVisibleResolve; function setupFakesForVisibilityStateManipulation() { // Fake window to fake `document.visibilityState`. @@ -900,12 +900,15 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { // Install services on fakeWin so some behaviors can be stubbed. installRuntimeServices(fakeWin); + whenFirstVisiblePromise = new Promise((resolve) => { + whenFirstVisibleResolve = resolve; + }); const unresolvedPromise = new Promise(() => {}); const viewportSize = {width: 0, height: 0}; env.sandbox.stub(Services, 'ampdoc').returns({ hasBeenVisible: () => {}, onVisibilityChanged: () => {}, - whenFirstVisible: () => unresolvedPromise, + whenFirstVisible: () => whenFirstVisiblePromise, getVisibilityState: () => viewerVisibilityState, getFirstVisibleTime: () => 0, isSingleDoc: () => true, @@ -967,7 +970,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { const perf = Services.performanceFor(env.win); - expect(perf.events_.length).to.equal(4); + expect(perf.events_.length).to.equal(3); expect(perf.events_[0]).to.be.jsonEqual( { label: 'fp', @@ -980,10 +983,6 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { { label: 'fcpv', delta: 15, - }, - { - label: 'cls-fcp', - delta: 15, } ); @@ -1033,7 +1032,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { }; // Fake a triggering of the first-input event. performanceObserver.triggerCallback(list); - expect(perf.events_.length).to.equal(4); + expect(perf.events_.length).to.equal(3); expect(perf.events_[0]).to.be.jsonEqual( { label: 'fp', @@ -1046,10 +1045,6 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { { label: 'fcpv', delta: 15, - }, - { - label: 'cls-fcp', - delta: 15, } ); }); @@ -1223,6 +1218,82 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { (windowEventListeners[eventName] || []).forEach((cb) => cb(event)); } + // TODO(#33207): Remove after data collection + it('Forwards cls-fcp and cls-ofv', async () => { + fakeWin.PerformanceObserver.supportedEntryTypes = [ + 'layout-shift', + 'paint', + ]; + fakeWin.document.visibilityState = 'hidden'; + + // Document should be initially hidden. + expect(fakeWin.document.visibilityState).to.equal('hidden'); + + // visibilitychange/beforeunload listeners are now added. + const perf = getPerformance(); + perf.coreServicesAvailable(); + + // Pre-visible and pre-fcp layout shifts + performanceObserver.triggerCallback({ + getEntries() { + return [ + { + entryType: 'layout-shift', + value: 0.25, + hadRecentInput: false, + startTime: 0, + }, + { + entryType: 'layout-shift', + value: 0.25, + hadRecentInput: false, + startTime: 50, + }, + ]; + }, + }); + + whenFirstVisibleResolve(); + await new Promise(setTimeout); + toggleVisibility(fakeWin, true); + + // Post visible, pre-fcp layout-shift + performanceObserver.triggerCallback({ + getEntries() { + return [ + { + entryType: 'layout-shift', + value: 0.5, + hadRecentInput: false, + startTime: 100, + }, + { + entryType: 'paint', + name: 'first-contentful-paint', + startTime: 150, + duration: 0, + }, + { + entryType: 'layout-shift', + value: 0.5, + hadRecentInput: false, + startTime: 200, + }, + ]; + }, + }); + + toggleVisibility(fakeWin, false); + const clsEvents = perf.events_.filter((event) => + event.label.startsWith('cls') + ); + expect(clsEvents).jsonEqual([ + {label: 'cls-ofv', delta: 0.5}, + {label: 'cls-fcp', delta: 1}, + {label: 'cls', delta: 1.5}, + ]); + }); + it('for Chromium 77', () => { // Specify an Android Chrome user agent, which supports the // visibilitychange event. @@ -1258,7 +1329,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { toggleVisibility(fakeWin, false); let clsEvents = perf.events_.filter((event) => event.label === 'cls'); expect(clsEvents.length).equal(1); - expect(perf.events_[0]).to.be.jsonEqual({ + expect(perf.events_).deep.includes({ label: 'cls', delta: 0.55, }); @@ -1285,7 +1356,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { toggleVisibility(fakeWin, false); clsEvents = perf.events_.filter((event) => event.label.startsWith('cls')); - expect(clsEvents.length).to.equal(2); + expect(clsEvents.length).to.equal(4); expect(clsEvents).to.deep.include({ label: 'cls-2', delta: 1.5501, @@ -1301,7 +1372,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { toggleVisibility(fakeWin, false); clsEvents = perf.events_.filter((event) => event.label.startsWith('cls')); - expect(clsEvents.length).to.equal(2); + expect(clsEvents.length).to.equal(4); }); it('when the viewer visibility changes to inactive', () => { @@ -1336,7 +1407,7 @@ describes.realWin('PeformanceObserver metrics', {amp: true}, (env) => { const clsEvents = perf.events_.filter((evt) => evt.label.startsWith('cls') ); - expect(clsEvents.length).to.equal(1); + expect(clsEvents.length).to.equal(3); expect(perf.events_).deep.include({ label: 'cls', delta: 0.55,