Skip to content

Commit 26c30f0

Browse files
keithwrightboskeithwrightbos
authored
Cleanup AdSense/Doubleclick round adx/ady (ampproject#30081)
1 parent 5247739 commit 26c30f0

File tree

6 files changed

+38
-118
lines changed

6 files changed

+38
-118
lines changed

‎ads/google/a4a/test/test-utils.js‎

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -380,16 +380,12 @@ describe('Google A4A utils', () => {
380380
});
381381
const impl = new MockA4AImpl(elem);
382382
noopMethods(impl, fixture.ampdoc, window.sandbox);
383-
return fixture.addElement(elem).then(() => {
384-
return googleAdUrl(impl, '', 0, [], false, []).then((url1) => {
385-
expect(url1).to.match(/ady=11.1/);
386-
expect(url1).to.match(/adx=12.1/);
387-
return googleAdUrl(impl, '', 0, [], true, []).then((url1) => {
388-
expect(url1).to.match(/ady=11/);
389-
expect(url1).to.match(/adx=12/);
390-
});
391-
});
392-
});
383+
return fixture.addElement(elem).then(() =>
384+
googleAdUrl(impl, '', 0, [], []).then((url1) => {
385+
expect(url1).to.match(/ady=11/);
386+
expect(url1).to.match(/adx=12/);
387+
})
388+
);
393389
});
394390
});
395391

@@ -415,13 +411,8 @@ describe('Google A4A utils', () => {
415411
const getScrollTop = () => 34.2;
416412
const viewportStub = window.sandbox.stub(Services, 'viewportForDoc');
417413
viewportStub.returns({getRect, getSize, getScrollTop, getScrollLeft});
418-
return fixture.addElement(elem).then(() => {
419-
return googleAdUrl(impl, '', 0, {}, false, []).then((url1) => {
420-
expect(url1).to.match(/scr_x=12.1&scr_y=34.2/);
421-
});
422-
return googleAdUrl(impl, '', 0, {}, true, []).then((url1) => {
423-
expect(url1).to.match(/scr_x=12&scr_y=34/);
424-
});
414+
return googleAdUrl(impl, '', 0, {}, []).then((url1) => {
415+
expect(url1).to.match(/scr_x=12&scr_y=34/);
425416
});
426417
});
427418
});
@@ -442,11 +433,9 @@ describe('Google A4A utils', () => {
442433
const impl = new MockA4AImpl(elem);
443434
noopMethods(impl, fixture.ampdoc, window.sandbox);
444435
return fixture.addElement(elem).then(() => {
445-
return googleAdUrl(impl, '', 0, {}, false, ['789', '098']).then(
446-
(url1) => {
447-
expect(url1).to.match(/eid=123%2C456%2C789%2C098/);
448-
}
449-
);
436+
return googleAdUrl(impl, '', 0, {}, ['789', '098']).then((url1) => {
437+
expect(url1).to.match(/eid=123%2C456%2C789%2C098/);
438+
});
450439
});
451440
});
452441
});
@@ -466,7 +455,7 @@ describe('Google A4A utils', () => {
466455
impl.win.AMP_CONFIG = {type: 'production'};
467456
impl.win.location.hash = 'foo,deid=123456,654321,bar';
468457
return fixture.addElement(elem).then(() => {
469-
return googleAdUrl(impl, '', 0, [], false, []).then((url1) => {
458+
return googleAdUrl(impl, '', 0, [], []).then((url1) => {
470459
expect(url1).to.match(/[&?]debug_experiment_id=123456%2C654321/);
471460
});
472461
});
@@ -487,7 +476,7 @@ describe('Google A4A utils', () => {
487476
noopMethods(impl, fixture.ampdoc, window.sandbox);
488477
impl.win.gaGlobal = {cid: 'foo', hid: 'bar'};
489478
return fixture.addElement(elem).then(() => {
490-
return googleAdUrl(impl, '', 0, [], false, []).then((url) => {
479+
return googleAdUrl(impl, '', 0, [], []).then((url) => {
491480
expect(url).to.match(/[&?]ga_cid=foo[&$]/);
492481
expect(url).to.match(/[&?]ga_hid=bar[&$]/);
493482
});
@@ -517,9 +506,9 @@ describe('Google A4A utils', () => {
517506
},
518507
});
519508
return fixture.addElement(elem).then(() => {
520-
return expect(
521-
googleAdUrl(impl, '', 0, {}, false, [])
522-
).to.eventually.match(/[&?]bc=7[&$]/);
509+
return expect(googleAdUrl(impl, '', 0, {}, [])).to.eventually.match(
510+
/[&?]bc=7[&$]/
511+
);
523512
});
524513
});
525514
});
@@ -544,9 +533,9 @@ describe('Google A4A utils', () => {
544533
sandbox: {},
545534
});
546535
return fixture.addElement(elem).then(() => {
547-
return expect(
548-
googleAdUrl(impl, '', 0, {}, false, [])
549-
).to.eventually.match(/[&?]bc=1[&$]/);
536+
return expect(googleAdUrl(impl, '', 0, {}, [])).to.eventually.match(
537+
/[&?]bc=1[&$]/
538+
);
550539
});
551540
});
552541
});
@@ -575,7 +564,7 @@ describe('Google A4A utils', () => {
575564
});
576565
return fixture.addElement(elem).then(() => {
577566
return expect(
578-
googleAdUrl(impl, '', 0, {}, false, [])
567+
googleAdUrl(impl, '', 0, {}, [])
579568
).to.eventually.not.match(/[&?]bc=1[&$]/);
580569
});
581570
});
@@ -608,7 +597,7 @@ describe('Google A4A utils', () => {
608597
expectAsyncConsoleError(/Referrer timeout/, 1);
609598
return fixture.addElement(elem).then(() => {
610599
return expect(
611-
googleAdUrl(impl, '', 0, {}, false, [])
600+
googleAdUrl(impl, '', 0, {}, [])
612601
).to.eventually.not.match(/[&?]ref=[&$]/);
613602
});
614603
});
@@ -623,11 +612,9 @@ describe('Google A4A utils', () => {
623612
const impl = new MockA4AImpl(elem);
624613
noopMethods(impl, fixture.ampdoc, window.sandbox);
625614
return fixture.addElement(elem).then(() => {
626-
return googleAdUrl(impl, '', Date.now(), [], false, []).then(
627-
(url) => {
628-
expect(url).to.match(/[&?]bdt=[1-9][0-9]*[&$]/);
629-
}
630-
);
615+
return googleAdUrl(impl, '', Date.now(), [], []).then((url) => {
616+
expect(url).to.match(/[&?]bdt=[1-9][0-9]*[&$]/);
617+
});
631618
});
632619
});
633620
});

