Skip to content

Commit 95cbd6f

Browse files
authored
Do not initialize performance if required apis are not available (#1895)
1 parent 728f4f5 commit 95cbd6f

File tree

6 files changed

+56
-16
lines changed

6 files changed

+56
-16
lines changed

packages/performance/index.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import {
2424
import { PerformanceController } from './src/controllers/perf';
2525
import { setupApi } from './src/services/api_service';
2626
import { SettingsService } from './src/services/settings_service';
27-
import { consoleLogger } from './src/utils/console_logger';
2827
import { ERROR_FACTORY, ErrorCode } from './src/utils/errors';
2928
import { FirebasePerformance } from '@firebase/performance-types';
3029

@@ -50,14 +49,8 @@ export function registerPerformance(instance: FirebaseNamespace): void {
5049
);
5150
}
5251

53-
if (window && fetch && Promise) {
54-
setupApi(window);
55-
registerPerformance(firebase);
56-
} else {
57-
consoleLogger.info(
58-
'Firebase Performance cannot start if browser does not support fetch and Promise.'
59-
);
60-
}
52+
setupApi(window);
53+
registerPerformance(firebase);
6154

6255
declare module '@firebase/app-types' {
6356
interface FirebaseNamespace {

packages/performance/src/controllers/perf.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@
1616
*/
1717

1818
import { expect } from 'chai';
19+
import { stub } from 'sinon';
1920
import { PerformanceController } from '../controllers/perf';
2021
import { Trace } from '../resources/trace';
21-
import { setupApi } from '../services/api_service';
22+
import { Api, setupApi } from '../services/api_service';
2223
import { FirebaseApp } from '@firebase/app-types';
24+
import * as initializationService from '../services/initialization_service';
25+
import { consoleLogger } from '../utils/console_logger';
26+
import '../../test/setup';
2327

2428
describe('Firebase Performance Test', () => {
2529
setupApi(window);
@@ -38,6 +42,18 @@ describe('Firebase Performance Test', () => {
3842
options: fakeFirebaseConfig
3943
} as unknown) as FirebaseApp;
4044

45+
describe('#constructor', () => {
46+
it('does not initialize performance if the required apis are not available', () => {
47+
stub(Api.prototype, 'requiredApisAvailable').returns(false);
48+
stub(initializationService, 'getInitializationPromise');
49+
stub(consoleLogger, 'info');
50+
new PerformanceController(fakeFirebaseApp);
51+
52+
expect(initializationService.getInitializationPromise).not.be.called;
53+
expect(consoleLogger.info).be.called;
54+
});
55+
});
56+
4157
describe('#trace', () => {
4258
it('creates a custom trace', () => {
4359
const controller = new PerformanceController(fakeFirebaseApp);

packages/performance/src/controllers/perf.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,20 @@ import { Trace } from '../resources/trace';
1818
import { setupOobResources } from '../services/oob_resources_service';
1919
import { SettingsService } from '../services/settings_service';
2020
import { getInitializationPromise } from '../services/initialization_service';
21+
import { Api } from '../services/api_service';
2122
import { FirebaseApp } from '@firebase/app-types';
2223
import { FirebasePerformance } from '@firebase/performance-types';
24+
import { consoleLogger } from '../utils/console_logger';
2325

2426
export class PerformanceController implements FirebasePerformance {
2527
constructor(readonly app: FirebaseApp) {
26-
getInitializationPromise().then(setupOobResources, setupOobResources);
28+
if (Api.getInstance().requiredApisAvailable()) {
29+
getInitializationPromise().then(setupOobResources, setupOobResources);
30+
} else {
31+
consoleLogger.info(
32+
'Firebase Performance cannot start if browser does not support fetch and Promise or cookie is disabled.'
33+
);
34+
}
2735
}
2836

2937
trace(name: string): Trace {

packages/performance/src/services/api_service.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export class Api {
4444
private PerformanceObserver: typeof PerformanceObserver;
4545
private windowLocation: Location;
4646
onFirstInputDelay?: Function;
47-
localStorage: Storage;
47+
localStorage!: Storage;
4848
document: Document;
4949
navigator: Navigator;
5050

@@ -57,7 +57,10 @@ export class Api {
5757
this.windowLocation = window.location;
5858
this.navigator = window.navigator;
5959
this.document = window.document;
60-
this.localStorage = window.localStorage;
60+
if (this.navigator && this.navigator.cookieEnabled) {
61+
// If user blocks cookies on the browser, accessing localStorage will throw an exception.
62+
this.localStorage = window.localStorage;
63+
}
6164
if (window.perfMetrics && window.perfMetrics.onFirstInputDelay) {
6265
this.onFirstInputDelay = window.perfMetrics.onFirstInputDelay;
6366
}
@@ -104,6 +107,13 @@ export class Api {
104107
);
105108
}
106109

110+
requiredApisAvailable(): boolean {
111+
if (fetch && Promise && this.navigator && this.navigator.cookieEnabled) {
112+
return true;
113+
}
114+
return false;
115+
}
116+
107117
setupObserver(
108118
entryType: EntryType,
109119
callback: (entry: PerformanceEntry) => void

packages/performance/src/services/perf_logger.test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ describe('Performance Monitoring > perf_logger', () => {
3838
const EFFECTIVE_CONNECTION_TYPE = 2;
3939
const SERVICE_WORKER_STATUS = 3;
4040
const TIME_ORIGIN = 1556512199893.9033;
41+
const TRACE_NAME = 'testTrace';
42+
const START_TIME = 12345;
43+
const DURATION = 321;
4144

4245
let addToQueueStub: SinonStub<
4346
Array<{ message: string; eventTime: number }>,
@@ -80,9 +83,6 @@ describe('Performance Monitoring > perf_logger', () => {
8083

8184
describe('logTrace', () => {
8285
it('creates, serializes and sends a trace to cc service', () => {
83-
const TRACE_NAME = 'testTrace';
84-
const START_TIME = 12345;
85-
const DURATION = 321;
8686
const EXPECTED_TRACE_MESSAGE = `{"application_info":{"google_app_id":"${APP_ID}",\
8787
"app_instance_id":"${IID}","web_app_info":{"sdk_version":"${SDK_VERSION}",\
8888
"page_url":"${PAGE_URL}","service_worker_status":${SERVICE_WORKER_STATUS},\
@@ -101,6 +101,15 @@ describe('Performance Monitoring > perf_logger', () => {
101101
EXPECTED_TRACE_MESSAGE
102102
);
103103
});
104+
105+
it('does not log an event if cookies are disabled in the browser', () => {
106+
stub(Api.prototype, 'requiredApisAvailable').returns(false);
107+
const trace = new Trace(TRACE_NAME);
108+
trace.record(START_TIME, DURATION);
109+
clock.tick(1);
110+
111+
expect(addToQueueStub).not.to.be.called;
112+
});
104113
});
105114

106115
describe('logNetworkRequest', () => {

packages/performance/src/services/perf_logger.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ export function logTrace(trace: Trace): void {
107107
if (!settingsService.dataCollectionEnabled && !trace.isAuto) {
108108
return;
109109
}
110+
// Do not log if required apis are not available.
111+
if (!Api.getInstance().requiredApisAvailable()) {
112+
return;
113+
}
110114
// Only log the page load auto traces if page is visible.
111115
if (trace.isAuto && getVisibilityState() !== VisibilityState.VISIBLE) {
112116
return;

0 commit comments

Comments
 (0)