From cf97ffeea0f02a3011832057fca1edb1f7ef85fb Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Fri, 24 Jan 2020 16:53:15 -0500 Subject: [PATCH 1/9] Rename enum values --- tools/experiments/experiments.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index f398b635c73a..f43b231b3a84 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -60,10 +60,10 @@ const RTV_CHANNEL_ID = 'rtv-channel'; /** * The different states of the AMP_CANARY cookie. */ -const AMP_CANARY_COOKIE = { - DISABLED: '0', - CANARY: '1', - RC: '2', +const AMP_OPT_IN_COOKIE = { + DISABLED: '', + EXPERIMENTAL: 'experimental', + BETA: 'beta', }; /** @const {!Array} */ @@ -154,7 +154,7 @@ function build() { if (!rtvInput.value) { showConfirmation_( 'Do you really want to opt out of RTV?', - setAmpCanaryCookie_.bind(null, AMP_CANARY_COOKIE.DISABLED) + setAmpCanaryCookie_.bind(null, AMP_OPT_IN_COOKIE.DISABLED) ); } else if (RTV_PATTERN.test(rtvInput.value)) { showConfirmation_( @@ -270,9 +270,9 @@ function updateExperimentRow(experiment) { function isExperimentOn_(id) { switch (id) { case EXPERIMENTAL_CHANNEL_ID: - return getCookie(window, 'AMP_CANARY') == AMP_CANARY_COOKIE.CANARY; + return getCookie(window, 'AMP_CANARY') == AMP_OPT_IN_COOKIE.EXPERIMENTAL; case BETA_CHANNEL_ID: - return getCookie(window, 'AMP_CANARY') == AMP_CANARY_COOKIE.RC; + return getCookie(window, 'AMP_CANARY') == AMP_OPT_IN_COOKIE.BETA; case RTV_CHANNEL_ID: return RTV_PATTERN.test(getCookie(window, 'AMP_CANARY')); default: @@ -283,14 +283,14 @@ function isExperimentOn_(id) { /** * Opts in to / out of the "canary" or "rc" runtime types by setting the * AMP_CANARY cookie. - * @param {string} cookieState One of AMP_CANARY_COOKIE.{DISABLED|CANARY|RC} or - * a 15-digit RTV. + * @param {string} cookieState One of the AMP_OPT_IN_COOKIE enum values, or a + * 15-digit RTV. */ function setAmpCanaryCookie_(cookieState) { let validUntil = 0; if (RTV_PATTERN.test(cookieState)) { validUntil = Date.now() + RTV_COOKIE_MAX_AGE_MS; - } else if (cookieState != AMP_CANARY_COOKIE.DISABLED) { + } else if (cookieState != AMP_OPT_IN_COOKIE.DISABLED) { validUntil = Date.now() + COOKIE_MAX_AGE_MS; } const cookieOptions = { @@ -325,11 +325,11 @@ function toggleExperiment_(id, name, opt_on) { showConfirmation_(`${confirmMessage}: "${name}"`, () => { if (id == EXPERIMENTAL_CHANNEL_ID) { setAmpCanaryCookie_( - on ? AMP_CANARY_COOKIE.CANARY : AMP_CANARY_COOKIE.DISABLED + on ? AMP_OPT_IN_COOKIE.EXPERIMENTAL : AMP_OPT_IN_COOKIE.DISABLED ); } else if (id == BETA_CHANNEL_ID) { setAmpCanaryCookie_( - on ? AMP_CANARY_COOKIE.RC : AMP_CANARY_COOKIE.DISABLED + on ? AMP_OPT_IN_COOKIE.BETA : AMP_OPT_IN_COOKIE.DISABLED ); } else { toggleExperiment(window, id, on); @@ -366,10 +366,10 @@ function showConfirmation_(message, callback) { } /** - * Loads the AMP_CONFIG objects from whatever the v0.js is that the - * user has (depends on whether they opted into canary or RC), so that - * experiment state can reflect the default activated experiments. - * @return {*} TODO(#23582): Specify return type + * Loads the AMP_CONFIG objects from whatever the v0.js is that the user has + * (depends on whether they opted into beta/experimental), so that experiment + * state can reflect the default activated experiments. + * @return {Promise} the active AMP_CONFIG, parsed as a JSON object */ function getAmpConfig() { const deferred = new Deferred(); From 9f1cdfee33d45023a553bc5da8c2373fcef669ce Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Fri, 24 Jan 2020 16:54:48 -0500 Subject: [PATCH 2/9] Back-support legacy values --- tools/experiments/experiments.js | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index f43b231b3a84..4626fede01f9 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -66,6 +66,16 @@ const AMP_OPT_IN_COOKIE = { BETA: 'beta', }; +/** + * Legacy values for __Host-AMP_OPT_IN cookie. + * TODO(#25205): remove this once the CDN stops supporting these values. + */ +const _LEGACY_AMP_OPT_IN_COOKIE = { + DISABLED: '0', + EXPERIMENTAL: '1', + BETA: '2', +}; + /** @const {!Array} */ const CHANNELS = [ // Experimental Channel @@ -270,19 +280,24 @@ function updateExperimentRow(experiment) { function isExperimentOn_(id) { switch (id) { case EXPERIMENTAL_CHANNEL_ID: - return getCookie(window, 'AMP_CANARY') == AMP_OPT_IN_COOKIE.EXPERIMENTAL; + return [ + AMP_OPT_IN_COOKIE.EXPERIMENTAL, + _LEGACY_AMP_OPT_IN_COOKIE.EXPERIMENTAL, + ].includes(optInCookieValue); case BETA_CHANNEL_ID: - return getCookie(window, 'AMP_CANARY') == AMP_OPT_IN_COOKIE.BETA; + return [AMP_OPT_IN_COOKIE.BETA, _LEGACY_AMP_OPT_IN_COOKIE.BETA].includes( + optInCookieValue + ); case RTV_CHANNEL_ID: - return RTV_PATTERN.test(getCookie(window, 'AMP_CANARY')); + return RTV_PATTERN.test(optInCookieValue); default: return isExperimentOn(window, /*OK*/ id); } } /** - * Opts in to / out of the "canary" or "rc" runtime types by setting the - * AMP_CANARY cookie. + * Opts in to / out of the "beta" or "experimental" runtime types by setting the + * __Host-AMP_OPT_IN cookie. * @param {string} cookieState One of the AMP_OPT_IN_COOKIE enum values, or a * 15-digit RTV. */ From b70fb35f4cbb89283c3bf830b5c62194a64eb599 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Fri, 24 Jan 2020 16:55:29 -0500 Subject: [PATCH 3/9] Double-support old and new cookie name --- tools/experiments/experiments.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 4626fede01f9..efab2ed24c49 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -58,7 +58,7 @@ const BETA_CHANNEL_ID = 'beta-channel'; const RTV_CHANNEL_ID = 'rtv-channel'; /** - * The different states of the AMP_CANARY cookie. + * The different states of the __Host-AMP_OPT_IN cookie. */ const AMP_OPT_IN_COOKIE = { DISABLED: '', @@ -175,7 +175,10 @@ function build() { }); if (isExperimentOn_(RTV_CHANNEL_ID)) { - rtvInput.value = getCookie(window, 'AMP_CANARY'); + // TODO(#25205): remove this once the CDN stops supporting the AMP_CANARY + // cookie. + rtvInput.value = + getCookie(window, '__Host-AMP_OPT_IN') || getCookie(window, 'AMP_CANARY'); rtvInput.dispatchEvent(new Event('input')); document.getElementById('rtv-details').open = true; } @@ -278,6 +281,10 @@ function updateExperimentRow(experiment) { * @return {boolean} */ function isExperimentOn_(id) { + // TODO(#25205): remove this once the CDN stops supporting the AMP_CANARY + // cookie. + const optInCookieValue = + getCookie(window, '__Host-AMP_OPT_IN') || getCookie(window, 'AMP_CANARY'); switch (id) { case EXPERIMENTAL_CHANNEL_ID: return [ @@ -318,7 +325,11 @@ function setAmpCanaryCookie_(cookieState) { sameSite: SameSite.NONE, secure: true, }; - setCookie(window, 'AMP_CANARY', cookieState, validUntil, cookieOptions); + // TODO(#25205): remove this once the CDN stops supporting the AMP_CANARY + // cookie. + ['__Host-AMP_OPT_IN', 'AMP_CANARY'].forEach(cookieName => { + setCookie(window, cookieName, cookieState, validUntil, cookieOptions); + }); // Reflect default experiment state. self.location.reload(); } @@ -396,7 +407,7 @@ function getAmpConfig() { xhr.addEventListener('error', () => { reject(new Error(xhr.statusText)); }); - // Cache bust, so we immediately reflect AMP_CANARY cookie changes. + // Cache bust, so we immediately reflect cookie changes. xhr.open('GET', '/v0.js?' + Math.random(), true); xhr.send(null); return promise From 38bb167f2ba307a354bac355571022ed663a33c8 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Fri, 24 Jan 2020 17:16:02 -0500 Subject: [PATCH 4/9] Do not set Domain value in __Host- prefixed cookies --- tools/experiments/experiments.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index efab2ed24c49..6f64559406e1 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -316,8 +316,6 @@ function setAmpCanaryCookie_(cookieState) { validUntil = Date.now() + COOKIE_MAX_AGE_MS; } const cookieOptions = { - // Set explicit domain, so the cookie gets sent to sub domains. - domain: location.hostname, allowOnProxyOrigin: true, // Make sure the cookie is available for the script loads coming from // other domains. Chrome's default of LAX would otherwise prevent it @@ -325,11 +323,18 @@ function setAmpCanaryCookie_(cookieState) { sameSite: SameSite.NONE, secure: true, }; + setCookie( + window, + '__Host-AMP_OPT_IN', + cookieState, + validUntil, + cookieOptions + ); // TODO(#25205): remove this once the CDN stops supporting the AMP_CANARY // cookie. - ['__Host-AMP_OPT_IN', 'AMP_CANARY'].forEach(cookieName => { - setCookie(window, cookieName, cookieState, validUntil, cookieOptions); - }); + // Set explicit domain, so the cookie gets sent to sub domains. + cookieOptions.domain = location.hostname; + setCookie(window, 'AMP_CANARY', cookieState, validUntil, cookieOptions); // Reflect default experiment state. self.location.reload(); } From 9d200b4b7f60a1324da1b51cec30f2f861b09bc4 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Fri, 24 Jan 2020 17:32:32 -0500 Subject: [PATCH 5/9] Explicit zero --- tools/experiments/experiments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 6f64559406e1..a6544bbe4eae 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -61,7 +61,7 @@ const RTV_CHANNEL_ID = 'rtv-channel'; * The different states of the __Host-AMP_OPT_IN cookie. */ const AMP_OPT_IN_COOKIE = { - DISABLED: '', + DISABLED: '0', EXPERIMENTAL: 'experimental', BETA: 'beta', }; From 13f85fd1a6fc804822aeaf72ce92c6ea3b8cf1cb Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Mon, 27 Jan 2020 17:36:12 -0500 Subject: [PATCH 6/9] :%s/setAmpCanaryCookie_/setAmpOptInCookie_/g --- tools/experiments/experiments.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index a6544bbe4eae..00f465a10f2e 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -164,12 +164,12 @@ function build() { if (!rtvInput.value) { showConfirmation_( 'Do you really want to opt out of RTV?', - setAmpCanaryCookie_.bind(null, AMP_OPT_IN_COOKIE.DISABLED) + setAmpOptInCookie_.bind(null, AMP_OPT_IN_COOKIE.DISABLED) ); } else if (RTV_PATTERN.test(rtvInput.value)) { showConfirmation_( `Do you really want to opt in to RTV ${rtvInput.value}?`, - setAmpCanaryCookie_.bind(null, rtvInput.value) + setAmpOptInCookie_.bind(null, rtvInput.value) ); } }); @@ -308,7 +308,7 @@ function isExperimentOn_(id) { * @param {string} cookieState One of the AMP_OPT_IN_COOKIE enum values, or a * 15-digit RTV. */ -function setAmpCanaryCookie_(cookieState) { +function setAmpOptInCookie_(cookieState) { let validUntil = 0; if (RTV_PATTERN.test(cookieState)) { validUntil = Date.now() + RTV_COOKIE_MAX_AGE_MS; @@ -355,11 +355,11 @@ function toggleExperiment_(id, name, opt_on) { showConfirmation_(`${confirmMessage}: "${name}"`, () => { if (id == EXPERIMENTAL_CHANNEL_ID) { - setAmpCanaryCookie_( + setAmpOptInCookie_( on ? AMP_OPT_IN_COOKIE.EXPERIMENTAL : AMP_OPT_IN_COOKIE.DISABLED ); } else if (id == BETA_CHANNEL_ID) { - setAmpCanaryCookie_( + setAmpOptInCookie_( on ? AMP_OPT_IN_COOKIE.BETA : AMP_OPT_IN_COOKIE.DISABLED ); } else { From 4d34ea0e79be17992e87dde7c422a23acd705236 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Mon, 27 Jan 2020 17:37:33 -0500 Subject: [PATCH 7/9] TODO nightly --- tools/experiments/experiments.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index 00f465a10f2e..f7a16be1e2fd 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -64,6 +64,7 @@ const AMP_OPT_IN_COOKIE = { DISABLED: '0', EXPERIMENTAL: 'experimental', BETA: 'beta', + // NIGHTLY: 'nightly', // TODO(#25616): add when CDN supports nightly builds. }; /** From 88443f8259e4194a42457a944569bd4ed00756be Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Mon, 27 Jan 2020 17:41:18 -0500 Subject: [PATCH 8/9] Update comments --- tools/experiments/experiments.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index f7a16be1e2fd..db0ee87edd32 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -304,8 +304,8 @@ function isExperimentOn_(id) { } /** - * Opts in to / out of the "beta" or "experimental" runtime types by setting the - * __Host-AMP_OPT_IN cookie. + * Opts in to / out of the "beta" or "experimental" channels or a specific RTV + * or by setting the __Host-AMP_OPT_IN cookie. * @param {string} cookieState One of the AMP_OPT_IN_COOKIE enum values, or a * 15-digit RTV. */ @@ -399,8 +399,8 @@ function showConfirmation_(message, callback) { /** * Loads the AMP_CONFIG objects from whatever the v0.js is that the user has - * (depends on whether they opted into beta/experimental), so that experiment - * state can reflect the default activated experiments. + * (depends on whether they opted into beta, experimental, or a specific RTV) so + * that experiment state can reflect the default activated experiments. * @return {Promise} the active AMP_CONFIG, parsed as a JSON object */ function getAmpConfig() { From 5ff49cb739aa1cf7721f7cf88b40d835fd32ffd6 Mon Sep 17 00:00:00 2001 From: Daniel Rozenberg Date: Tue, 28 Jan 2020 00:50:48 +0000 Subject: [PATCH 9/9] Update tools/experiments/experiments.js Co-Authored-By: Ryan Cebulko --- tools/experiments/experiments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index db0ee87edd32..09dc8b0c4583 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -305,7 +305,7 @@ function isExperimentOn_(id) { /** * Opts in to / out of the "beta" or "experimental" channels or a specific RTV - * or by setting the __Host-AMP_OPT_IN cookie. + * by setting the __Host-AMP_OPT_IN cookie. * @param {string} cookieState One of the AMP_OPT_IN_COOKIE enum values, or a * 15-digit RTV. */