From 3640f2cff6cdd0882451149a8112cf87549c2223 Mon Sep 17 00:00:00 2001 From: Caleb Cordry Date: Wed, 30 Oct 2019 20:43:04 -0700 Subject: [PATCH] A4A: Change a few window refs to ampdoc (#25321) --- ads/google/a4a/test/test-utils.js | 23 +++++++++++-------- ads/google/a4a/utils.js | 8 +++---- .../0.1/amp-ad-network-doubleclick-impl.js | 2 +- src/custom-element.js | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/ads/google/a4a/test/test-utils.js b/ads/google/a4a/test/test-utils.js index 647292de7e45..9d1f1cc55494 100644 --- a/ads/google/a4a/test/test-utils.js +++ b/ads/google/a4a/test/test-utils.js @@ -1001,10 +1001,11 @@ describe('Google A4A utils', () => { }); describes.realWin('#groupAmpAdsByType', {amp: true}, env => { - let doc, win; + let doc, win, ampdoc; beforeEach(() => { win = env.win; doc = win.document; + ampdoc = env.ampdoc; }); function createResource(config, tagName = 'amp-ad', parent = doc.body) { @@ -1023,14 +1024,18 @@ describes.realWin('#groupAmpAdsByType', {amp: true}, env => { sandbox .stub(IniLoad, 'getMeasuredResources') .callsFake((doc, win, fn) => Promise.resolve(resources.filter(fn))); - return groupAmpAdsByType(win, 'doubleclick', () => 'foo').then(result => { - expect(Object.keys(result).length).to.equal(1); - expect(result['foo']).to.be.ok; - expect(result['foo'].length).to.equal(1); - return result['foo'][0].then(baseElement => - expect(baseElement.element.getAttribute('type')).to.equal('doubleclick') - ); - }); + return groupAmpAdsByType(ampdoc, 'doubleclick', () => 'foo').then( + result => { + expect(Object.keys(result).length).to.equal(1); + expect(result['foo']).to.be.ok; + expect(result['foo'].length).to.equal(1); + return result['foo'][0].then(baseElement => + expect(baseElement.element.getAttribute('type')).to.equal( + 'doubleclick' + ) + ); + } + ); }); it('should find amp-ad within sticky container', () => { diff --git a/ads/google/a4a/utils.js b/ads/google/a4a/utils.js index ad4f41837762..df24ffb2f54e 100644 --- a/ads/google/a4a/utils.js +++ b/ads/google/a4a/utils.js @@ -203,12 +203,12 @@ export function googleBlockParameters(a4a, opt_experimentIds) { } /** - * @param {!Window} win + * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {string} type matching typing attribute. * @param {function(!Element):string} groupFn * @return {!Promise>>>} */ -export function groupAmpAdsByType(win, type, groupFn) { +export function groupAmpAdsByType(ampdoc, type, groupFn) { // Look for amp-ad elements of correct type or those contained within // standard container type. Note that display none containers will not be // included as they will never be measured. @@ -217,10 +217,8 @@ export function groupAmpAdsByType(win, type, groupFn) { // visible). const ampAdSelector = r => r.element./*OK*/ querySelector(`amp-ad[type=${type}]`); - const {documentElement} = win.document; - const ampdoc = Services.ampdoc(documentElement); return ( - getMeasuredResources(ampdoc, win, r => { + getMeasuredResources(ampdoc, ampdoc.win, r => { const isAmpAdType = r.element.tagName == 'AMP-AD' && r.element.getAttribute('type') == type; if (isAmpAdType) { diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js index 66df66a0892f..683faf5fa449 100644 --- a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js +++ b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js @@ -1407,7 +1407,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { */ groupSlotsForSra() { return groupAmpAdsByType( - this.win, + this.getAmpDoc(), this.element.getAttribute('type'), getNetworkId ); diff --git a/src/custom-element.js b/src/custom-element.js index 1cbe71b9e55d..d0e03d1f9cba 100644 --- a/src/custom-element.js +++ b/src/custom-element.js @@ -564,7 +564,7 @@ function createBaseCustomElementClass(win) { // If we do early preconnects we delay them a bit. This is kind of // an unfortunate trade off, but it seems faster, because the DOM // operations themselves are not free and might delay - startupChunk(self.document, () => { + startupChunk(this.getAmpDoc(), () => { const TAG = this.tagName; if (!this.ownerDocument) { dev().error(TAG, 'preconnect without ownerDocument');