Skip to content

Commit

Permalink
Chore: Only getBrowserInfo() once per preview instance (#393)
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsum authored Sep 14, 2017
1 parent faef3df commit ba0d1f7
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 30 deletions.
24 changes: 24 additions & 0 deletions src/lib/Browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,30 @@ class Browser {
(Browser.isIOS() && /(?:OS\s)10_3/i.test(userAgent)) || (Browser.isMac() && Browser.getName() === 'Safari')
);
}

/**
* Gets browser capability information.
*
* @private
* @return {Object} Browser capability information
*/
static getBrowserInfo() {
return {
name: Browser.getName(),
swf: Browser.hasFlash(),
svg: Browser.hasSVG(),
mse: Browser.hasMSE(),
webgl: Browser.hasWebGL(),
mp3: Browser.canPlayMP3(),
dash: Browser.canPlayDash(),
box3d: Browser.supportsModel3D(),
h264: {
baseline: Browser.canPlayH264Baseline(),
main: Browser.canPlayH264Main(),
high: Browser.canPlayH264High()
}
};
}
}

export default Browser;
31 changes: 3 additions & 28 deletions src/lib/Logger.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import Browser from './Browser';

/* eslint-disable no-undef */
const CLIENT_NAME = __NAME__;
const CLIENT_VERSION = __VERSION__;
Expand All @@ -10,14 +8,15 @@ class Logger {
* [constructor]
*
* @param {string} locale - Locale
* @param {Object} browser - Browser information
* @return {Logger} Logger instance
*/
constructor(locale) {
constructor(locale, browser) {
this.start = Date.now();
this.log = {
locale,
event: 'preview',
browser: this.getBrowserInfo(),
browser,
client: {
name: CLIENT_NAME,
version: CLIENT_VERSION
Expand All @@ -35,30 +34,6 @@ class Logger {
};
}

/**
* Gets browser capability information.
*
* @private
* @return {Object} Browser capability information
*/
getBrowserInfo() {
return {
name: Browser.getName(),
swf: Browser.hasFlash(),
svg: Browser.hasSVG(),
mse: Browser.hasMSE(),
webgl: Browser.hasWebGL(),
mp3: Browser.canPlayMP3(),
dash: Browser.canPlayDash(),
box3d: Browser.supportsModel3D(),
h264: {
baseline: Browser.canPlayH264Baseline(),
main: Browser.canPlayH264Main(),
high: Browser.canPlayH264High()
}
};
}

/**
* Marks file as cached.
*
Expand Down
3 changes: 2 additions & 1 deletion src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class Preview extends EventEmitter {

this.cache = new Cache();
this.ui = new PreviewUI();
this.browserInfo = Browser.getBrowserInfo();
}

/**
Expand Down Expand Up @@ -536,7 +537,7 @@ class Preview extends EventEmitter {
this.open = true;

// Init performance logging
this.logger = new Logger(this.location.locale);
this.logger = new Logger(this.location.locale, this.browserInfo);

// Clear any existing retry timeouts
clearTimeout(this.retryTimeout);
Expand Down
2 changes: 1 addition & 1 deletion src/lib/__tests__/Logger-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const sandbox = sinon.sandbox.create();

describe('lib/Logger', () => {
beforeEach(() => {
logger = new Logger('FOO');
logger = new Logger('FOO', {});
});

afterEach(() => {
Expand Down
2 changes: 2 additions & 0 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -716,9 +716,11 @@ describe('lib/Preview', () => {
});

it('should set the preview to open, and initialize the performance logger', () => {
sandbox.stub(Browser, 'getBrowserInfo');
preview.load('0');
expect(preview.open).to.be.true;
expect(preview.logger instanceof Logger);
expect(Browser.getBrowserInfo).to.not.be.called; // cached from preview constructor
});

it('should clear the retry timeout', () => {
Expand Down

0 comments on commit ba0d1f7

Please sign in to comment.