Skip to content

Commit

Permalink
Implement ad size optimization for AdSense.
Browse files Browse the repository at this point in the history
See #23568.
  • Loading branch information
riklund committed Sep 9, 2019
1 parent ec0bc77 commit 099ce8a
Show file tree
Hide file tree
Showing 5 changed files with 399 additions and 39 deletions.
4 changes: 3 additions & 1 deletion build-system/tasks/presubmit-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -482,15 +482,17 @@ const forbiddenTerms = {
// https://amp.dev/documentation/guides-and-tutorials/learn/a4a_spec
'src/services.js',
'src/service/cid-impl.js',
'extensions/amp-user-notification/0.1/amp-user-notification.js',
'extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js',
'extensions/amp-app-banner/0.1/amp-app-banner.js',
'extensions/amp-consent/0.1/consent-state-manager.js',
'extensions/amp-user-notification/0.1/amp-user-notification.js',
],
},
'localStorage': {
message: requiresReviewPrivacy,
whitelist: [
'extensions/amp-access/0.1/amp-access-iframe.js',
'extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js',
'extensions/amp-script/0.1/amp-script.js',
'extensions/amp-web-push/0.1/amp-web-push-helper-frame.js',
'extensions/amp-web-push/0.1/amp-web-push-permission-dialog.js',
Expand Down
4 changes: 3 additions & 1 deletion extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,9 @@ export class AmpA4A extends AMP.BaseElement {
return this.isRelayoutNeededFlag;
}

/** @override */
/** @override
@return {!Promise|undefined}
*/
buildCallback() {
this.creativeSize_ = {
width: this.element.getAttribute('width'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ const ADSENSE_BASE_URL = 'https://googleads.g.doubleclick.net/pagead/ads';
/** @const {string} */
const TAG = 'amp-ad-network-adsense-impl';

/** @const {!{branch: string, control: string, experiment: string}}
@visibleForTesting
*/
export const AD_SIZE_OPTIMIZATION_EXP = {
branch: 'adsense-ad-size-optimization',
control: '368226510',
experiment: '368226511',
};

/**
* Shared state for AdSense ad slots. This is used primarily for ad request url
* parameters that depend on previous slots.
Expand Down Expand Up @@ -170,6 +179,12 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
* @private {boolean}
*/
this.shouldSandbox_ = false;

/**
* Testing callback invoked when auto ad size settings are persisted in
* localstorage.
* @private {function()} */
this.onAutoAdSizeSettingsStored_ = () => {};
}

/**
Expand Down Expand Up @@ -247,35 +262,165 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
return getAmpAdRenderOutsideViewport(this.element) || 3;
}

/** @override */
/** @override
@return {!Promise|undefined}.
*/
buildCallback() {
super.buildCallback();
this.identityTokenPromise_ = Services.viewerForDoc(this.getAmpDoc())
.whenFirstVisible()
.then(() =>
getIdentityToken(this.win, this.getAmpDoc(), super.getConsentPolicy())
);
this.autoFormat_ = this.element.getAttribute('data-auto-format') || '';

if (this.isResponsive_()) {
// Attempt to resize to the correct height. The width should already be
// 100vw, but is fixed here so that future resizes of the viewport don't
// affect it.
const viewportSize = this.getViewport().getSize();
return this.attemptChangeSize(
AmpAdNetworkAdsenseImpl.getResponsiveHeightForContext_(
this.autoFormat_,
viewportSize,
this.element,
this.isInResponsiveHeightFixExperimentBranch()
),
viewportSize.width
).catch(() => {});
return this.maybeUpgradeToResponsive_().then(() => {
this.autoFormat_ = this.element.getAttribute('data-auto-format') || '';

if (this.isResponsive_()) {
// Attempt to resize to the correct height. The width should already be
// 100vw, but is fixed here so that future resizes of the viewport don't
// affect it.
const viewportSize = this.getViewport().getSize();
return this.attemptChangeSize(
AmpAdNetworkAdsenseImpl.getResponsiveHeightForContext_(
this.autoFormat_,
viewportSize,
this.element,
this.isInResponsiveHeightFixExperimentBranch()
),
viewportSize.width
).catch(() => {});
}

// This should happen last, as some diversion criteria rely on some of the
// preceding logic (specifically responsive logic).
this.divertExperiments();
this.maybeAddSinglePassExperiment();
});
}

/**
* Upgrades the ad unit to responsive if there is an opt-in setting in localstorage.
* See https://github.com/ampproject/amphtml/issues/23568 for design.
* @return {!Promise} a promise that resolves when any upgrade is complete.
* @private
*/
maybeUpgradeToResponsive_() {
if (!this.isInAdSizeOptimizationExperimentBranch()) {
return Promise.resolve();
}
// If the ad unit is already responsive, there's nothing to do.
// Note that we can't use this.autoFormat_ here because it's not
// populated yet.
if (this.element.hasAttribute('data-auto-format')) {
return Promise.resolve();
}
// This should happen last, as some diversion criteria rely on some of the
// preceding logic (specifically responsive logic).
this.divertExperiments();
this.maybeAddSinglePassExperiment();
return (
Services.storageForDoc(this.element)
.then(storage => storage.get(this.getAdSizeOptimizationStorageKey()))
.then(isAdSizeOptimizationEnabled => {
if (isAdSizeOptimizationEnabled) {
this.upgradeToResponsive_();
}
})
// Do nothing if we fail to read localstorage.
.catch(() => {
dev().warn(TAG, 'Failed to look up publisher ad size settings.');
})
);
}

/**
* Actually upgrades the ad unit to responsive.
*
* @private
*/
upgradeToResponsive_() {
this.element.setAttribute('height', ADSENSE_RSPV_WHITELISTED_HEIGHT);
this.element.setAttribute('width', '100vw');
this.element.setAttribute('data-full-width', '');
this.element.setAttribute('data-auto-format', 'rspv');

devAssert(this.isValidElement(), 'Upgraded element is not valid.');
}

/**
* Sets up a listener for a pingback of publisher settings in the ad response and
* writing such settings to localstorage.
*
* Note that we can have multiple listeners on the same page, which is okay because
* the settings for publishers should not change between different slots.
*
* See https://github.com/ampproject/amphtml/issues/23568 for design.
*
* @private
*/
attachSettingsListener_() {
const listener = event => {
if (event['source'] != this.iframe.contentWindow) {
return;
}
const data = getData(event);
// data will look like this:
// {
// 'googMsgType': 'adsense-settings',
// 'adClient': 'ca-pub-123',
// 'enableAutoAdSize': '1'
// }
if (!!data && data['googMsgType'] != 'adsense-settings') {
return;
}
if (data['adClient'] != this.getAdClientId()) {
return;
}
const autoAdSizeStatus = data['enableAutoAdSize'] == '1';
this.win.removeEventListener('message', listener);

Services.storageForDoc(this.element)
.then(storage =>
storage
.set(this.getAdSizeOptimizationStorageKey(), autoAdSizeStatus)
.then(() => {
dev().info(
TAG,
`Saved publisher auto ad size setting: ${autoAdSizeStatus}`
);
this.onAutoAdSizeSettingsStored_();
})
)
// Do nothing if we fail to write to localstorage.
.catch(() => {
dev().warn(TAG, 'Failed to persist publisher auto ad size setting.');
});
};
this.win.addEventListener('message', listener);
}

/**
* Selects into the ad size optimization experiment.
* Note that this needs to be done before responsive sizing, so it must
* be separate from divertExperiments below.
* @return {boolean}
*/
isInAdSizeOptimizationExperimentBranch() {
const experimentInfoMap = /** @type {!Object<string,
!../../../src/experiments.ExperimentInfo>} */ ({
[[AD_SIZE_OPTIMIZATION_EXP.branch]]: {
isTrafficEligible: () => this.isResponsive_(),
branches: [
[AD_SIZE_OPTIMIZATION_EXP.control],
[AD_SIZE_OPTIMIZATION_EXP.experiment],
],
},
});
const setExps = randomlySelectUnsetExperiments(this.win, experimentInfoMap);
Object.keys(setExps).forEach(expName =>
addExperimentIdToElement(setExps[expName], this.element)
);
return (
getExperimentBranch(this.win, AD_SIZE_OPTIMIZATION_EXP.branch) ==
AD_SIZE_OPTIMIZATION_EXP.experiment
);
}

/** @override */
Expand Down Expand Up @@ -338,6 +483,26 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
);
}

/**
* @return {string} ad client ID for the current ad unit.
*/
getAdClientId() {
const adClientId = (
this.element.getAttribute('data-ad-client') || ''
).toLowerCase();
if (!/^ca-/i.test(adClientId)) {
return `ca-${adClientId}`;
}
return adClientId;
}

/**
* @return {string} ad size optimization storage key.
*/
getAdSizeOptimizationStorageKey() {
return `aas-${this.getAdClientId()}`;
}

/** @override */
getAdUrl(consentState) {
if (
Expand All @@ -351,12 +516,7 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
// validateData, from 3p/3p/js, after moving it someplace common.
const startTime = Date.now();
const global = this.win;
let adClientId = this.element.getAttribute('data-ad-client');
// Ensure client id format: lower case with 'ca-' prefix.
adClientId = adClientId.toLowerCase();
if (adClientId.substring(0, 3) != 'ca-') {
adClientId = 'ca-' + adClientId;
}
const adClientId = this.getAdClientId();
const adTestOn =
this.element.getAttribute('data-adtest') ||
isInManualExperiment(this.element);
Expand Down Expand Up @@ -542,6 +702,9 @@ export class AmpAdNetworkAdsenseImpl extends AmpA4A {
/** @override */
onCreativeRender(creativeMetaData) {
super.onCreativeRender(creativeMetaData);
if (this.isInAdSizeOptimizationExperimentBranch()) {
this.attachSettingsListener_();
}
this.isAmpCreative_ = !!creativeMetaData;
if (
creativeMetaData &&
Expand Down

0 comments on commit 099ce8a

Please sign in to comment.