Skip to content

Commit

Permalink
Prefix remaining window globals in src/ (ampproject#24147)
Browse files Browse the repository at this point in the history
* Prefix remaining globals in src.

* Fix lint.
  • Loading branch information
William Chou authored Aug 23, 2019
1 parent 38f3b0a commit 6e8add4
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 19 deletions.
1 change: 0 additions & 1 deletion extensions/amp-access/0.1/test/test-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ describe('JwtHelper', () => {

windowApi = {
crypto: {subtle},
services: {},
};
helper = new JwtHelper(windowApi);
});
Expand Down
9 changes: 5 additions & 4 deletions src/3p-frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export function setDefaultBootstrapBaseUrlForTesting(url) {
* @param {*} win
*/
export function resetBootstrapBaseUrlForTesting(win) {
win.defaultBootstrapSubDomain = undefined;
win.__AMP_DEFAULT_BOOTSTRAP_SUBDOMAIN = undefined; // eslint-disable-line google-camelcase/google-camelcase
}

/**
Expand All @@ -251,11 +251,12 @@ export function getDefaultBootstrapBaseUrl(parentWindow, opt_srcFileBasename) {
return getDevelopmentBootstrapBaseUrl(parentWindow, srcFileBasename);
}
// Ensure same sub-domain is used despite potentially different file.
parentWindow.defaultBootstrapSubDomain =
parentWindow.defaultBootstrapSubDomain || getSubDomain(parentWindow);
parentWindow.__AMP_DEFAULT_BOOTSTRAP_SUBDOMAIN = // eslint-disable-line google-camelcase/google-camelcase
parentWindow.__AMP_DEFAULT_BOOTSTRAP_SUBDOMAIN ||
getSubDomain(parentWindow);
return (
'https://' +
parentWindow.defaultBootstrapSubDomain +
parentWindow.__AMP_DEFAULT_BOOTSTRAP_SUBDOMAIN +
`.${urls.thirdPartyFrameHost}/${internalRuntimeVersion()}/` +
`${srcFileBasename}.html`
);
Expand Down
26 changes: 15 additions & 11 deletions src/experiments.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ function selectRandomItem(arr) {
*
* Check whether a given experiment is set using isExperimentOn(win,
* experimentName) and, if it is on, look for which branch is selected in
* win.experimentBranches[experimentName].
* win.__AMP_EXPERIMENT_BRANCHES[experimentName].
*
* @param {!Window} win Window context on which to save experiment
* selection state.
Expand All @@ -315,39 +315,41 @@ function selectRandomItem(arr) {
* branches.
*/
export function randomlySelectUnsetExperiments(win, experiments) {
win.experimentBranches = win.experimentBranches || {};
win.__AMP_EXPERIMENT_BRANCHES = win.__AMP_EXPERIMENT_BRANCHES || {};
const selectedExperiments = {};
for (const experimentName in experiments) {
// Skip experimentName if it is not a key of experiments object or if it
// has already been populated by some other property.
if (!hasOwn(experiments, experimentName)) {
continue;
}
if (hasOwn(win.experimentBranches, experimentName)) {
if (hasOwn(win.__AMP_EXPERIMENT_BRANCHES, experimentName)) {
selectedExperiments[experimentName] =
win.experimentBranches[experimentName];
win.__AMP_EXPERIMENT_BRANCHES[experimentName];
continue;
}

if (
!experiments[experimentName].isTrafficEligible ||
!experiments[experimentName].isTrafficEligible(win)
) {
win.experimentBranches[experimentName] = null;
win.__AMP_EXPERIMENT_BRANCHES[experimentName] = null;
continue;
}

// If we're in the experiment, but we haven't already forced a specific
// experiment branch (e.g., via a test setup), then randomize the branch
// choice.
if (
!win.experimentBranches[experimentName] &&
!win.__AMP_EXPERIMENT_BRANCHES[experimentName] &&
isExperimentOn(win, /*OK*/ experimentName)
) {
const {branches} = experiments[experimentName];
win.experimentBranches[experimentName] = selectRandomItem(branches);
win.__AMP_EXPERIMENT_BRANCHES[experimentName] = selectRandomItem(
branches
);
selectedExperiments[experimentName] =
win.experimentBranches[experimentName];
win.__AMP_EXPERIMENT_BRANCHES[experimentName];
}
}
return selectedExperiments;
Expand All @@ -363,7 +365,9 @@ export function randomlySelectUnsetExperiments(win, experiments) {
* null if experimentName has been tested but no branch was enabled).
*/
export function getExperimentBranch(win, experimentName) {
return win.experimentBranches ? win.experimentBranches[experimentName] : null;
return win.__AMP_EXPERIMENT_BRANCHES
? win.__AMP_EXPERIMENT_BRANCHES[experimentName]
: null;
}

/**
Expand All @@ -377,7 +381,7 @@ export function getExperimentBranch(win, experimentName) {
* @visibleForTesting
*/
export function forceExperimentBranch(win, experimentName, branchId) {
win.experimentBranches = win.experimentBranches || {};
win.__AMP_EXPERIMENT_BRANCHES = win.__AMP_EXPERIMENT_BRANCHES || {};
toggleExperiment(win, experimentName, !!branchId, true);
win.experimentBranches[experimentName] = branchId;
win.__AMP_EXPERIMENT_BRANCHES[experimentName] = branchId;
}
4 changes: 2 additions & 2 deletions src/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -711,13 +711,13 @@ export function rethrowAsync(var_args) {
* on Log and closure literally can't even.
* @type {{user: ?Log, dev: ?Log, userForEmbed: ?Log}}
*/
self.log = self.log || {
self.__AMP_LOG = self.__AMP_LOG || {
user: null,
dev: null,
userForEmbed: null,
};

const logs = self.log;
const logs = self.__AMP_LOG;

/**
* Eventually holds a constructor for Log objects. Lazily initialized, so we
Expand Down
2 changes: 1 addition & 1 deletion test/unit/test-experiments.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ describe('experiment branch tests', () => {
isExperimentOn(sandbox.win, 'testExperimentId'),
'experiment is on'
).to.be.false;
expect(sandbox.win.experimentBranches).to.be.empty;
expect(sandbox.win.__AMP_EXPERIMENT_BRANCHES).to.be.empty;
});

it('handles experiment not diverted path', () => {
Expand Down

0 comments on commit 6e8add4

Please sign in to comment.