Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(perf): add setFlushParams method #1862

Merged
merged 1 commit into from
Feb 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update the return doc comment above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! done.

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