Skip to content

Commit

Permalink
Merge pull request #1862 from erwinmombay/set-query-params
Browse files Browse the repository at this point in the history
feature(perf): add `setParams` method
  • Loading branch information
erwinmombay committed Feb 11, 2016
2 parents 4760a0f + 94e7951 commit 34c5b86
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 132 deletions.
4 changes: 3 additions & 1 deletion src/document-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@

import {getService} from './service';
import {assert} from './asserts';
import {parseUrl} from './url';
import {parseUrl, getSourceUrl} from './url';

/**
* @param {!Window} win
* @return {{canonicalUrl: string, pageViewId: string}} Info about the doc
* - 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', () => {
Expand All @@ -33,6 +34,7 @@ export function documentInfoFor(win) {
'AMP files are required to have a <link rel=canonical> tag.')
.href).href,
pageViewId: getPageViewId(win),
sourceUrl: getSourceUrl(win.location.href),
};
});
}
Expand Down
114 changes: 82 additions & 32 deletions src/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, (string|number|boolean|Array|Object|null)>} 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
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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.
*
Expand All @@ -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);
}
Expand All @@ -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();
}
}

Expand All @@ -215,56 +260,61 @@ 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) {
this.events_.shift();
}

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);
});
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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++) {
Expand Down
23 changes: 23 additions & 0 deletions src/service/viewer-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/functional/test-3p-frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
14 changes: 14 additions & 0 deletions test/functional/test-document-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
Loading

0 comments on commit 34c5b86

Please sign in to comment.