diff --git a/src/document-info.js b/src/document-info.js index bcd94a6f03aa..db04fca60980 100644 --- a/src/document-info.js +++ b/src/document-info.js @@ -16,7 +16,7 @@ import {getService} from './service'; import {assert} from './asserts'; -import {parseUrl} from './url'; +import {parseUrl, getSourceUrl} from './url'; /** * @param {!Window} win @@ -24,6 +24,7 @@ import {parseUrl} from './url'; * - canonicalUrl: The doc's canonical. * - pageViewId: Id for this page view. Low entropy but should be unique * for concurrent page views of a user. + * - sourceUrl: the source url of an amp document. */ export function documentInfoFor(win) { return getService(win, 'documentInfo', () => { @@ -33,6 +34,7 @@ export function documentInfoFor(win) { 'AMP files are required to have a tag.') .href).href, pageViewId: getPageViewId(win), + sourceUrl: getSourceUrl(win.location.href), }; }); } diff --git a/src/performance.js b/src/performance.js index fba29bd4e9ad..bbc75555e0e8 100644 --- a/src/performance.js +++ b/src/performance.js @@ -15,6 +15,7 @@ */ import {all} from './promise'; +import {documentInfoFor} from './document-info'; import {documentStateFor} from './document-state'; import {getService} from './service'; import {loadPromise} from './event-helper'; @@ -46,6 +47,24 @@ const ENSURE_NON_ZERO = 1000; class TickEventDef {} +/** + * Increments the value, else defaults to 0 for the given object key. + * @param {!Object} obj + * @param {?string} name + */ +function incOrDef(obj, name) { + if (!name) { + return; + } + + if (!obj[name]) { + obj[name] = 1; + } else { + obj[name]++; + } +} + + /** * Performance holds the mechanism to call `tick` to stamp out important * events in the lifecycle of the AMP runtime. It can hold a small amount @@ -104,6 +123,14 @@ export class Performance { this.viewer_.onVisibilityChanged(this.flush.bind(this)); this.measureUserPerceivedVisualCompletenessTime_(); + this.setDocumentInfoParams_(); + + // forward all queued ticks to the viewer. + this.flushQueuedTicks_(); + // We need to call flush right away in case the viewer is available + // later than the amp codebase had invoked the performance services' + // `flush` method to forward ticks. + this.flush(); } /** @@ -143,6 +170,7 @@ export class Performance { * Returns a promise that is resolved when resources in viewport * have been finished being laid out. * @return {!Promise} + * @private */ whenViewportLayoutComplete_() { return this.whenReadyToRetrieveResources_().then(() => { @@ -162,6 +190,16 @@ export class Performance { return this.whenReadyToRetrieveResourcesPromise_; } + /** + * Forward an object to be appended as search params to the external + * intstrumentation library. + * @param {!JSONObject} params + * @private + */ + setFlushParams_(params) { + this.viewer_.setFlushParams(params); + } + /** * Ticks a timing event. * @@ -173,8 +211,15 @@ export class Performance { * `tickDelta` instead. */ tick(label, opt_from, opt_value) { - if (this.tick_) { - this.tick_(label, opt_from, opt_value); + opt_from = opt_from == undefined ? null : opt_from; + opt_value = opt_value == undefined ? timer.now() : opt_value; + + if (this.viewer_) { + this.viewer_.tick({ + 'label': label, + 'from': opt_from, + 'value': opt_value, + }); } else { this.queueTick_(label, opt_from, opt_value); } @@ -195,11 +240,11 @@ export class Performance { /** - * Calls the flush callback function set through setTickFunction. + * Calls the "flushTicks" function on the viewer. */ flush() { - if (this.flush_) { - this.flush_(); + if (this.viewer_) { + this.viewer_.flushTicks(); } } @@ -215,10 +260,6 @@ export class Performance { * @private */ queueTick_(label, opt_from, opt_value) { - if (opt_value == undefined) { - opt_value = timer.now(); - } - // Start dropping the head of the queue if we've reached the limit // so that we don't take up too much memory in the runtime. if (this.events_.length >= QUEUE_LIMIT) { @@ -226,45 +267,54 @@ export class Performance { } this.events_.push({ - label: label, - opt_from: opt_from, - opt_value: opt_value + 'label': label, + 'from': opt_from, + 'value': opt_value, }); } - /** @private */ + /** + * Forwards all queued ticks to the viewer tick method. + * @private + */ flushQueuedTicks_() { - if (!this.tick_) { + if (!this.viewer_) { return; } this.events_.forEach(tickEvent => { - this.tick_(tickEvent.label, tickEvent.opt_from, tickEvent.opt_value); + this.viewer_.tick(tickEvent); }); this.events_.length = 0; } /** - * Sets the `tick` function. - * - * @param {funtion(string,?string=,number=)} tick function that the tick - * events get forwarded to. Function can take in a `label` as the first - * argument and an optional `opt_from` label to use - * as a relative start for this tick. A third argument `opt_value` can - * also be provided to indicate when to record the tick at. - * @param {function()=} opt_flush callback function that is called - * when we are ready for the ticks to be forwarded to an endpoint. + * Calls "setFlushParams_" with relevant document information. + * @return {!Promise} + * @private */ - setTickFunction(tick, opt_flush) { - this.tick_ = tick; - this.flush_ = opt_flush; - this.flushQueuedTicks_(); - // We need to call flush right away in case `setTickFunction` is called - // later than the amp codebase had invoked the performance services' - // `flush` method to forward ticks. - this.flush(); + setDocumentInfoParams_() { + return this.whenViewportLayoutComplete_().then(() => { + const params = Object.create(null); + const sourceUrl = documentInfoFor(this.win).sourceUrl + .replace(/#.*/, ''); + params['sourceUrl'] = sourceUrl; + + this.resources_.get().forEach(r => { + const el = r.element; + const name = el.tagName.toLowerCase(); + incOrDef(params, name); + if (name == 'amp-ad') { + incOrDef(params, `ad-${el.getAttribute('type')}`); + } + }); + + // this should be guaranteed to be at the very least on the last + // visibility flush. + this.setFlushParams_(params); + }); } } diff --git a/src/runtime.js b/src/runtime.js index fa4b7cde3069..6e956ef7c2c6 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -21,7 +21,6 @@ import {getMode} from './mode'; import {installStyles} from './styles'; import {installCoreServices} from './amp-core-service'; import {isExperimentOn, toggleExperiment} from './experiments'; -import {performanceFor} from './performance'; import {registerElement} from './custom-element'; import {registerExtendedElement} from './extended-element'; import {resourcesFor} from './resources'; @@ -133,12 +132,10 @@ export function adopt(global) { * Sets the function to forward tick events to. * @param {funtion(string,?string=,number=)} fn * @param {function()=} opt_flush + * @deprecated * @export */ - global.AMP.setTickFunction = (fn, opt_flush) => { - const perf = performanceFor(global); - perf.setTickFunction(fn, opt_flush); - }; + global.AMP.setTickFunction = () => {}; // Execute asynchronously scheduled elements. for (let i = 0; i < preregisteredElements.length; i++) { diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 86060b19ae67..a8676fb4eb76 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -607,6 +607,29 @@ export class Viewer { this.sendMessage_('broadcast', message, false); } + /** + * Triggers "tick" event for the viewer. + * @param {!JSONObject} message + */ + tick(message) { + this.sendMessage_('tick', message, false); + } + + /** + * Triggers "sendCsi" event for the viewer. + */ + flushTicks() { + this.sendMessage_('sendCsi', undefined, false); + } + + /** + * Triggers "setFlushParams" event for the viewer. + * @param {!JSONObject} message + */ + setFlushParams(message) { + this.sendMessage_('setFlushParams', message, false); + } + /** * Registers receiver for the broadcast events. * @param {function(!JSONObject)} handler diff --git a/test/functional/test-3p-frame.js b/test/functional/test-3p-frame.js index 8c6ba8703856..b2ab5b46dd42 100644 --- a/test/functional/test-3p-frame.js +++ b/test/functional/test-3p-frame.js @@ -141,6 +141,7 @@ describe('3p-frame', () => { expect(c).to.not.be.null; expect(c.textContent).to.contain('pong'); validateData(win.context.data, ['ping', 'testAttr']); + document.head.removeChild(link); }); }); diff --git a/test/functional/test-document-info.js b/test/functional/test-document-info.js index 97b3ab2aa4c1..69f2afedf76d 100644 --- a/test/functional/test-document-info.js +++ b/test/functional/test-document-info.js @@ -37,6 +37,20 @@ describe('document-info', () => { }); }); + it('should provide the sourceUrl', () => { + const win = { + document: { + querySelector() { return 'http://www.origin.com/foo/?f=0'; } + }, + Math: {random() { return 0.123456789; }}, + location: { + href: 'https://cdn.ampproject.org/v/www.origin.com/foo/?f=0' + }, + }; + expect(documentInfoFor(win).sourceUrl).to.equal( + 'http://www.origin.com/foo/?f=0'); + }); + it('should provide the pageViewId', () => { return getWin('https://twitter.com/').then(win => { win.Math.random = () => { diff --git a/test/functional/test-performance.js b/test/functional/test-performance.js index 494ff985da19..fba88935522b 100644 --- a/test/functional/test-performance.js +++ b/test/functional/test-performance.js @@ -16,7 +16,7 @@ import * as sinon from 'sinon'; import {Performance, performanceFor} from '../../src/performance'; -import {adopt} from '../../src/runtime'; +import {getService, resetServiceForTesting} from '../../src/service'; import {resourcesFor} from '../../src/resources'; import {viewerFor} from '../../src/viewer'; @@ -39,11 +39,7 @@ describe('performance', () => { sandbox = null; }); - describe('when no tick function is set,', () => { - - it('should not have a tick function', () => { - expect(perf.tick_).to.be.undefined; - }); + describe('when viewer is not ready', () => { it('should queue up tick events', () => { expect(perf.events_.length).to.equal(0); @@ -60,11 +56,12 @@ describe('performance', () => { perf.tickDelta('test', 99); expect(perf.events_.length).to.equal(2); - expect(perf.events_[0]).to.jsonEqual({label: '_test', opt_value: 1000}); - expect(perf.events_[1]).to.jsonEqual({ + expect(perf.events_[0]) + .to.be.jsonEqual({label: '_test', from: null, value: 1000}); + expect(perf.events_[1]).to.be.jsonEqual({ label: 'test', - opt_from: '_test', - opt_value: 1099 + from: '_test', + value: 1099, }); }); @@ -83,10 +80,10 @@ describe('performance', () => { clock.tick(150); perf.tick('start0'); - expect(perf.events_[0]).to.deep.equal({ + expect(perf.events_[0]).to.be.jsonEqual({ label: 'start0', - opt_from: undefined, - opt_value: 150 + from: null, + value: 150, }); }); @@ -94,10 +91,10 @@ describe('performance', () => { clock.tick(150); perf.tick('start0', 'start1', 300); - expect(perf.events_[0]).to.deep.equal({ + expect(perf.events_[0]).to.be.jsonEqual({ label: 'start0', - opt_from: 'start1', - opt_value: 300 + from: 'start1', + value: 300, }); }); @@ -112,41 +109,37 @@ describe('performance', () => { } expect(perf.events_.length).to.equal(50); - expect(perf.events_[0]).to.deep.equal({ + expect(perf.events_[0]).to.be.jsonEqual({ label: 'start0', - opt_from: undefined, - opt_value: tickTime + from: null, + value: tickTime, }); clock.tick(1); perf.tick('start50'); - expect(perf.events_[0]).to.deep.equal({ + expect(perf.events_[0]).to.be.jsonEqual({ label: 'start1', - opt_from: undefined, - opt_value: tickTime + from: null, + value: tickTime, }); - expect(perf.events_[49]).to.deep.equal({ + expect(perf.events_[49]).to.be.jsonEqual({ label: 'start50', - opt_from: undefined, - opt_value: tickTime + 1 + from: null, + value: tickTime + 1, }); }); }); - describe('when tick function is set,', () => { - let spy; + describe('when viewer is ready,', () => { + let tickSpy; + let flushTicksSpy; + let viewer; beforeEach(() => { - spy = sinon.spy(); - }); - - it('should be able to install a performance function', () => { - expect(perf.tick_).to.be.undefined; - - perf.setTickFunction(spy); - - expect(perf.tick_).to.be.an.instanceof(Function); + viewer = viewerFor(window); + tickSpy = sandbox.stub(viewer, 'tick'); + flushTicksSpy = sandbox.stub(viewer, 'flushTicks'); }); it('should forward all queued tick events', () => { @@ -156,14 +149,18 @@ describe('performance', () => { expect(perf.events_.length).to.equal(2); - perf.setTickFunction(spy); + perf.coreServicesAvailable(); - expect(spy.firstCall.args[0]).to.equal('start0'); - expect(spy.firstCall.args[1]).to.equal(undefined); - expect(spy.firstCall.args[2]).to.equal(0); - expect(spy.secondCall.args[0]).to.equal('start1'); - expect(spy.secondCall.args[1]).to.equal('start0'); - expect(spy.secondCall.args[2]).to.equal(1); + expect(tickSpy.firstCall.args[0]).to.be.jsonEqual({ + label: 'start0', + from: null, + value: 0, + }); + expect(tickSpy.secondCall.args[0]).to.be.jsonEqual({ + label: 'start1', + from: 'start0', + value: 1, + }); }); it('should have no more queued tick events after flush', () => { @@ -172,74 +169,42 @@ describe('performance', () => { expect(perf.events_.length).to.equal(2); - perf.setTickFunction(spy); + perf.coreServicesAvailable(); expect(perf.events_.length).to.equal(0); }); it('should forward tick events', () => { - perf.setTickFunction(spy); + perf.coreServicesAvailable(); - perf.tick('start0'); clock.tick(100); + perf.tick('start0'); perf.tick('start1', 'start0', 300); - expect(spy.firstCall.args[0]).to.equal('start0'); - expect(spy.firstCall.args[1]).to.equal(undefined); - expect(spy.firstCall.args[2]).to.equal(undefined); - - expect(spy.secondCall.args[0]).to.equal('start1'); - expect(spy.secondCall.args[1]).to.equal('start0'); - expect(spy.secondCall.args[2]).to.equal(300); + expect(tickSpy.firstCall.args[0]).to.be.jsonEqual({ + label: 'start0', + from: null, + value: 100, + }); + expect(tickSpy.secondCall.args[0]).to.be.jsonEqual({ + label: 'start1', + from: 'start0', + value: 300, + }); }); it('should call the flush callback', () => { - perf.setTickFunction(function() {}, spy); - - // We start at 1 since setting the tick function and flush - // causes 1 flush call for any queued events. - expect(spy.callCount).to.equal(1); + expect(flushTicksSpy.callCount).to.equal(0); + // coreServicesAvailable calls flush once. + perf.coreServicesAvailable(); + expect(flushTicksSpy.callCount).to.equal(1); perf.flush(); - expect(spy.callCount).to.equal(2); + expect(flushTicksSpy.callCount).to.equal(2); perf.flush(); - expect(spy.callCount).to.equal(3); + expect(flushTicksSpy.callCount).to.equal(3); }); }); - it('can set the performance function through the runtime', () => { - const perf = performanceFor(window); - const spy = sandbox.spy(perf, 'setTickFunction'); - const fn = function() {}; - - adopt(window); - - window.AMP.setTickFunction(fn); - - expect(spy.firstCall.args[0]).to.equal(fn); - }); - - it('can set the flush function through the runtime', () => { - const perf = performanceFor(window); - const spy = sandbox.spy(perf, 'setTickFunction'); - const fn = function() {}; - - adopt(window); - - window.AMP.setTickFunction(function() {}, fn); - - expect(spy.firstCall.args[1]).to.equal(fn); - }); - - it('should call the flush function after its set', () => { - const flushSpy = sinon.spy(); - - adopt(window); - - window.AMP.setTickFunction(function() {}, flushSpy); - - expect(flushSpy.calledOnce).to.be.true; - }); - describe('coreServicesAvailable', () => { let tickSpy; let viewer; @@ -344,4 +309,46 @@ describe('performance', () => { }); }); }); + + it('should setFlushParams', () => { + const perf = performanceFor(window); + const viewer = viewerFor(window); + sandbox.stub(perf, 'whenViewportLayoutComplete_') + .returns(Promise.resolve()); + const setFlushParamsSpy = sandbox.stub(viewer, 'setFlushParams'); + perf.coreServicesAvailable(); + resetServiceForTesting(window, 'documentInfo'); + const info = { + canonicalUrl: 'https://foo.bar/baz', + pageViewId: 12345, + sourceUrl: 'https://hello.world/baz/#development' + }; + getService(window, 'documentInfo', () => info); + + const ad1 = document.createElement('amp-ad'); + ad1.setAttribute('type', 'abc'); + const ad2 = document.createElement('amp-ad'); + ad2.setAttribute('type', 'xyz'); + const ad3 = document.createElement('amp-ad'); + sandbox.stub(perf.resources_, 'get').returns([ + {element: document.createElement('amp-img')}, + {element: document.createElement('amp-img')}, + {element: document.createElement('amp-anim')}, + {element: ad1}, + {element: ad2}, + {element: ad3}, + ]); + + return perf.setDocumentInfoParams_().then(() => { + expect(setFlushParamsSpy.lastCall.args[0]).to.be.jsonEqual({ + sourceUrl: 'https://hello.world/baz/', + 'amp-img': 2, + 'amp-anim': 1, + 'amp-ad': 3, + 'ad-abc': 1, + 'ad-xyz': 1, + 'ad-null': 1, + }); + }); + }); });