Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-2298: Avoid possible race condition in ABTest & CMP on Dawn #687

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Changes from all commits
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -19,7 +19,12 @@ import globals from './Globals';
import { getJson } from '../utils/utils';
import { log } from '../utils/common';

const { BROWSER_INFO, CMP_BASE_URL, EXTENSION_VERSION } = globals;
const {
BROWSER_INFO_READY,
BROWSER_INFO,
CMP_BASE_URL,
EXTENSION_VERSION
} = globals;

/** Helper class for handling A/B tests.
* @memberof BackgroundClasses
@@ -54,33 +59,34 @@ class ABTest {
fetch(irDebugOverride) {
log('A/B Tests: fetching...');

const URL = ABTest._buildURL(irDebugOverride);

return getJson(URL).then((data) => {
if (data && Array.isArray(data)) {
log('A/B Tests: fetched', JSON.stringify(data));
this._updateTests(data);
log('A/B Tests: tests updated to', this.getTests());
} else {
log('A/B Tests: no tests found.');
}
}).catch(() => {
log('A/B Tests: error fetching.');
});
return ABTest._buildURL(irDebugOverride)
.then(url => getJson(url))
.then((data) => {
if (data && Array.isArray(data)) {
log('A/B Tests: fetched', JSON.stringify(data));
this._updateTests(data);
log('A/B Tests: tests updated to', this.getTests());
} else {
log('A/B Tests: no tests found.');
}
}).catch(() => {
log('A/B Tests: error fetching.');
});
}

silentFetch(ir) {
const URL = ABTest._buildURL(ir);

return getJson(URL).then((data) => {
if (data && Array.isArray(data)) {
this._updateTests(data);
}
return 'resolved';
}).catch(() => 'rejected');
return ABTest._buildURL(ir)
.then(url => getJson(url))
.then((data) => {
if (data && Array.isArray(data)) {
this._updateTests(data);
}
return 'resolved';
}).catch(() => 'rejected');
}

static _buildURL(ir) {
static async _buildURL(ir) {
await BROWSER_INFO_READY;
return (`${CMP_BASE_URL}/abtestcheck
?os=${encodeURIComponent(BROWSER_INFO.os)}
&install_date=${encodeURIComponent(conf.install_date)}
@@ -16,7 +16,12 @@ import globals from './Globals';
import { getJson } from '../utils/utils';
import { log } from '../utils/common';

const { BROWSER_INFO, CMP_BASE_URL, EXTENSION_VERSION } = globals;
const {
BROWSER_INFO_READY,
BROWSER_INFO,
CMP_BASE_URL,
EXTENSION_VERSION
} = globals;

/**
* Class for handling notification and/or marketing campaigns.
@@ -36,27 +41,26 @@ class CMP {
return Promise.resolve(false);
}

const URL = CMP._buildUrl();

return getJson(URL).then((data) => {
if (CMP._isNewData(data)) {
this._updateCampaigns(data);
return this.CMP_DATA;
}
// getJson() returned a 204, meaning no new campaigns available
log('No CMP data to fetch at this time');
globals.SESSION.cmp_data = [];
return false;
}).catch((err) => {
log('Error in fetchCMPData', err);
return false;
});
return CMP._buildUrl()
.then(url => getJson(url))
.then((data) => {
if (CMP._isNewData(data)) {
this._updateCampaigns(data);
return this.CMP_DATA;
}
// getJson() returned a 204, meaning no new campaigns available
log('No CMP data to fetch at this time');
globals.SESSION.cmp_data = [];
return false;
}).catch((err) => {
log('Error in fetchCMPData', err);
return false;
});
}

debugFetch() {
const URL = CMP._buildUrl();

return getJson(URL)
return CMP._buildUrl()
.then(url => getJson(url))
.then((data) => {
if (CMP._isNewData(data)) {
this._updateCampaigns(data);
@@ -86,7 +90,8 @@ class CMP {
this.CMP_DATA = data.Campaigns;
}

static _buildUrl() {
static async _buildUrl() {
await BROWSER_INFO_READY;
return (`${CMP_BASE_URL}/check
?os=${encodeURIComponent(BROWSER_INFO.os)}
&hw=${encodeURIComponent(conf.enable_human_web ? '1' : '0')}
ProTip! Use n and p to navigate between commits in a pull request.