‎ads/google/a4a/utils.js‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,12 @@ export function isReportingEnabled(ampElement) {
191191
/**
192192
* Has side-effect of incrementing ifi counter on window.
193193
* @param {!../../../extensions/amp-a4a/0.1/amp-a4a.AmpA4A} a4a
194-
* @param {boolean} roundLocations when true scr_x & scr_y are rounded
195194
* @param {!Array<string>=} opt_experimentIds Any experiments IDs (in addition
196195
* to those specified on the ad element) that should be included in the
197196
* request.
198197
* @return {!Object<string,null|number|string>} block level parameters
199198
*/
200-
export function googleBlockParameters(a4a, roundLocations, opt_experimentIds) {
199+
export function googleBlockParameters(a4a, opt_experimentIds) {
201200
const {element: adElement, win} = a4a;
202201
const slotRect = a4a.getPageLayoutBox();
203202
const iframeDepth = iframeNestingDepth(win);
@@ -210,8 +209,8 @@ export function googleBlockParameters(a4a, roundLocations, opt_experimentIds) {
210209
'adf': DomFingerprint.generate(adElement),
211210
'nhd': iframeDepth,
212211
'eid': eids,
213-
'adx': roundLocations ? Math.round(slotRect.left) : slotRect.left,
214-
'ady': roundLocations ? Math.round(slotRect.top) : slotRect.top,
212+
'adx': Math.round(slotRect.left),
213+
'ady': Math.round(slotRect.top),
215214
'oid': '2',
216215
'act': enclosingContainers.length ? enclosingContainers.join() : null,
217216
};
@@ -274,10 +273,9 @@ export function groupAmpAdsByType(ampdoc, type, groupFn) {
274273
/**
275274
* @param {! ../../../extensions/amp-a4a/0.1/amp-a4a.AmpA4A} a4a
276275
* @param {number} startTime
277-
* @param {boolean} roundLocations when true scr_x & scr_y are rounded
278276
* @return {!Promise<!Object<string,null|number|string>>}
279277
*/
280-
export function googlePageParameters(a4a, startTime, roundLocations) {
278+
export function googlePageParameters(a4a, startTime) {
281279
const {win} = a4a;
282280
const ampDoc = a4a.getAmpDoc();
283281
// Do not wait longer than 1 second to retrieve referrer to ensure
@@ -330,12 +328,8 @@ export function googlePageParameters(a4a, startTime, roundLocations) {
330328
'ish': win != win.top ? viewportSize.height : null,
331329
'art': getAmpRuntimeTypeParameter(win),
332330
'vis': visibilityStateCodes[visibilityState] || '0',
333-
'scr_x': roundLocations
334-
? Math.round(viewport.getScrollLeft())
335-
: viewport.getScrollLeft(),
336-
'scr_y': roundLocations
337-
? Math.round(viewport.getScrollTop())
338-
: viewport.getScrollTop(),
331+
'scr_x': Math.round(viewport.getScrollLeft()),
332+
'scr_y': Math.round(viewport.getScrollTop()),
339333
'bc': getBrowserCapabilitiesBitmap(win) || null,
340334
'debug_experiment_id':
341335
(/(?:#|,)deid=([\d,]+)/i.exec(win.location.hash) || [])[1] || null,
@@ -353,7 +347,6 @@ export function googlePageParameters(a4a, startTime, roundLocations) {
353347
* @param {string} baseUrl
354348
* @param {number} startTime
355349
* @param {!Object<string,null|number|string>} parameters
356-
* @param {boolean} roundLocations when true adx/ady/scr_x/scr_y are rounded
357350
* @param {!Array<string>=} opt_experimentIds Any experiments IDs (in addition
358351
* to those specified on the ad element) that should be included in the
359352
* request.
@@ -364,21 +357,14 @@ export function googleAdUrl(
364357
baseUrl,
365358
startTime,
366359
parameters,
367-
roundLocations,
368360
opt_experimentIds
369361
) {
370362
// TODO: Maybe add checks in case these promises fail.
371-
const blockLevelParameters = googleBlockParameters(
372-
a4a,
373-
roundLocations,
374-
opt_experimentIds
375-
);
376-
return googlePageParameters(a4a, startTime, roundLocations).then(
377-
(pageLevelParameters) => {
378-
Object.assign(parameters, blockLevelParameters, pageLevelParameters);
379-
return truncAndTimeUrl(baseUrl, parameters, startTime);
380-
}
381-
);
363+
const blockLevelParameters = googleBlockParameters(a4a, opt_experimentIds);
364+
return googlePageParameters(a4a, startTime).then((pageLevelParameters) => {
365+
Object.assign(parameters, blockLevelParameters, pageLevelParameters);
366+
return truncAndTimeUrl(baseUrl, parameters, startTime);
367+
});
382368
}
383369

384370
/**

‎build-system/global-configs/canary-config.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,5 @@
3838
"amp-ad-no-center-css": 0,
3939
"sticky-ad-padding-bottom": 1,
4040
"analytics-chunks": 1,
41-
"ad-adsense-gam-round-params": 0.1,
4241
"render-on-idle-fix": 1
4342
}

‎build-system/global-configs/prod-config.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,5 @@
3636
"amp-ad-no-center-css": 0,
3737
"analytics-chunks": 1,
3838
"sticky-ad-padding-bottom": 1,
39-
"render-on-idle-fix": 1,
40-
"ad-adsense-gam-round-params": 0.1
39+
"render-on-idle-fix": 1
4140
}

‎extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import {ResponsiveState} from './responsive-state';
4646
import {Services} from '../../../src/services';
4747
import {
4848
addExperimentIdToElement,
49-
isInExperiment,
5049
isInManualExperiment,
5150
} from '../../../ads/google/a4a/traffic-experiments';
5251
import {computedStyle, setStyles} from '../../../src/style';
@@ -68,13 +67,6 @@ const ADSENSE_BASE_URL = 'https://googleads.g.doubleclick.net/pagead/ads';
6867
/** @const {string} */
6968
const TAG = 'amp-ad-network-adsense-impl';
7069

71-
/** @const @enum {string} */
72-
const ROUND_LOCATION_PARAMS_HOLDBACK_EXP = {
73-
ID: 'ad-adsense-gam-round-params',
74-
CONTROL: '21067039',
75-
EXPERIMENT: '21067040',
76-
};
77-
7870
/**
7971
* Shared state for AdSense ad slots. This is used primarily for ad request url
8072
* parameters that depend on previous slots.
@@ -251,14 +243,6 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
251243
STICKY_AD_PADDING_BOTTOM_EXP.experiment,
252244
],
253245
},
254-
{
255-
experimentId: ROUND_LOCATION_PARAMS_HOLDBACK_EXP.ID,
256-
isTrafficEligible: () => true,
257-
branches: [
258-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.CONTROL,
259-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT,
260-
],
261-
},
262246
]);
263247
const setExps = randomlySelectUnsetExperiments(
264248
this.win,
@@ -423,10 +407,6 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
423407
'pucrd': identity.pucrd || null,
424408
...parameters,
425409
},
426-
!isInExperiment(
427-
this.element,
428-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT
429-
),
430410
experimentIds
431411
);
432412
});

‎extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js‎

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,6 @@ const ZINDEX_EXP_BRANCHES = {
146146
HOLDBACK: '21065357',
147147
};
148148

149-
/** @const @enum {string} */
150-
const ROUND_LOCATION_PARAMS_HOLDBACK_EXP = {
151-
ID: 'ad-adsense-gam-round-params',
152-
CONTROL: '21067041',
153-
EXPERIMENT: '21067042',
154-
};
155-
156149
/**
157150
* Required size to be sent with fluid requests.
158151
* @const {string}
@@ -457,14 +450,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
457450
STICKY_AD_PADDING_BOTTOM_EXP.experiment,
458451
],
459452
},
460-
{
461-
experimentId: ROUND_LOCATION_PARAMS_HOLDBACK_EXP.ID,
462-
isTrafficEligible: () => true,
463-
branches: [
464-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.CONTROL,
465-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT,
466-
],
467-
},
468453
]);
469454
const setExps = this.randomlySelectUnsetExperiments_(experimentInfoList);
470455
Object.keys(setExps).forEach(
@@ -659,12 +644,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
659644
'spsa': this.isSinglePageStoryAd
660645
? `${pageLayoutBox.width}x${pageLayoutBox.height}`
661646
: null,
662-
...googleBlockParameters(
663-
this,
664-
!this.experimentIds.includes(
665-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT
666-
)
667-
),
647+
...googleBlockParameters(this),
668648
};
669649
}
670650

@@ -778,9 +758,6 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
778758
this.getPageParameters(consentTuple, /* instances= */ undefined),
779759
rtcParams
780760
),
781-
!this.experimentIds.includes(
782-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT
783-
),
784761
this.experimentIds
785762
).then((adUrl) => this.getAdUrlDeferred.resolve(adUrl));
786763
});
@@ -1993,15 +1970,7 @@ function constructSRARequest_(a4a, instances) {
19931970
return Promise.all(
19941971
instances.map((instance) => instance.getAdUrlDeferred.promise)
19951972
)
1996-
.then(() =>
1997-
googlePageParameters(
1998-
a4a,
1999-
startTime,
2000-
!instances[0].experimentIds.includes(
2001-
ROUND_LOCATION_PARAMS_HOLDBACK_EXP.EXPERIMENT
2002-
)
2003-
)
2004-
)
1973+
.then(() => googlePageParameters(a4a, startTime))
20051974
.then((googPageLevelParameters) => {
20061975
const blockParameters = constructSRABlockParameters(instances);
20071976
return truncAndTimeUrl(

0 commit comments

Comments
 (0)