Skip to content

Commit

Permalink
A4A: Change a few window refs to ampdoc (#25321)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebcordry authored Oct 31, 2019
1 parent 3905e6b commit 3640f2c
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 16 deletions.
23 changes: 14 additions & 9 deletions ads/google/a4a/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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', () => {
Expand Down
8 changes: 3 additions & 5 deletions ads/google/a4a/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<!Object<string,!Array<!Promise<!../../../src/base-element.BaseElement>>>>}
*/
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.
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
*/
groupSlotsForSra() {
return groupAmpAdsByType(
this.win,
this.getAmpDoc(),
this.element.getAttribute('type'),
getNetworkId
);
Expand Down
2 changes: 1 addition & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit 3640f2c

Please sign in to comment.