Doubleclick Fast Fetch: Measure potential delay of waiting for all ad urls to be constructed#9244
Doubleclick Fast Fetch: Measure potential delay of waiting for all ad urls to be constructed#9244keithwrightbos merged 10 commits intoampproject:masterfrom google:a4a_sra_delay_measurement
Conversation
|
@dvoytenko mind taking a look? I still need to create tests. |
dvoytenko
left a comment
There was a problem hiding this comment.
Looks good. Couple comments.
extensions/amp-a4a/0.1/amp-a4a.js
Outdated
| } | ||
| const getAdUrlsPromise = []; | ||
| resources.forEach(r => getAdUrlsPromise.push(r.element.getImpl().then( | ||
| instance => { |
There was a problem hiding this comment.
Updated to comment:
// Note that ad url promise could be null if isValidElement returns false.
| } | ||
|
|
||
| /** | ||
| * Returns reference to implementation after it has been built. |
There was a problem hiding this comment.
It'd be !Promise<!./base-element.BaseElement> - promise response is non-nullable.
extensions/amp-a4a/0.1/amp-a4a.js
Outdated
| return; | ||
| } | ||
| this.win['a4a-measuring-ad-urls'] = | ||
| (() => resourcesForDoc(this.element).getMeasuredResources(this.win, |
There was a problem hiding this comment.
Why are you wrapping this in a closure?
extensions/amp-a4a/0.1/amp-a4a.js
Outdated
| throw cancellation(); | ||
| } | ||
| const getAdUrlsPromise = []; | ||
| resources.forEach(r => getAdUrlsPromise.push(r.element.getImpl().then( |
extensions/amp-a4a/0.1/amp-a4a.js
Outdated
| 'sraBuildRequestDelay', { | ||
| 'met.delta.AD_SLOT_ID': Math.round( | ||
| this.getNow_() - startTime), | ||
| 'totalSlotCount.AD_SLOT_ID': getAdUrlsPromise.length, |
| r.element.getAttribute('type') == type; | ||
| }) | ||
| .then(resources => { | ||
| if (promiseId != this.promiseId_) { |
There was a problem hiding this comment.
Why does a cancellation matter?
There was a problem hiding this comment.
Want to ensure we don't send the CSI ping if the page was destroyed. Its arguably an edge case but see no negative.
|
@jridgewell PTAL, would like to get this into today's cut. Thanks |
In advance of adding Doubleclick Fast Fetch SRA support (see #9115), measure the amount of time between first ad slot url construction and last. This will help determine if explicit SRA (waiting for each slot to build its URL) is viable by determining the amount of delay it imposes on the first slot. Note: