From 8fdc15594202aae8106c6de1efd8bf778d7f70f0 Mon Sep 17 00:00:00 2001 From: Yuxuan Zhou Date: Fri, 25 May 2018 19:30:30 -0700 Subject: [PATCH] Separate consent state enum file (#15567) * seperate file * import * add consent.js * dep-check-fix * fix test --- ads/google/adsense.js | 5 +- ads/google/imaVideo.js | 5 +- build-system/dep-check-config.js | 2 + extensions/amp-a4a/0.1/amp-a4a.js | 2 +- extensions/amp-ad/0.1/amp-ad-3p-impl.js | 6 +- .../amp-ad/0.1/test/test-amp-ad-3p-impl.js | 13 +++-- extensions/amp-ima-video/0.1/amp-ima-video.js | 2 +- src/consent-state.js | 40 +------------ src/consent.js | 56 +++++++++++++++++++ src/custom-element.js | 1 - 10 files changed, 80 insertions(+), 52 deletions(-) create mode 100644 src/consent.js diff --git a/ads/google/adsense.js b/ads/google/adsense.js index 00ef2eb38fd4..5abc3da39d5b 100644 --- a/ads/google/adsense.js +++ b/ads/google/adsense.js @@ -15,6 +15,7 @@ */ import {ADSENSE_RSPV_WHITELISTED_HEIGHT} from './utils'; +import {CONSENT_POLICY_STATE} from '../../src/consent-state'; import {camelCaseToDash} from '../../src/string'; import {setStyles} from '../../src/style'; import {user} from '../../src/log'; @@ -72,12 +73,12 @@ export function adsense(global, data) { }); const initializer = {}; switch (global.context.initialConsentState) { - case 4: // CONSENT_POLICY_STATE.UNKNOWN + case CONSENT_POLICY_STATE.UNKNOWN: if (data['npaOnUnknownConsent'] != 'true') { // Unknown w/o NPA results in no ad request. return; } - case 2: // CONSENT_POLICY_STATE.INSUFFICIENT + case CONSENT_POLICY_STATE.INSUFFICIENT: (global.adsbygoogle = global.adsbygoogle || []) ['requestNonPersonalizedAds'] = true; break; diff --git a/ads/google/imaVideo.js b/ads/google/imaVideo.js index a0409e2732c7..b27d94d85b34 100644 --- a/ads/google/imaVideo.js +++ b/ads/google/imaVideo.js @@ -14,6 +14,7 @@ * limitations under the License. */ +import {CONSENT_POLICY_STATE} from '../../src/consent-state'; import {ImaPlayerData} from './ima-player-data'; import {camelCaseToTitleCase, px, setStyle, setStyles} from '../../src/style'; import {isObject} from '../../src/types'; @@ -571,11 +572,11 @@ function onBigPlayTouchMove() { export function requestAds() { adsRequested = true; adRequestFailed = false; - if (consentState == 4) { // UNKNOWN + if (consentState == CONSENT_POLICY_STATE.UNKNOWN) { // We're unaware of the user's consent state - do not request ads. imaLoadAllowed = false; return; - } else if (consentState == 2) { // INSUFFICIENT + } else if (consentState == CONSENT_POLICY_STATE.INSUFFICIENT) { // User has provided consent state but has not consented to personalized // ads. adsRequest.adTagUrl += '&npa=1'; diff --git a/build-system/dep-check-config.js b/build-system/dep-check-config.js index 6a303b3ad78e..17c28ca63b79 100644 --- a/build-system/dep-check-config.js +++ b/build-system/dep-check-config.js @@ -104,6 +104,7 @@ exports.rules = [ '3p/**->src/3p-frame-messaging.js', '3p/**->src/observable.js', '3p/**->src/amp-events.js', + '3p/**->src/consent-state.js', '3p/polyfills.js->src/polyfills/math-sign.js', '3p/polyfills.js->src/polyfills/object-assign.js', '3p/messaging.js->src/event-helper.js', @@ -129,6 +130,7 @@ exports.rules = [ 'ads/**->src/types.js', 'ads/**->src/string.js', 'ads/**->src/style.js', + 'ads/**->src/consent-state.js', 'ads/google/adsense-amp-auto-ads.js->src/experiments.js', 'ads/google/doubleclick.js->src/experiments.js', // ads/google/a4a doesn't contain 3P ad code and should probably move diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js index 284a679c8b7d..a45fb4e4b739 100644 --- a/extensions/amp-a4a/0.1/amp-a4a.js +++ b/extensions/amp-a4a/0.1/amp-a4a.js @@ -17,7 +17,6 @@ import {A4AVariableSource} from './a4a-variable-source'; import { CONSENT_POLICY_STATE, // eslint-disable-line no-unused-vars - getConsentPolicyState, } from '../../../src/consent-state'; import {Layout, isLayoutSizeDefined} from '../../../src/layout'; import {LayoutPriority} from '../../../src/layout'; @@ -43,6 +42,7 @@ import { } from '../../amp-ad/0.1/concurrent-load'; import {getBinaryType} from '../../../src/experiments'; import {getBinaryTypeNumericalCode} from '../../../ads/google/a4a/utils'; +import {getConsentPolicyState} from '../../../src/consent'; import {getContextMetadata} from '../../../src/iframe-attributes'; import {getMode} from '../../../src/mode'; import {tryResolve} from '../../../src/utils/promise'; diff --git a/extensions/amp-ad/0.1/amp-ad-3p-impl.js b/extensions/amp-ad/0.1/amp-ad-3p-impl.js index ea1200dbbabb..199801699276 100644 --- a/extensions/amp-ad/0.1/amp-ad-3p-impl.js +++ b/extensions/amp-ad/0.1/amp-ad-3p-impl.js @@ -18,8 +18,6 @@ import {AmpAdUIHandler} from './amp-ad-ui'; import {AmpAdXOriginIframeHandler} from './amp-ad-xorigin-iframe-handler'; import { CONSENT_POLICY_STATE, // eslint-disable-line no-unused-vars - getConsentPolicySharedData, - getConsentPolicyState, } from '../../../src/consent-state'; import { Layout, // eslint-disable-line no-unused-vars @@ -40,6 +38,10 @@ import { incrementLoadingAds, is3pThrottled, } from './concurrent-load'; +import { + getConsentPolicySharedData, + getConsentPolicyState, +} from '../../../src/consent'; import {getIframe} from '../../../src/3p-frame'; import { googleLifecycleReporterFactory, diff --git a/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js b/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js index 46d2a8222fee..0216aed6f9ea 100644 --- a/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js +++ b/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js @@ -17,14 +17,16 @@ import '../../../amp-ad/0.1/amp-ad'; import '../../../amp-sticky-ad/1.0/amp-sticky-ad'; import * as adCid from '../../../../src/ad-cid'; -import * as consent from '../../../../src/consent-state'; +import * as consent from '../../../../src/consent'; import * as lolex from 'lolex'; import {AmpAd3PImpl} from '../amp-ad-3p-impl'; +import {CONSENT_POLICY_STATE} from '../../../../src/consent-state'; import {LayoutPriority} from '../../../../src/layout'; import {adConfig} from '../../../../ads/_config'; import {createElementWithAttributes} from '../../../../src/dom'; import {macroTask} from '../../../../testing/yield'; import {stubService} from '../../../../testing/test-helper'; +import {user} from '../../../../src/log'; function createAmpAd(win, attachToAmpdoc = false, ampdoc) { const ampAdElement = createElementWithAttributes(win.document, 'amp-ad', { @@ -140,7 +142,7 @@ describes.realWin('amp-ad-3p-impl', { it('should propagate consent state to ad iframe', () => { ad3p.element.setAttribute('data-block-on-consent', ''); sandbox.stub(consent, 'getConsentPolicyState') - .resolves(consent.CONSENT_POLICY_STATE.SUFFICIENT); + .resolves(CONSENT_POLICY_STATE.SUFFICIENT); sandbox.stub(consent, 'getConsentPolicySharedData') .resolves({a: 1, b: 2}); @@ -151,7 +153,7 @@ describes.realWin('amp-ad-3p-impl', { expect(data).to.be.ok; expect(data._context).to.be.ok; expect(data._context.initialConsentState) - .to.equal(consent.CONSENT_POLICY_STATE.SUFFICIENT); + .to.equal(CONSENT_POLICY_STATE.SUFFICIENT); expect(data._context.consentSharedData) .to.deep.equal({a: 1, b: 2}); }); @@ -237,6 +239,7 @@ describes.realWin('amp-ad-3p-impl', { win.document.head.appendChild(meta); ad3p.config.remoteHTMLDisabled = true; ad3p.onLayoutMeasure(); + sandbox.stub(user(), 'error'); return ad3p.layoutCallback().then(() => { expect(win.document.querySelector('iframe[src="' + 'http://ads.localhost:9876/dist.3p/current/frame.max.html"]')) @@ -300,7 +303,9 @@ describes.realWin('amp-ad-3p-impl', { win.document.head.appendChild(meta); ad3p.config.remoteHTMLDisabled = true; ad3p.buildCallback(); - ad3p.preconnectCallback(); + allowConsoleError(() => { + ad3p.preconnectCallback(); + }); return whenFirstVisible.then(() => { expect(Array.from(win.document.querySelectorAll('link[rel=preload]')) .some(link => link.href == diff --git a/extensions/amp-ima-video/0.1/amp-ima-video.js b/extensions/amp-ima-video/0.1/amp-ima-video.js index 4f8d18bf832d..4673197ab29c 100644 --- a/extensions/amp-ima-video/0.1/amp-ima-video.js +++ b/extensions/amp-ima-video/0.1/amp-ima-video.js @@ -25,7 +25,7 @@ import { removeElement, } from '../../../src/dom'; import {dict} from '../../../src/utils/object'; -import {getConsentPolicyState} from '../../../src/consent-state'; +import {getConsentPolicyState} from '../../../src/consent'; import { getData, listen, diff --git a/src/consent-state.js b/src/consent-state.js index e29c6f2631e9..91b053c3e9bc 100644 --- a/src/consent-state.js +++ b/src/consent-state.js @@ -14,7 +14,7 @@ * limitations under the License. */ -import {Services} from './services'; +// This file will be imported by 3P scripts. /** * Possible consent policy state to proceed with. @@ -27,41 +27,3 @@ export const CONSENT_POLICY_STATE = { UNKNOWN_NOT_REQUIRED: 3, UNKNOWN: 4, }; - -// TODO(@zhouyx): Move following functions to a different file - -/** - * Returns a promise that resolve when all consent state the policy wait - * for resolve. Or if consent service is not available. - * @param {!./service/ampdoc-impl.AmpDoc} ampdoc - * @param {string} policyId - * @return {!Promise} - */ -export function getConsentPolicyState(ampdoc, policyId) { - return Services.consentPolicyServiceForDocOrNull(ampdoc) - .then(consentPolicy => { - if (!consentPolicy) { - return null; - } - return consentPolicy.whenPolicyResolved( - /** @type {string} */ (policyId)); - }); -} - -/** - * Returns a promise that resolves to a sharedData retrieved from consent - * remote endpoint. - * @param {!./service/ampdoc-impl.AmpDoc} ampdoc - * @param {string} policyId - * @return {!Promise} - */ -export function getConsentPolicySharedData(ampdoc, policyId) { - return Services.consentPolicyServiceForDocOrNull(ampdoc) - .then(consentPolicy => { - if (!consentPolicy) { - return null; - } - return consentPolicy.getMergedSharedData( - /** @type {string} */ (policyId)); - }); -} diff --git a/src/consent.js b/src/consent.js new file mode 100644 index 000000000000..ac4879db9357 --- /dev/null +++ b/src/consent.js @@ -0,0 +1,56 @@ +/** + * Copyright 2018 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + CONSENT_POLICY_STATE, // eslint-disable-line no-unused-vars +} from './consent-state'; +import {Services} from './services'; + +/** + * Returns a promise that resolve when all consent state the policy wait + * for resolve. Or if consent service is not available. + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc + * @param {string} policyId + * @return {!Promise} + */ +export function getConsentPolicyState(ampdoc, policyId) { + return Services.consentPolicyServiceForDocOrNull(ampdoc) + .then(consentPolicy => { + if (!consentPolicy) { + return null; + } + return consentPolicy.whenPolicyResolved( + /** @type {string} */ (policyId)); + }); +} + +/** + * Returns a promise that resolves to a sharedData retrieved from consent + * remote endpoint. + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc + * @param {string} policyId + * @return {!Promise} + */ +export function getConsentPolicySharedData(ampdoc, policyId) { + return Services.consentPolicyServiceForDocOrNull(ampdoc) + .then(consentPolicy => { + if (!consentPolicy) { + return null; + } + return consentPolicy.getMergedSharedData( + /** @type {string} */ (policyId)); + }); +} diff --git a/src/custom-element.js b/src/custom-element.js index 8760896d12fa..0db74d003df6 100644 --- a/src/custom-element.js +++ b/src/custom-element.js @@ -16,7 +16,6 @@ import * as dom from './dom'; import {AmpEvents} from './amp-events'; - import {CommonSignals} from './common-signals'; import {ElementStub} from './element-stub'; import {