diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index a23f0775712db..9ff71c56a126a 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -477,6 +477,7 @@ const forbiddenTerms = { 'localStorage': { message: requiresReviewPrivacy, whitelist: [ + 'src/experiments.js', 'src/service/cid-impl.js', 'src/service/storage-impl.js', 'testing/fake-dom.js', @@ -923,23 +924,24 @@ const forbiddenTermsSrcInclusive = { 'code. Use a property of urls from src/config.js instead.', whitelist: [ 'ads/_a4a-config.js', - 'build-system/app.js', - 'build-system/app-index/amphtml-helpers.js', 'build-system/amp4test.js', + 'build-system/app-index/amphtml-helpers.js', + 'build-system/app-video-testbench.js', + 'build-system/app.js', + 'build-system/shadow-viewer.js', + 'build-system/tasks/check-links.js', + 'build-system/tasks/extension-generator/index.js', + 'build-system/tasks/helpers.js', 'dist.3p/current/integration.js', 'extensions/amp-iframe/0.1/amp-iframe.js', 'src/3p-frame.js', 'src/config.js', 'testing/local-amp-chrome-extension/background.js', 'tools/errortracker/errortracker.go', + 'tools/experiments/experiments.js', 'validator/engine/validator-in-browser.js', 'validator/nodejs/index.js', 'validator/webui/serve-standalone.go', - 'build-system/app-video-testbench.js', - 'build-system/shadow-viewer.js', - 'build-system/tasks/check-links.js', - 'build-system/tasks/extension-generator/index.js', - 'build-system/tasks/helpers.js', ], }, '\\<\\<\\<\\<\\<\\<': { diff --git a/src/experiments.js b/src/experiments.js index 81fb8aabfbbf6..dee91e320c6be 100644 --- a/src/experiments.js +++ b/src/experiments.js @@ -21,18 +21,16 @@ * Experiments page: https://cdn.ampproject.org/experiments.html * */ -import {getCookie, setCookie} from './cookies'; +import {dev, user} from './log'; +import {getMode} from './mode'; import {hasOwn} from './utils/object'; import {parseQueryString} from './url'; /** @const {string} */ -const COOKIE_NAME = 'AMP_EXP'; +const TAG = 'EXPERIMENTS'; -/** @const {number} */ -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 LOCAL_STORAGE_KEY = 'amp-experiment-toggles'; /** @const {string} */ const TOGGLES_WINDOW_PROPERTY = '__AMP__EXPERIMENT_TOGGLES'; @@ -84,7 +82,7 @@ export function isExperimentOn(win, experimentId) { * @param {boolean=} opt_on * @param {boolean=} opt_transientExperiment Whether to toggle the * experiment state "transiently" (i.e., for this page load only) or - * durably (by saving the experiment IDs to the cookie after toggling). + * durably (by saving the experiment IDs after toggling). * Default: false (save durably). * @return {boolean} New state for experimentId. */ @@ -101,17 +99,27 @@ export function toggleExperiment( toggles[experimentId] = on; if (!opt_transientExperiment) { - const cookieToggles = getExperimentTogglesFromCookie(win); - cookieToggles[experimentId] = on; - saveExperimentTogglesToCookie(win, cookieToggles); + const storedToggles = getExperimentToggles(win); + storedToggles[experimentId] = on; + saveExperimentToggles(win, storedToggles); + // Avoid affecting tests that spy/stub warn(). + if (!getMode().test) { + user().warn( + TAG, + '"%s" experiment %s for the domain "%s". See: https://amp.dev/documentation/guides-and-tutorials/learn/experimental', + experimentId, + on ? 'enabled' : 'disabled', + win.location.hostname + ); + } } } return on; } /** - * Calculate whether the experiment is on or off based off of the - * cookieFlag or the global config frequency given. + * Calculate whether the experiment is on or off based off of its default value, + * stored overriden value, or the global config frequency given. * @param {!Window} win * @return {!Object} */ @@ -151,7 +159,7 @@ export function experimentToggles(win) { } } - Object.assign(toggles, getExperimentTogglesFromCookie(win)); + Object.assign(toggles, getExperimentToggles(win)); if ( win.AMP_CONFIG && @@ -189,9 +197,16 @@ export function experimentTogglesOrNull(win) { * @param {!Window} win * @return {!Object} */ -function getExperimentTogglesFromCookie(win) { - const experimentCookie = getCookie(win, COOKIE_NAME); - const tokens = experimentCookie ? experimentCookie.split(/\s*,\s*/g) : []; +function getExperimentToggles(win) { + let experimentsString = ''; + try { + if ('localStorage' in win) { + experimentsString = win.localStorage.getItem(LOCAL_STORAGE_KEY); + } + } catch (e) { + dev().expectedError(TAG, 'localStorage not supported.'); + } + const tokens = experimentsString ? experimentsString.split(/\s*,\s*/g) : []; const toggles = Object.create(null); for (let i = 0; i < tokens.length; i++) { @@ -204,7 +219,6 @@ function getExperimentTogglesFromCookie(win) { toggles[tokens[i]] = true; } } - return toggles; } @@ -213,33 +227,28 @@ function getExperimentTogglesFromCookie(win) { * @param {!Window} win * @param {!Object} toggles */ -function saveExperimentTogglesToCookie(win, toggles) { +function saveExperimentToggles(win, toggles) { const experimentIds = []; for (const experiment in toggles) { experimentIds.push((toggles[experiment] === false ? '-' : '') + experiment); } - - setCookie( - win, - COOKIE_NAME, - experimentIds.join(','), - Date.now() + COOKIE_EXPIRATION_INTERVAL, - { - // Set explicit domain, so the cookie gets send to sub domains. - domain: win.location.hostname, - allowOnProxyOrigin: true, + try { + if ('localStorage' in win) { + win.localStorage.setItem(LOCAL_STORAGE_KEY, experimentIds.join(',')); } - ); + } catch (e) { + user().error(TAG, 'Failed to save experiments to localStorage.'); + } } /** - * See getExperimentTogglesFromCookie(). + * See getExperimentToggles(). * @param {!Window} win * @return {!Object} * @visibleForTesting */ -export function getExperimentToglesFromCookieForTesting(win) { - return getExperimentTogglesFromCookie(win); +export function getExperimentTogglesForTesting(win) { + return getExperimentToggles(win); } /** @@ -248,9 +257,7 @@ export function getExperimentToglesFromCookieForTesting(win) { * @visibleForTesting */ export function resetExperimentTogglesForTesting(win) { - setCookie(win, COOKIE_NAME, '', 0, { - domain: win.location.hostname, - }); + saveExperimentToggles(win, {}); win[TOGGLES_WINDOW_PROPERTY] = null; } diff --git a/test/unit/test-cid.js b/test/unit/test-cid.js index 2465c897829ea..372ee450fc492 100644 --- a/test/unit/test-cid.js +++ b/test/unit/test-cid.js @@ -75,7 +75,8 @@ describe('cid', () => { storage[key] = value; }, getItem: key => { - expect(key).to.equal('amp-cid'); + // isExperimentOn() in the code paths causes "amp-experiment-toggles". + expect(['amp-cid', 'amp-experiment-toggles']).to.contain(key); return storage[key]; }, }, diff --git a/test/unit/test-experiments.js b/test/unit/test-experiments.js index 52f2c422dc912..e9fab0ff3bf16 100644 --- a/test/unit/test-experiments.js +++ b/test/unit/test-experiments.js @@ -19,7 +19,7 @@ import { experimentToggles, getBinaryType, getExperimentBranch, - getExperimentToglesFromCookieForTesting, + getExperimentTogglesForTesting, isCanary, isExperimentOn, randomlySelectUnsetExperiments, @@ -28,12 +28,20 @@ import { } from '../../src/experiments'; import {createElementWithAttributes} from '../../src/dom'; +function fakeLocalStorage(initial = {}) { + const state = Object.assign({}, initial); + return { + getItem: key => (key in state ? state[key] : null), + setItem: (key, value) => (state[key] = value), + }; +} + describe('experimentToggles', () => { it('should return experiment status map', () => { const win = { - document: { - cookie: 'AMP_EXP=-exp3,exp4,exp5', - }, + localStorage: fakeLocalStorage({ + 'amp-experiment-toggles': '-exp3,exp4,exp5', + }), AMP_CONFIG: { exp1: 1, exp2: 0, @@ -55,9 +63,9 @@ describe('experimentToggles', () => { it('should cache experiment toggles on window', () => { const win = { - document: { - cookie: 'AMP_EXP=-exp3,exp4,exp5', - }, + localStorage: fakeLocalStorage({ + 'amp-experiment-toggles': '-exp3,exp4,exp5', + }), AMP_CONFIG: { exp1: 1, exp2: 0, @@ -103,9 +111,7 @@ describe('isExperimentOn', () => { beforeEach(() => { sandbox = sinon.sandbox; win = { - document: { - cookie: '', - }, + localStorage: fakeLocalStorage(), AMP_CONFIG: {}, location: { hash: '', @@ -118,9 +124,9 @@ describe('isExperimentOn', () => { sandbox.restore(); }); - function expectExperiment(cookieString, experimentId) { + function expectExperiment(storedString, experimentId) { resetExperimentTogglesForTesting(win); - win.document.cookie = cookieString; + win.localStorage.setItem('amp-experiment-toggles', storedString); return expect(isExperimentOn(win, /*OK*/ experimentId)); } @@ -129,37 +135,35 @@ describe('isExperimentOn', () => { expectExperiment(null, 'e1').to.be.false; expectExperiment(undefined, 'e1').to.be.false; expectExperiment('', 'e1').to.be.false; - expectExperiment('AMP_EXP', 'e1').to.be.false; - expectExperiment('AMP_EXP=', 'e1').to.be.false; }); it('should return "off" when value is not in the list', () => { - expectExperiment('AMP_EXP=e1a,e2', 'e1').to.be.false; + expectExperiment('e1a,e2', 'e1').to.be.false; }); it('should return "on" when value is in the list', () => { - expectExperiment('AMP_EXP=e1', 'e1').to.be.true; - expectExperiment('AMP_EXP=e1,e2', 'e1').to.be.true; - expectExperiment('AMP_EXP=e2,e1', 'e1').to.be.true; - expectExperiment('AMP_EXP=e2 , e1', 'e1').to.be.true; + expectExperiment('e1', 'e1').to.be.true; + expectExperiment('e1,e2', 'e1').to.be.true; + expectExperiment('e2,e1', 'e1').to.be.true; + expectExperiment('e2 , e1', 'e1').to.be.true; }); it('should return "off" when disabling value is in the list', () => { - expectExperiment('AMP_EXP=-e1', 'e1').to.be.false; - expectExperiment('AMP_EXP=e2,-e1', 'e1').to.be.false; - expectExperiment('AMP_EXP=-e1,e2', 'e1').to.be.false; - expectExperiment('AMP_EXP=e2 , -e1', 'e1').to.be.false; + expectExperiment('-e1', 'e1').to.be.false; + expectExperiment('e2,-e1', 'e1').to.be.false; + expectExperiment('-e1,e2', 'e1').to.be.false; + expectExperiment('e2 , -e1', 'e1').to.be.false; }); }); describe('with global flag', () => { it('should prioritize cookie flag', () => { win.AMP_CONFIG['e1'] = true; - expectExperiment('AMP_EXP=e1', 'e1').to.be.true; + expectExperiment('e1', 'e1').to.be.true; }); it('should fall back to global flag', () => { - const cookie = 'AMP_EXP=e2,e4'; + const cookie = 'e2,e4'; win.AMP_CONFIG['e1'] = 1; win.AMP_CONFIG['e2'] = 1; win.AMP_CONFIG['e3'] = 0; @@ -172,11 +176,11 @@ describe('isExperimentOn', () => { it('should return "off" when disabling value is in the list', () => { win.AMP_CONFIG['e1'] = true; - expectExperiment('AMP_EXP=-e1', 'e1').to.be.false; + expectExperiment('-e1', 'e1').to.be.false; }); it('should return "off" when not in cookie flag or global flag', () => { - expectExperiment('AMP_EXP=', 'e1').to.be.false; + expectExperiment('', 'e1').to.be.false; }); it('should calc if experiment should be "on"', () => { @@ -214,13 +218,11 @@ describe('isExperimentOn', () => { describe('toggleExperiment', () => { let sandbox; let clock; - let expTime; beforeEach(() => { sandbox = sinon.sandbox; clock = sandbox.useFakeTimers(); clock.tick(1); - expTime = new Date(1 + 180 * 24 * 60 * 60 * 1000).toUTCString(); }); afterEach(() => { @@ -228,94 +230,77 @@ describe('toggleExperiment', () => { resetExperimentTogglesForTesting(window); }); - function expectToggle(cookiesString, experimentId, opt_on) { - const doc = { - cookie: cookiesString, - }; + function expectToggle(storedString, experimentId, opt_on) { resetExperimentTogglesForTesting(window); - const on = toggleExperiment( - { - document: doc, - location: { - hostname: 'test.test', - href: 'https://test.test/test.html', - }, + const win = { + document: {}, + localStorage: fakeLocalStorage({'amp-experiment-toggles': storedString}), + location: { + hostname: 'test.test', + href: 'https://test.test/test.html', }, - 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('domain=test.test'); - expect(parts[3]).to.equal('expires=' + expTime); - } - return expect(`${on}; ${decodeURIComponent(parts[0])}`); + }; + const on = toggleExperiment(win, experimentId, opt_on); + const newString = win.localStorage.getItem('amp-experiment-toggles'); + return expect(`${on}; ${newString}`); } it('should toggle to "on" with no cookies, malformed or empty', () => { - expectToggle(null, 'e1').to.equal('true; AMP_EXP=e1'); - expectToggle(undefined, 'e2').to.equal('true; AMP_EXP=e2'); - expectToggle('', 'e3').to.equal('true; AMP_EXP=e3'); - expectToggle('AMP_EXP', 'e4').to.equal('true; AMP_EXP=e4'); - expectToggle('AMP_EXP=', 'e5').to.equal('true; AMP_EXP=e5'); + expectToggle(null, 'e1').to.equal('true; e1'); + expectToggle(undefined, 'e2').to.equal('true; e2'); + expectToggle('', 'e3').to.equal('true; e3'); + expectToggle('', 'e4').to.equal('true; e4'); }); 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'); + expectToggle('e1a,e2', 'e1').to.equal('true; e1a,e2,e1'); }); it('should toggle "off" when value is in the list', () => { - expectToggle('AMP_EXP=e1', 'e1').to.equal('false; AMP_EXP=-e1'); - expectToggle('AMP_EXP=e1,e2', 'e1').to.equal('false; AMP_EXP=-e1,e2'); - expectToggle('AMP_EXP=e2,e1', 'e1').to.equal('false; AMP_EXP=e2,-e1'); + expectToggle('e1', 'e1').to.equal('false; -e1'); + expectToggle('e1,e2', 'e1').to.equal('false; -e1,e2'); + expectToggle('e2,e1', 'e1').to.equal('false; e2,-e1'); }); 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'); + expectToggle('e2', 'e1', true).to.equal('true; e2,e1'); + expectToggle('e1', 'e1', true).to.equal('true; e1'); }); it('should set "off" when requested', () => { - expectToggle('AMP_EXP=e2,e1', 'e1', false).to.equal( - 'false; AMP_EXP=e2,-e1' - ); - expectToggle('AMP_EXP=e1', 'e1', false).to.equal('false; AMP_EXP=-e1'); + expectToggle('e2,e1', 'e1', false).to.equal('false; e2,-e1'); + expectToggle('e1', 'e1', false).to.equal('false; -e1'); }); - it('should not set cookies when toggling and transientExperiment', () => { + it('should not set localStorage when transientExperiment==true', () => { const win = { - document: { - cookie: '', - }, + localStorage: fakeLocalStorage(), }; toggleExperiment(win, 'e1', true, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); toggleExperiment(win, 'e2', false, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); toggleExperiment(win, 'e3', undefined, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); // But all of those experiment states should be durable in the window // environment. expect(isExperimentOn(win, 'e1'), 'e1 is on').to.be.true; expect(isExperimentOn(win, 'e2'), 'e2 is off').to.be.false; expect(isExperimentOn(win, 'e3'), 'e3 is on').to.be.true; toggleExperiment(win, 'e1', false, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); toggleExperiment(win, 'e2', true, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); toggleExperiment(win, 'e3', undefined, true); - expect(win.document.cookie).to.equal(''); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.equal(null); expect(isExperimentOn(win, 'e1'), 'e1 is off').to.be.false; expect(isExperimentOn(win, 'e2'), 'e2 is on').to.be.true; expect(isExperimentOn(win, 'e3'), 'e3 is off').to.be.false; }); - it('should set cookies when toggling and !transientExperiment', () => { + it('should set localStorage when !transientExperiment', () => { const win = { - document: { - cookie: '', - }, + localStorage: fakeLocalStorage(), location: { hostname: 'test.test', href: 'https://test.test/test.html', @@ -327,25 +312,13 @@ describe('toggleExperiment', () => { toggleExperiment(win, 'e3', true, undefined); toggleExperiment(win, 'e4', undefined, false); - expect(getExperimentToglesFromCookieForTesting(win)).to.not.have.property( + expect(getExperimentTogglesForTesting(win)).to.not.have.property( 'transient' ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e1', - true - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e2', - true - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e3', - true - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e4', - true - ); + expect(getExperimentTogglesForTesting(win)).to.have.property('e1', true); + expect(getExperimentTogglesForTesting(win)).to.have.property('e2', true); + expect(getExperimentTogglesForTesting(win)).to.have.property('e3', true); + expect(getExperimentTogglesForTesting(win)).to.have.property('e4', true); // All of those experiment states should be durable in the window // environment. @@ -361,25 +334,13 @@ describe('toggleExperiment', () => { toggleExperiment(win, 'e3', false, undefined); toggleExperiment(win, 'e4', undefined, false); - expect(getExperimentToglesFromCookieForTesting(win)).to.not.have.property( + expect(getExperimentTogglesForTesting(win)).to.not.have.property( 'transient' ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e1', - false - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e2', - false - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e3', - false - ); - expect(getExperimentToglesFromCookieForTesting(win)).to.have.property( - 'e4', - false - ); + expect(getExperimentTogglesForTesting(win)).to.have.property('e1', false); + expect(getExperimentTogglesForTesting(win)).to.have.property('e2', false); + expect(getExperimentTogglesForTesting(win)).to.have.property('e3', false); + expect(getExperimentTogglesForTesting(win)).to.have.property('e4', false); expect(isExperimentOn(win, 'transient'), 'transient is on').to.be.false; expect(isExperimentOn(win, 'e1'), 'e1 is on').to.be.false; @@ -388,11 +349,9 @@ describe('toggleExperiment', () => { expect(isExperimentOn(win, 'e4'), 'e4 is on').to.be.false; }); - it('should not mess up cookies when toggling w/o setting cookie ', () => { + it('should not mess up localStorage when transientExperiment==true ', () => { const win = { - document: { - cookie: '', - }, + localStorage: fakeLocalStorage(), location: { hostname: 'test.test', href: 'https://test.test/test.html', @@ -403,10 +362,10 @@ describe('toggleExperiment', () => { toggleExperiment(win, 'e1', true); toggleExperiment(win, 'e2', true); toggleExperiment(win, 'e3', true); - expect(win.document.cookie).to.contain('e0'); - expect(win.document.cookie).to.contain('e1'); - expect(win.document.cookie).to.contain('e2'); - expect(win.document.cookie).to.contain('e3'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e0'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e1'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e2'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e3'); expect(isExperimentOn(win, 'e0'), 'e0').to.be.true; expect(isExperimentOn(win, 'e1'), 'e1').to.be.true; expect(isExperimentOn(win, 'e2'), 'e2').to.be.true; @@ -414,13 +373,19 @@ describe('toggleExperiment', () => { toggleExperiment(win, 'x0', false, true); toggleExperiment(win, 'x1', true, true); toggleExperiment(win, 'x2', undefined, true); - expect(win.document.cookie).to.contain('e0'); - expect(win.document.cookie).to.contain('e1'); - expect(win.document.cookie).to.contain('e2'); - expect(win.document.cookie).to.contain('e3'); - expect(win.document.cookie).to.not.contain('x0'); - expect(win.document.cookie).to.not.contain('x1'); - expect(win.document.cookie).to.not.contain('x2'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e0'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e1'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e2'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e3'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.not.contain( + 'x0' + ); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.not.contain( + 'x1' + ); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.not.contain( + 'x2' + ); expect(isExperimentOn(win, 'x0'), 'x0').to.be.false; expect(isExperimentOn(win, 'x1'), 'x1').to.be.true; expect(isExperimentOn(win, 'x2'), 'x2').to.be.true; @@ -429,13 +394,17 @@ describe('toggleExperiment', () => { toggleExperiment(win, 'e4', false); toggleExperiment(win, 'e5', true); toggleExperiment(win, 'e6', false); - expect(win.document.cookie).to.contain('e0'); - expect(win.document.cookie).to.contain('e1'); - expect(win.document.cookie).to.contain('e2'); - expect(win.document.cookie).to.contain('e3'); - expect(win.document.cookie).to.not.contain('e4'); - expect(win.document.cookie).to.contain('e5'); - expect(win.document.cookie).to.not.contain('e6'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e0'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e1'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e2'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e3'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.not.contain( + 'e4' + ); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.contain('e5'); + expect(win.localStorage.getItem('amp-experiment-toggles')).to.not.contain( + 'e6' + ); expect(isExperimentOn(win, 'e4'), 'e4').to.be.false; expect(isExperimentOn(win, 'e5'), 'e5').to.be.true; expect(isExperimentOn(win, 'e6'), 'e6').to.be.false; @@ -443,12 +412,10 @@ describe('toggleExperiment', () => { it('should override global settings', () => { const win = { - document: { - cookie: '', - }, 'AMP_CONFIG': { 'e1': 1, }, + localStorage: fakeLocalStorage(), location: { hostname: 'test.test', href: 'http://foo.bar', @@ -624,11 +591,11 @@ describe('experiment branch tests', () => { location: { hostname: 'test.server.name.com', }, + localStorage: fakeLocalStorage(), AMP_CONFIG: { testExperimentId: experimentFrequency, }, document: { - cookie: null, querySelector: () => {}, }, }; diff --git a/tools/experiments/README.md b/tools/experiments/README.md index 94ed15522668a..7030d6bce24ac 100644 --- a/tools/experiments/README.md +++ b/tools/experiments/README.md @@ -4,17 +4,20 @@ AMP experiments are features that are released but not yet ready for wide use, s Developers and users can opt-in into these features before they are fully released. However, experimental components should be used with caution, as they may contain bugs or have unexpected side effects. -## Enable an experiment for yourself +## Enable an experimental component -Experimental components can be served from `https://cdn.ampproject.org` or any other domain. +#### Served from cdn.ampproject.org -### Served from cdn.ampproject.org +For content served from `https://*.cdn.ampproject.org`, +go to `/experiments.html` on a Google AMP Cache subdomain and enable (or disable) any experimental component by toggling them on (or off). -For content served from `https://cdn.ampproject.org`, go to the [AMP experiments page](https://cdn.ampproject.org/experiments.html) and enable (or disable) any experimental component by toggling them on (or off). Opting in will set a cookie on your browser that will enable the experiment on all AMP pages served through the Google AMP Cache. +For example, to enable experiments on cached AMP pages whose source origin is `www.example.com`, go to `www-example-com.cdn.ampproject.org/experiments.html`. -### Served from other domains +Experiment opt-ins are saved to `localStorage` and only enables the experiment on AMP pages served from the current domain. -For content served from any other domain, you can toggle experiments in the Chrome DevTools Console when development mode is enabled by using: +#### Served from other domains + +For content served from non-CDN domains, experiments can be toggled in the devtools console using: ```javascript AMP.toggleExperiment('experiment') diff --git a/tools/experiments/experiments.html b/tools/experiments/experiments.html index e4984defc12d7..3d2c6edbbb378 100644 --- a/tools/experiments/experiments.html +++ b/tools/experiments/experiments.html @@ -13,6 +13,10 @@ font-family: Roboto, verdana, arial, sans-serif; } + [hidden] { + display: none !important; + } + .header-title { color: #1b1464; font-family: Roboto, verdana, arial, sans-serif; @@ -32,6 +36,10 @@ color: #fff; } + h2 { + color: #147bc1; + } + .content { margin: 16px; display: block; @@ -68,6 +76,11 @@ min-width: 60px; } + #subdomain { + color: gray; + font-family: 'Courier New', Courier, monospace; + } + tr[data-on='0'] button { background: #aaa; color: #777; @@ -94,6 +107,23 @@ display: block; } + #redirect > input { + border: solid 1px gray; + width: calc(100% - 20px); + padding: 10px; + } + + #redirect > button { + border: solid 1px gray; + padding: 5px 15px; + margin: 5px; + } + + #redirect > h4 { + border: inset; + padding: 10px; + } + .popup-graypane { position: fixed; top: 0; @@ -160,22 +190,60 @@

AMP Experiments

+

Release channels

- The switches below will toggle experiments for all documents served through the - Google AMP Cache; - that is, the cached copies of valid AMP content published to the web at - the cdn.ampproject.org domain. See the AMP project - documentation - to find out more about experiments, and how to activate them outside of - the Google AMP Cache (including in local development). + These switches control which release channel (prod, RC, or dev) of AMP JS + will be served through the Google AMP Cache. +

+
+ +
+
+
+ + +
+

Experiments

+ +

+ Experiments can no longer be enabled on the top-level cdn.ampproject.org/experiments.html. + They must be toggled on the specific subdomain e.g. www-example-com.cdn.ampproject.org/experiments.html. +

+ +

+ Find the AMP Cache experiments page for your source domain or AMP page URL: +

+ + + + +

+
+ + +
+

Experiments

+ +

+ These switches toggle experiments for documents served at the current subdomain + of the Google AMP Cache:

+

cdn.ampproject.org

+

- Please exercise caution. These experiments are not fully tested and might + Please exercise caution. These experiments are not fully tested and might contain bugs or have unexpected side effects. Please only activate them if you work on the AMP Project or plan to create AMP documents that use unreleased features for testing purposes.

+ +

+ See the AMP project + documentation + to find out more about experiments, and how to activate them outside of + the Google AMP Cache (including in local development). +

diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 901f4734fecd8..a771b51bd26fa 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -23,6 +23,7 @@ import {getMode} from '../../src/mode'; import {isExperimentOn, toggleExperiment} from '../../src/experiments'; import {listenOnce} from '../../src/event-helper'; import {onDocumentReady} from '../../src/document-ready'; +import {parseUrlDeprecated} from '../../src/url'; //TODO(@cramforce): For type. Replace with forward declaration. import {reportError} from '../../src/error'; @@ -59,7 +60,7 @@ const AMP_CANARY_COOKIE = { }; /** @const {!Array} */ -const EXPERIMENTS = [ +const CHANNELS = [ // Canary (Dev Channel) { id: CANARY_EXPERIMENT_ID, @@ -76,6 +77,10 @@ const EXPERIMENTS = [ 'https://github.com/ampproject/amphtml/blob/master/' + 'contributing/release-schedule.md#amp-release-candidate-rc-channel', }, +]; + +/** @const {!Array} */ +const EXPERIMENTS = [ { id: 'alp', name: 'Activates support for measuring incoming clicks.', @@ -411,14 +416,54 @@ if (getMode().localDev) { * Builds the expriments tbale. */ function build() { - const table = document.getElementById('experiments-table'); + const {host} = window.location; + + const subdomain = document.getElementById('subdomain'); + subdomain.textContent = host; + + // #redirect contains UI that generates a subdomain experiments page link + // given a "google.com/amp/..." viewer URL. + const redirect = document.getElementById('redirect'); + const input = redirect.querySelector('input'); + const button = redirect.querySelector('button'); + const anchor = redirect.querySelector('a'); + button.addEventListener('click', function() { + let urlString = input.value.trim(); + // Avoid protocol-less urlString from being parsed as a relative URL. + const hasProtocol = /^https?:\/\//.test(urlString); + if (!hasProtocol) { + urlString = 'https://' + urlString; + } + const url = parseUrlDeprecated(urlString); + if (url) { + const subdomain = url.hostname.replace(/\./g, '-'); + const href = `https://${subdomain}.cdn.ampproject.org/experiments.html`; + anchor.href = href; + anchor.textContent = href; + } + }); + + const channelsTable = document.getElementById('channels-table'); + CHANNELS.forEach(function(experiment) { + channelsTable.appendChild(buildExperimentRow(experiment)); + }); + + const experimentsTable = document.getElementById('experiments-table'); EXPERIMENTS.forEach(function(experiment) { - table.appendChild(buildExperimentRow(experiment)); + experimentsTable.appendChild(buildExperimentRow(experiment)); }); + + if (host === 'cdn.ampproject.org') { + const experimentsDesc = document.getElementById('experiments-desc'); + experimentsDesc.setAttribute('hidden', ''); + experimentsTable.setAttribute('hidden', ''); + } else { + redirect.setAttribute('hidden', ''); + } } /** - * Builds one row of the experiments table. + * Builds one row in the channel or experiments table. * @param {!ExperimentDef} experiment */ function buildExperimentRow(experiment) { @@ -486,7 +531,7 @@ function buildLinkMaybe(text, link) { * Updates states of all experiments in the table. */ function update() { - EXPERIMENTS.forEach(function(experiment) { + CHANNELS.concat(EXPERIMENTS).forEach(function(experiment) { updateExperimentRow(experiment); }); }