diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index 9dd8262a61d6..03fc40879052 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -68,15 +68,14 @@ var forbiddenTerms = { 'src/cookies.js', 'src/experiments.js', 'test/functional/test-cookies.js', - 'tools/experiments/experiments.js', ] }, 'setCookie\\W': { message: requiresReviewPrivacy, whitelist: [ 'src/cookies.js', + 'src/experiments.js', 'test/functional/test-cookies.js', - 'tools/experiments/experiments.js', ] }, 'eval\\(': '', diff --git a/src/experiments.js b/src/experiments.js index f83955a60c7f..a24724ee36ff 100644 --- a/src/experiments.js +++ b/src/experiments.js @@ -14,24 +14,79 @@ * limitations under the License. */ -import {getCookie} from './cookies'; +/** + * @fileoverview Experiments system allows a developer to opt-in to test + * features that are not yet fully tested. + * + * Experiments page: https://cdn.ampproject.org/experiments.html * + */ + +import {getCookie, setCookie} from './cookies'; +import {timer} from './timer'; + + +/** @const {string} */ +const COOKIE_NAME = 'AMP_EXP'; + +/** @const {number} */ +const COOKIE_MAX_AGE_DAYS = 180; // 6 month + +/** @const {time} */ +const COOKIE_EXPIRATION_INTERVAL = COOKIE_MAX_AGE_DAYS * 24 * 60 * 60 * 1000; /** * Whether the specified experiment is on or off. - * - * All experiments are opt-in. A user has to visit the experiments page and - * manually toggle the experiment on. - * TODO(dvoytenko): provide the URL for the experiments page once ready. - * * @param {!Window} win * @param {string} experimentId * @return {boolean} */ export function isExperimentOn(win, experimentId) { - const experimentCookie = getCookie(win, 'AMP_EXP'); - if (!experimentCookie) { - return false; + return getExperimentIds(win).indexOf(experimentId) != -1; +} + + +/** + * Toggles the expriment on or off. Returns the actual value of the expriment + * after toggling is done. + * @param {!Window} win + * @param {string} experimentId + * @param {boolean=} opt_on + * @return {boolean} + */ +export function toggleExperiment(win, experimentId, opt_on) { + const experimentIds = getExperimentIds(win); + const currentlyOn = experimentIds.indexOf(experimentId) != -1; + const on = opt_on !== undefined ? opt_on : !currentlyOn; + if (on != currentlyOn) { + if (on) { + experimentIds.push(experimentId); + } else { + experimentIds.splice(experimentIds.indexOf(experimentId), 1); + } + saveExperimentIds(win, experimentIds); } - return experimentCookie.split(/\s*,\s*/g).indexOf(experimentId) != -1; + return on; +} + + +/** + * Returns a set of experiment IDs currently on. + * @param {!Window} win + * @return {!Array} + */ +function getExperimentIds(win) { + const experimentCookie = getCookie(win, COOKIE_NAME); + return experimentCookie ? experimentCookie.split(/\s*,\s*/g) : []; +} + + +/** + * Saves a set of experiment IDs currently on. + * @param {!Window} win + * @param {!Array} experimentIds + */ +function saveExperimentIds(win, experimentIds) { + setCookie(win, COOKIE_NAME, experimentIds.join(','), + timer.now() + COOKIE_EXPIRATION_INTERVAL); } diff --git a/src/runtime.js b/src/runtime.js index 5d659b3ceef1..db0c71f53b0a 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -19,6 +19,7 @@ import {BaseTemplate, registerExtendedTemplate} from './template'; import {assert} from './asserts'; import {getMode} from './mode'; import {installStyles} from './styles'; +import {isExperimentOn, toggleExperiment} from './experiments'; import {performanceFor} from './performance'; import {registerElement} from './custom-element'; import {registerExtendedElement} from './extended-element'; @@ -99,6 +100,10 @@ export function adopt(global) { global.AMP.toggleRuntime = viewer.toggleRuntime.bind(viewer); /** @const */ global.AMP.resources = resourcesFor(global); + /** @const */ + global.AMP.isExperimentOn = isExperimentOn.bind(null, global); + /** @const */ + global.AMP.toggleExperiment = toggleExperiment.bind(null, global); } const viewport = viewportFor(global); diff --git a/test/functional/test-cid.js b/test/functional/test-cid.js index 4141ee175365..98e4610ef894 100644 --- a/test/functional/test-cid.js +++ b/test/functional/test-cid.js @@ -146,12 +146,12 @@ describe('cid', () => { it('should pick up the cid value from storage', () => { storage['amp-cid'] = JSON.stringify({ - cid: 'XXX', + cid: 'YYY', time: timer.now(), }); return compare( 'e2', - 'sha384(XXXhttp://www.origin.come2)'); + 'sha384(YYYhttp://www.origin.come2)'); }); it('should work without mocking', () => { @@ -309,8 +309,8 @@ describe('getSourceOrigin', () => { it('should fail on invalid source origin', () => { expect(() => { - getSourceOrigin(parseUrl('https://cdn.ampproject.org/v/xxx/')); - }).to.throw(/Expected a \. in origin http:\/\/xxx/); + getSourceOrigin(parseUrl('https://cdn.ampproject.org/v/yyy/')); + }).to.throw(/Expected a \. in origin http:\/\/yyy/); }); it('should fail on non-proxy origin', () => { diff --git a/test/functional/test-experiments.js b/test/functional/test-experiments.js index 27747b73d449..8d7c78b89754 100644 --- a/test/functional/test-experiments.js +++ b/test/functional/test-experiments.js @@ -14,7 +14,8 @@ * limitations under the License. */ -import {isExperimentOn} from '../../src/experiments'; +import {isExperimentOn, toggleExperiment} from '../../src/experiments'; +import * as sinon from 'sinon'; describe('isExperimentOn', () => { @@ -46,3 +47,66 @@ describe('isExperimentOn', () => { expectExperiment('AMP_EXP=e2 , e1', 'e1').to.be.true; }); }); + + +describe('toggleExperiment', () => { + + let sandbox; + let clock; + let expTime; + + beforeEach(() => { + sandbox = sinon.sandbox.create(); + clock = sandbox.useFakeTimers(); + clock.tick(1); + expTime = new Date(1 + 180 * 24 * 60 * 60 * 1000).toUTCString(); + }); + + afterEach(() => { + clock.restore(); + clock = null; + sandbox.restore(); + sandbox = null; + }); + + function expectToggle(cookiesString, experimentId, opt_on) { + const doc = { + cookie: cookiesString + }; + const on = toggleExperiment({document: doc}, experimentId, opt_on); + const parts = doc.cookie.split(/\s*;\s*/g); + if (parts.length > 1) { + expect(parts[1]).to.equal('path=/'); + expect(parts[2]).to.equal('expires=' + expTime); + } + return expect(`${on}; ${decodeURIComponent(parts[0])}`); + } + + it('should toggle to "on" with no cookies, malformed or empty', () => { + expectToggle(null, 'e1').to.equal('true; AMP_EXP=e1'); + expectToggle(undefined, 'e1').to.equal('true; AMP_EXP=e1'); + expectToggle('', 'e1').to.equal('true; AMP_EXP=e1'); + expectToggle('AMP_EXP', 'e1').to.equal('true; AMP_EXP=e1'); + expectToggle('AMP_EXP=', 'e1').to.equal('true; AMP_EXP=e1'); + }); + + it('should toggle "on" when value is not in the list', () => { + expectToggle('AMP_EXP=e1a,e2', 'e1').to.equal('true; AMP_EXP=e1a,e2,e1'); + }); + + it('should toggle "off" when value is in the list', () => { + expectToggle('AMP_EXP=e1', 'e1').to.equal('false; AMP_EXP='); + expectToggle('AMP_EXP=e1,e2', 'e1').to.equal('false; AMP_EXP=e2'); + expectToggle('AMP_EXP=e2,e1', 'e1').to.equal('false; AMP_EXP=e2'); + }); + + it('should set "on" when requested', () => { + expectToggle('AMP_EXP=e2', 'e1', true).to.equal('true; AMP_EXP=e2,e1'); + expectToggle('AMP_EXP=e1', 'e1', true).to.equal('true; AMP_EXP=e1'); + }); + + it('should set "off" when requested', () => { + expectToggle('AMP_EXP=e2,e1', 'e1', false).to.equal('false; AMP_EXP=e2'); + expectToggle('AMP_EXP=e1', 'e1', false).to.equal('false; AMP_EXP='); + }); +}); diff --git a/tools/experiments/README.md b/tools/experiments/README.md index 89f95957ff59..f7523288d1ab 100644 --- a/tools/experiments/README.md +++ b/tools/experiments/README.md @@ -8,3 +8,8 @@ Experiments UI is a available at: [https://cdn.ampproject.org/experiments.js](https://cdn.ampproject.org/experiments.js) +Alternatively, the expriments can be toggled in the devtools console in the development +mode using: +``` +AMP.toggleExperiment('experiment name') +``` diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 181948d28e54..501d168c4e20 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -16,11 +16,7 @@ import '../../src/polyfills'; import {onDocumentReady} from '../../src/document-state'; -import {getCookie, setCookie} from '../../src/cookies'; - - -/** @const {number} */ -const COOKIE_MAX_AGE_DAYS = 180; // 6 month +import {isExperimentOn, toggleExperiment} from '../../src/experiments'; /** @@ -90,8 +86,10 @@ function buildExperimentRow(experiment) { buttonOff.textContent = 'Off'; button.appendChild(buttonOff); - button.addEventListener('click', toggleExperiment.bind(null, experiment.id, - undefined)); + button.addEventListener('click', () => { + toggleExperiment(window, experiment.id); + update(); + }); return tr; } @@ -136,49 +134,7 @@ function updateExperimentRow(experiment) { if (!tr) { return; } - tr.setAttribute('data-on', isExperimentOn(experiment.id) ? 1 : 0); -} - - -/** - * Returns a set of experiment IDs currently on. - * @return {!Array} - */ -function getExperimentIds() { - const experimentCookie = getCookie(window, 'AMP_EXP'); - return experimentCookie ? experimentCookie.split(/\s*,\s*/g) : []; -} - - -/** - * Returns whether the experiment is on or off. - * @param {string} id - * @return {boolean} - */ -function isExperimentOn(id) { - return getExperimentIds().indexOf(id) != -1; -} - - -/** - * Toggles the expriment. - * @param {string} id - * @param {boolean=} opt_on - */ -function toggleExperiment(id, opt_on) { - const experimentIds = getExperimentIds(); - const currentlyOn = experimentIds.indexOf(id) != -1; - const on = opt_on !== undefined ? opt_on : !currentlyOn; - if (on != currentlyOn) { - if (on) { - experimentIds.push(id); - } else { - experimentIds.splice(experimentIds.indexOf(id), 1); - } - setCookie(window, 'AMP_EXP', experimentIds.join(','), - new Date().getTime() + COOKIE_MAX_AGE_DAYS * 24 * 60 * 60 * 1000); - } - update(); + tr.setAttribute('data-on', isExperimentOn(window, experiment.id) ? 1 : 0); }