Merges isDevChannel() and isDevChannelVersionDoNotUse_() to isCanary() (

ampproject#6803) * Merges isDevChannel() and isDevChannelVersionDoNotUse_(). * Update js doc * Remove unnecessary dep-checks. * fix
abasse · Jan 7, 2017 · c7631f9 · c7631f9
1 parent 2f6d82d
commit c7631f9
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 59 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -338,19 +338,6 @@ var forbiddenTerms = {
       'tools/experiments/experiments.js',
     ],
   },
-  'isDevChannel\\W': {
-    message: requiresReviewPrivacy,
-    whitelist: [
-      'src/experiments.js',
-      'tools/experiments/experiments.js',
-    ],
-  },
-  'isDevChannelVersionDoNotUse_\\W': {
-    message: shouldNeverBeUsed,
-    whitelist: [
-      'src/experiments.js',
-    ],
-  },
   'isTrustedViewer': {
     message: requiresReviewPrivacy,
     whitelist: [
@@ -503,8 +490,6 @@ var forbiddenTerms = {
       'src/service-worker/error-reporting.js',
       'src/mode.js',
       'src/experiments.js',
-      'src/error.js',
-      'src/3p-frame.js',
       'src/config.js',
       'dist.3p/current/integration.js',
     ],

diff --git a/src/3p-frame.js b/src/3p-frame.js
@@ -16,7 +16,7 @@
 
 import {dev, user} from './log';
 import {documentInfoForDoc} from './document-info';
-import {isExperimentOn, experimentToggles} from './experiments';
+import {isExperimentOn, experimentToggles, isCanary} from './experiments';
 import {getLengthNumeral} from '../src/layout';
 import {tryParseJson} from './json';
 import {getMode} from './mode';
@@ -83,7 +83,7 @@ function getFrameAttributes(parentWindow, element, opt_type, opt_context) {
     },
     tagName: element.tagName,
     mode: getModeObject(),
-    canary: !!(parentWindow.AMP_CONFIG && parentWindow.AMP_CONFIG.canary),
+    canary: isCanary(parentWindow),
     hidden: !viewer.isVisible(),
     amp3pSentinel: generateSentinel(parentWindow),
     initialIntersection: element.getIntersectionChangeEntry(),

diff --git a/src/error.js b/src/error.js
@@ -22,6 +22,7 @@ import {USER_ERROR_SENTINEL, isUserErrorMessage} from './log';
 import {makeBodyVisible} from './style-installer';
 import {urls} from './config';
 import {isProxyOrigin} from './url';
+import {isCanary} from './experiments';
 
 
 /**
@@ -264,7 +265,7 @@ export function getErrorReportUrl(message, filename, line, col, error,
   if (self.context && self.context.location) {
     url += '&3p=1';
   }
-  if (self.AMP_CONFIG && self.AMP_CONFIG.canary) {
+  if (isCanary(self)) {
     url += '&ca=1';
   }
   if (self.location.ancestorOrigins && self.location.ancestorOrigins[0]) {

diff --git a/src/experiments.js b/src/experiments.js
@@ -34,35 +34,15 @@ 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';
-
 /** @type {Object<string, boolean>|undefined} */
 let toggles_;
 
 /**
- * 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_(win)) {
-    return true;
-  }
-  return false;
-}
-
-
-/**
- * Whether the version corresponds to the dev-channel binary.
+ * Whether we are in canary.
  * @param {!Window} win
  * @return {boolean}
- * @private Visible for testing only!
  */
-export function isDevChannelVersionDoNotUse_(win) {
+export function isCanary(win) {
   return !!(win.AMP_CONFIG && win.AMP_CONFIG.canary);
 }
 

diff --git a/test/functional/test-experiments.js b/test/functional/test-experiments.js
@@ -15,8 +15,7 @@
  */
 
 import {
-  isDevChannel,
-  isDevChannelVersionDoNotUse_,
+  isCanary,
   isExperimentOn,
   isExperimentOnAllowUrlOverride,
   experimentToggles,
@@ -418,31 +417,17 @@ describe('toggleExperiment', () => {
   });
 });
 
-describe('isDevChannel', () => {
-
-  function expectDevChannel(cookiesString) {
-    resetExperimentTogglesForTesting();
-    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;
-  });
+describe('isCanary', () => {
 
   it('should return value based on binary version', () => {
     const win = {
       AMP_CONFIG: {
         canary: 0,
       },
     };
-    expect(isDevChannelVersionDoNotUse_(win)).to.be.false;
+    expect(isCanary(win)).to.be.false;
     win.AMP_CONFIG.canary = 1;
-    expect(isDevChannelVersionDoNotUse_(win)).to.be.true;
+    expect(isCanary(win)).to.be.true;
   });
 });