From 4c02e246f7872075076b6c8c22a91a71f1131701 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Fri, 15 Jan 2016 13:54:32 -0800 Subject: [PATCH] isDevChannel method and whitelist for access --- build-system/tasks/presubmit-checks.js | 17 ++++++++++++++ extensions/amp-access/0.1/amp-access.js | 5 +++-- src/experiments.js | 30 +++++++++++++++++++++++++ test/functional/test-experiments.js | 25 ++++++++++++++++++++- 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index 520c9f26bb6c..f2c70fffe5cc 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -33,6 +33,9 @@ var privateServiceFactory = 'This service should only be installed in ' + 'the whitelisted files. Other modules should use a public function ' + 'typically called serviceNameFor.'; +var shouldNeverBeUsed = + 'Usage of this API is not allowed - only for internal purposes.'; + // Terms that must not appear in our source files. var forbiddenTerms = { 'DO NOT SUBMIT': '', @@ -151,6 +154,20 @@ var forbiddenTerms = { 'tools/experiments/experiments.js', ] }, + 'isDevChannel\\W': { + message: requiresReviewPrivacy, + whitelist: [ + 'extensions/amp-access/0.1/amp-access.js', + 'src/experiments.js', + 'tools/experiments/experiments.js', + ] + }, + 'isDevChannelVersionDoNotUse_\\W': { + message: shouldNeverBeUsed, + whitelist: [ + 'src/experiments.js', + ] + }, 'eval\\(': '', 'localStorage': { message: requiresReviewPrivacy, diff --git a/extensions/amp-access/0.1/amp-access.js b/extensions/amp-access/0.1/amp-access.js index 1abf4b575533..5801da62b247 100644 --- a/extensions/amp-access/0.1/amp-access.js +++ b/extensions/amp-access/0.1/amp-access.js @@ -23,7 +23,7 @@ import {documentStateFor} from '../../../src/document-state'; import {evaluateAccessExpr} from './access-expr'; import {getService} from '../../../src/service'; import {installStyles} from '../../../src/styles'; -import {isExperimentOn} from '../../../src/experiments'; +import {isDevChannel, isExperimentOn} from '../../../src/experiments'; import {listenOnce} from '../../../src/event-helper'; import {log} from '../../../src/log'; import {onDocumentReady} from '../../../src/document-state'; @@ -91,7 +91,8 @@ export class AccessService { installStyles(this.win.document, $CSS$, () => {}); /** @const @private {boolean} */ - this.isExperimentOn_ = isExperimentOn(this.win, EXPERIMENT); + this.isExperimentOn_ = (isExperimentOn(this.win, EXPERIMENT) || + isDevChannel(this.win)); const accessElement = document.getElementById('amp-access'); diff --git a/src/experiments.js b/src/experiments.js index a24724ee36ff..2b20b0157805 100644 --- a/src/experiments.js +++ b/src/experiments.js @@ -34,6 +34,36 @@ const COOKIE_MAX_AGE_DAYS = 180; // 6 month /** @const {time} */ const COOKIE_EXPIRATION_INTERVAL = COOKIE_MAX_AGE_DAYS * 24 * 60 * 60 * 1000; +/** @const {string} */ +const CANARY_EXPERIMENT_ID = 'dev-channel'; + + +/** + * Whether the scripts come from a dev channel. + * @param {!Window} win + * @return {boolean} + */ +export function isDevChannel(win) { + if (isExperimentOn(win, CANARY_EXPERIMENT_ID)) { + return true; + } + if (isDevChannelVersionDoNotUse_('$internalRuntimeVersion$')) { + return true; + } + return false; +} + + +/** + * Whether the version corresponds to the dev-channel binary. + * @param {string} version + * @return {boolean} + * @private Visible for testing only! + */ +export function isDevChannelVersionDoNotUse_(version) { + return !!version.match(/\-canary$/); +} + /** * Whether the specified experiment is on or off. diff --git a/test/functional/test-experiments.js b/test/functional/test-experiments.js index 8d7c78b89754..e2e3b38b68fb 100644 --- a/test/functional/test-experiments.js +++ b/test/functional/test-experiments.js @@ -14,7 +14,8 @@ * limitations under the License. */ -import {isExperimentOn, toggleExperiment} from '../../src/experiments'; +import {isDevChannel, isDevChannelVersionDoNotUse_, + isExperimentOn, toggleExperiment} from '../../src/experiments'; import * as sinon from 'sinon'; @@ -110,3 +111,25 @@ describe('toggleExperiment', () => { expectToggle('AMP_EXP=e1', 'e1', false).to.equal('false; AMP_EXP='); }); }); + + +describe('isDevChannel', () => { + + function expectDevChannel(cookiesString) { + return expect(isDevChannel({ + document: { + cookie: cookiesString + } + })); + } + + it('should return value based on cookie', () => { + expectDevChannel('AMP_EXP=other').to.be.false; + expectDevChannel('AMP_EXP=dev-channel').to.be.true; + }); + + it('should return value based on binary version', () => { + expect(isDevChannelVersionDoNotUse_('123456789')).to.be.false; + expect(isDevChannelVersionDoNotUse_('123456789-canary')).to.be.true; + }); +});