Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 5 additions & 6 deletions lighthouse-plugin-publisher-ads/audits/first-ad-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

const AdPaintTime = require('../computed/ad-paint-time');
const ComputedAdPaintTime = require('../computed/ad-paint-time');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename "ad paint" to "ad render" in a future PR? It's not really accurate after this change

Copy link
Collaborator

Choose a reason for hiding this comment

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

Created #122 for this

const i18n = require('lighthouse/lighthouse-core/lib/i18n/i18n');
const {auditNotApplicable, runWarning} = require('../messages/common-strings');
const {Audit} = require('lighthouse');
Expand All @@ -30,8 +30,8 @@ const UIStrings = {
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

// Point of diminishing returns.
const PODR = 3000; // ms
const MEDIAN = 4000; // ms
const PODR = 2700; // ms
const MEDIAN = 3700; // ms

/**
* Measures the first ad paint time.
Expand All @@ -51,7 +51,7 @@ class FirstAdPaint extends Audit {
// @ts-ignore
scoreDisplayMode: Audit.SCORING_MODES.NUMERIC,
// @ts-ignore
requiredArtifacts: ['devtoolsLogs', 'traces', 'IFrameElements'],
requiredArtifacts: ['devtoolsLogs', 'traces'],
};
}
/**
Expand All @@ -65,10 +65,9 @@ class FirstAdPaint extends Audit {
const metricData = {
devtoolsLog,
trace,
iframeElements: artifacts.IFrameElements,
settings: context.settings,
};
const {timing} = await AdPaintTime.request(metricData, context);
const {timing} = await ComputedAdPaintTime.request(metricData, context);

if (!(timing > 0)) { // Handle NaN, etc.
context.LighthouseRunWarnings.push(runWarning.NoAdRendered);
Expand Down
109 changes: 19 additions & 90 deletions lighthouse-plugin-publisher-ads/computed/ad-paint-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,47 +17,14 @@ const AdLanternMetric = require('./ad-lantern-metric');
const ComputedMetric = require('lighthouse/lighthouse-core/computed/metrics/metric');
// @ts-ignore
const makeComputedArtifact = require('lighthouse/lighthouse-core/computed/computed-artifact');
const {isGptAdRequest, isGptIframe} = require('../utils/resource-classification');
const {getPageStartTime, getImpressionStartTime} = require('../utils/network-timing');
const {isImpressionPing} = require('../utils/resource-classification');

/**
* Returns the frame ID of the given event, if present.
* @param {LH.TraceEvent} event
* @return {?string}
*/
function getFrame(event) {
// @ts-ignore
return event.args.frame || event.args.data && event.args.data.frame || null;
}

/**
* Returns the first timestamp of the given event for ad iframes, or 0 if no
* relevant timing is found.
* @param {string} eventName
* @param {LH.TraceEvent[]} traceEvents
* @param {Set<string>} adFrameIds
* @return {number}
*/
function getMinEventTime(eventName, traceEvents, adFrameIds) {
const times = traceEvents
.filter((e) => e.name == eventName)
.filter((e) => adFrameIds.has(getFrame(e) || ''))
.map((e) => e.ts);
return times.length ? Math.min(...times) : 0;
}

/**
* @param {MetricComputationData} data
* @return {Array<Artifacts['IFrameElement']>}
*/
function getAdIframes(data) {
const {iframeElements} = data;
if (!iframeElements) {
return [];
}
return iframeElements.filter(isGptIframe);
}
// @ts-ignore
// eslint-disable-next-line max-len
/** @typedef {import('lighthouse/lighthouse-core/lib/dependency-graph/base-node.js').Node} Node */

/** Computes simulated first ad request time using Lantern. */
/** Computes simulated first ad paint time using Lantern. */
class LanternAdPaintTime extends AdLanternMetric {
/**
* @param {LH.Gatherer.Simulation.Result} simulationResult
Expand All @@ -67,16 +34,9 @@ class LanternAdPaintTime extends AdLanternMetric {
*/
static getEstimateFromSimulation(simulationResult, extras) {
const {nodeTimings} = simulationResult;
const {iframes} = extras;
const adFrameIds = new Set(iframes.map(
/** @param {Artifacts['IFrameElement']} s */
(s) => s.frame && s.frame.id));
const adResponseMs = AdLanternMetric.findNetworkTiming(
nodeTimings, isGptAdRequest).endTime;
// TODO: filter out pixels from resources
const firstAdResource = AdLanternMetric.findNetworkTiming(
nodeTimings, (request) => adFrameIds.has(request.frameId)).endTime;
const timeInMs = adResponseMs + firstAdResource;
const timeInMs = AdLanternMetric.findNetworkTiming(
nodeTimings,
(req) => !!req.url && isImpressionPing(new URL(req.url))).startTime;
return {timeInMs, nodeTimings};
}
}
Expand All @@ -86,62 +46,31 @@ class LanternAdPaintTime extends AdLanternMetric {
// eslint-disable-next-line no-class-assign
LanternAdPaintTime = makeComputedArtifact(LanternAdPaintTime);

/** Computes the first ad paint time on the page */
/** Computes the first ad paint time metric. */
class AdPaintTime extends ComputedMetric {
/**
* @param {MetricComputationData} data
* @param {LH.Artifacts.MetricComputationData} data
* @param {LH.Audit.Context} context
* @return {Promise<LH.Artifacts.LanternMetric>}
* @override
*/
static async computeSimulatedMetric(data, context) {
const iframes = getAdIframes(data);
// @ts-ignore computeMetricWithGraphs is not a property of
// LanternAdPaintTime.
return LanternAdPaintTime.computeMetricWithGraphs(data, context, {iframes});
// @ts-ignore request does not exist on LanternAdPaintTime
return LanternAdPaintTime.request(data, context);
}

/**
* @param {MetricComputationData} data
* @param {LH.Artifacts.MetricComputationData} data
* @param {LH.Audit.Context} context
* @return {Promise<LH.Artifacts.Metric>}
* @override
*/
static async computeObservedMetric(data, context) {
const iframes = getAdIframes(data);
const {trace: {traceEvents}} = data;
const {ts: pageNavigationStart} =
traceEvents.find((e) => e.name == 'navigationStart') || {ts: 0};

if (!iframes.length || !pageNavigationStart) {
return Promise.resolve({timing: -1});
}
const adFrameIds = new Set(iframes.map((s) => s.frame && s.frame.id));
const adPaintTime =
getMinEventTime('firstContentfulPaint', traceEvents, adFrameIds) ||
getMinEventTime('firstPaint', traceEvents, adFrameIds);

let timingMs = 0;
if (adPaintTime) {
timingMs = (adPaintTime - pageNavigationStart) / 1000;
} else {
// If we don't find a first paint event in the trace, then fall back to
// the time of the first request.
// TODO(warrengm): Search child iframes if there is no paint event in the
// top frame.
const {networkRecords} = data;
// Note that we don't really care about whether this request is paintable
// or not. This is for two reasons:
// - Reduce variability due to which ad served.
// - Not consider factors that are outside the publisher's control.
// That is, developers don't choose which ad served.
const firstRequest = networkRecords.find(
(r) => adFrameIds.has(r.frameId) && r.resourceType != 'Document');
if (firstRequest) {
timingMs = firstRequest.endTime - (pageNavigationStart / 1e6);
}
}
return Promise.resolve({timing: timingMs});
const {networkRecords} = data;
const pageStartTime = getPageStartTime(networkRecords);
const impressionStartTime = getImpressionStartTime(networkRecords);
const firstPaintMs = (impressionStartTime - pageStartTime) * 1000;
return Promise.resolve({timing: firstPaintMs});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

const {expect} = require('chai');
const {isGoogleAds, isGptAdRequest, hasImpressionPath, isGptTag} = require('../../utils/resource-classification');
const {isGoogleAds, isGptAdRequest, isImpressionPing, isGptTag} = require('../../utils/resource-classification');
const {URL} = require('url');

describe('resource-classification', () => {
Expand Down Expand Up @@ -106,20 +106,25 @@ describe('resource-classification', () => {
});
});

describe('#hasImpressionPath', () => {
it('should return true for /pcs/view as the impression path', () => {
const url = new URL('https://googlesyndication.com/pcs/view?bar=baz');
expect(hasImpressionPath(url)).to.be.true;
describe('#isImpressionPing', () => {
it('should return true for securepubads.g.doubleclick.net/pcs/view as the impression path', () => {
const url = new URL('https://securepubads.g.doubleclick.net/pcs/view?bar=baz');
expect(isImpressionPing(url)).to.be.true;
});

it('should return true for googleads4.g.doubleclick.net/pcs/view as the impression path', () => {
const url = new URL('https://googleads4.g.doubleclick.net/pcs/view?bar=baz');
expect(isImpressionPing(url)).to.be.true;
});

it('should return true for /pagead/adview as the impression path', () => {
const url = new URL('https://googlesyndication.com/pagead/adview?bar=baz');
expect(hasImpressionPath(url)).to.be.true;
const url = new URL('https://securepubads.g.doubleclick.net/pagead/adview?bar=baz');
expect(isImpressionPing(url)).to.be.true;
});

it('should return false for any other impression path', () => {
const url = new URL('https://googlesyndication.com/file/folder/foo?bar=baz');
expect(hasImpressionPath(url)).to.be.false;
const url = new URL('https://securepubads.g.doubleclick.net/file/folder/foo?bar=baz');
expect(isImpressionPing(url)).to.be.false;
});
});

Expand Down
14 changes: 13 additions & 1 deletion lighthouse-plugin-publisher-ads/utils/network-timing.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const AdLanternMetric = require('../computed/ad-lantern-metric');
const LoadSimulator = require('lighthouse/lighthouse-core/computed/load-simulator');
const NetworkRecords = require('lighthouse/lighthouse-core/computed/network-records');
const PageDependencyGraph = require('lighthouse/lighthouse-core/computed/page-dependency-graph');
const {isGptAdRequest, isImplTag} = require('./resource-classification');
const {isGptAdRequest, isImplTag, isImpressionPing} = require('./resource-classification');
const {URL} = require('url');

/** @typedef {LH.Artifacts.NetworkRequest} NetworkRequest */
Expand Down Expand Up @@ -50,6 +50,17 @@ function getAdStartTime(networkRecords) {
return firstAdRecord ? firstAdRecord.startTime : -1;
}

/**
* Returns start time of first ad impression relative to system boot.
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
* @return {number}
*/
function getImpressionStartTime(networkRecords) {
const firstImpressionRecord = networkRecords.find(
(record) => isImpressionPing(record.url));
return firstImpressionRecord ? firstImpressionRecord.startTime : -1;
}

/**
* Returns start time of page request (s) relative to system boot.
* @param {LH.Artifacts.NetworkRequest[]} networkRecords
Expand Down Expand Up @@ -177,6 +188,7 @@ async function getScriptEvaluationTimes(trace, devtoolsLog, context) {

module.exports = {
getTagEndTime,
getImpressionStartTime,
getAdStartTime,
getPageStartTime,
getPageResponseTime,
Expand Down
27 changes: 17 additions & 10 deletions lighthouse-plugin-publisher-ads/utils/resource-classification.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,22 @@ function isImplTag(url) {
return /(^\/gpt\/pubads_impl_\d+.js)/.test(toURL(url).pathname);
}

/**
* Checks if the url is an impression ping.
* @param {URL|string} url
* @return {boolean}
*/
function isImpressionPing(url) {
const {host, pathname} = toURL(url);
return (
[
'securepubads.g.doubleclick.net',
'googleads4.g.doubleclick.net',
].includes(host) &&
['/pcs/view', '/pagead/adview'].includes(pathname)
);
}

/**
* Checks if the url is loading a gpt.js script.
* @param {URL|string} url
Expand Down Expand Up @@ -94,15 +110,6 @@ function isGptAdRequest(request) {
);
}

/**
* Checks if the url has an impression path.
* @param {URL} url
* @return {boolean}
*/
function hasImpressionPath(url) {
return url.pathname === '/pcs/view' ||
url.pathname === '/pagead/adview';
}

/**
* Returns header bidder or undefined if not a bid.
Expand Down Expand Up @@ -201,7 +208,7 @@ module.exports = {
isBidRequest,
isGoogleAds,
isGptAdRequest,
hasImpressionPath,
isImpressionPing,
isGpt,
isGptTag,
isImplTag,
Expand Down