took validateIndexedDBOpenable outside of isRequiredApisAvailable

XuechunHou · XuechunHou · commit e012877d55b4 · 2020-08-05T17:24:08.000-06:00
diff --git a/packages/performance/src/controllers/perf.test.ts b/packages/performance/src/controllers/perf.test.ts
@@ -22,6 +22,8 @@ import { Trace } from '../resources/trace';
 import { Api, setupApi } from '../services/api_service';
 import { FirebaseApp } from '@firebase/app-types';
 import * as initializationService from '../services/initialization_service';
+import * as FirebaseUtil from '@firebase/util';
+import { consoleLogger } from '../utils/console_logger';
 import '../../test/setup';
 
 describe('Firebase Performance Test', () => {
@@ -43,13 +45,30 @@ describe('Firebase Performance Test', () => {
 
   describe('#constructor', () => {
     it('does not initialize performance if the required apis are not available', () => {
-      stub(Api.prototype, 'requiredApisAvailable').returns(
+      stub(Api.prototype, 'requiredApisAvailable').returns(false);
+      stub(initializationService, 'getInitializationPromise');
+      new PerformanceController(fakeFirebaseApp);
+      expect(initializationService.getInitializationPromise).not.be.called;
+    });
+    it('does not initialize performance if validateIndexedDBOpenable return false', () => {
+      stub(Api.prototype, 'requiredApisAvailable').returns(true);
+      stub(FirebaseUtil, 'validateIndexedDBOpenable').returns(
         Promise.resolve(false)
       );
       stub(initializationService, 'getInitializationPromise');
       new PerformanceController(fakeFirebaseApp);
       expect(initializationService.getInitializationPromise).not.be.called;
     });
+
+    it('does not initialize performance if validateIndexedDBOpenable throws an error', () => {
+      stub(Api.prototype, 'requiredApisAvailable').returns(true);
+      stub(FirebaseUtil, 'validateIndexedDBOpenable').throws();
+      stub(initializationService, 'getInitializationPromise');
+      stub(consoleLogger, 'info');
+      new PerformanceController(fakeFirebaseApp);
+      expect(initializationService.getInitializationPromise).not.be.called;
+      expect(consoleLogger.info).to.be.called;
+    });
   });
 
   describe('#trace', () => {
diff --git a/packages/performance/src/controllers/perf.ts b/packages/performance/src/controllers/perf.ts
@@ -22,17 +22,26 @@ import { Api } from '../services/api_service';
 import { FirebaseApp } from '@firebase/app-types';
 import { FirebasePerformance } from '@firebase/performance-types';
 import { setupTransportService } from '../services/transport_service';
+import { validateIndexedDBOpenable } from '@firebase/util';
+import { consoleLogger } from '../utils/console_logger';
 export class PerformanceController implements FirebasePerformance {
   constructor(readonly app: FirebaseApp) {
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    Api.getInstance()
-      .requiredApisAvailable()
-      .then(isAvailable => {
-        if (isAvailable) {
-          setupTransportService();
-          getInitializationPromise().then(setupOobResources, setupOobResources);
-        }
-      });
+    if (Api.getInstance().requiredApisAvailable()) {
+      validateIndexedDBOpenable()
+        .then(isAvailable => {
+          if (isAvailable) {
+            setupTransportService();
+            getInitializationPromise().then(
+              setupOobResources,
+              setupOobResources
+            );
+          }
+        })
+        .catch(error => {
+          consoleLogger.info(`Environment doesn't support IndexedDB: ${error}`);
+        });
+    }
   }
 
   trace(name: string): Trace {
diff --git a/packages/performance/src/services/api_service.test.ts b/packages/performance/src/services/api_service.test.ts
@@ -19,9 +19,6 @@ import { stub } from 'sinon';
 import { expect } from 'chai';
 import { Api, setupApi } from './api_service';
 import '../../test/setup';
-import * as FirebaseUtil from '@firebase/util';
-import { consoleLogger } from '../utils/console_logger';
-
 describe('Firebase Performance > api_service', () => {
   const PAGE_URL = 'http://www.test.com/abcd?a=2';
   const PERFORMANCE_ENTRY: PerformanceEntry = {
@@ -53,74 +50,6 @@ describe('Firebase Performance > api_service', () => {
     setupApi(mockWindow);
     api = Api.getInstance();
   });
-  describe('requiredApisAvailable', () => {
-    it('call logger when fetch is not available', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').value(null);
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.be.called;
-        expect(isAvailable).to.be.false;
-      });
-    });
-    it('call logger when navigator is not available', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').returns(Promise.resolve(new Response('{}')));
-      stub(api, 'navigator').value(null);
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.be.called;
-        expect(isAvailable).to.be.false;
-      });
-    });
-    it('call logger when cookie is not enabled', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').returns(Promise.resolve(new Response('{}')));
-      stub(api.navigator, 'cookieEnabled').value(false);
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.be.called;
-        expect(isAvailable).to.be.false;
-      });
-    });
-
-    it('call logger when isIndexedDBAvailable returns false', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').returns(Promise.resolve(new Response('{}')));
-      stub(FirebaseUtil, 'isIndexedDBAvailable').returns(false);
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.be.called;
-        expect(isAvailable).to.be.false;
-      });
-    });
-
-    it('call logger when validateIndexedDBOpenable throws an exception', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').returns(Promise.resolve(new Response('{}')));
-      stub(FirebaseUtil, 'isIndexedDBAvailable').returns(true);
-      stub(FirebaseUtil, 'validateIndexedDBOpenable').throws();
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.be.called;
-        expect(isAvailable).to.be.false;
-      });
-    });
-    it('logger not called when function returns true', () => {
-      expect(window.Promise).to.exist;
-      stub(consoleLogger, 'info');
-      stub(window, 'fetch').returns(Promise.resolve(new Response('{}')));
-      stub(FirebaseUtil, 'isIndexedDBAvailable').returns(true);
-      stub(FirebaseUtil, 'validateIndexedDBOpenable').returns(
-        Promise.resolve(true)
-      );
-
-      return api.requiredApisAvailable().then(isAvailable => {
-        expect(consoleLogger.info).to.not.be.called;
-        expect(isAvailable).to.be.true;
-      });
-    });
-  });
 
   describe('getUrl', () => {
     it('removes the query params', () => {
diff --git a/packages/performance/src/services/api_service.ts b/packages/performance/src/services/api_service.ts
@@ -16,10 +16,7 @@
  */
 
 import { ERROR_FACTORY, ErrorCode } from '../utils/errors';
-import {
-  isIndexedDBAvailable,
-  validateIndexedDBOpenable
-} from '@firebase/util';
+import { isIndexedDBAvailable } from '@firebase/util';
 import { consoleLogger } from '../utils/console_logger';
 declare global {
   interface Window {
@@ -113,7 +110,7 @@ export class Api {
     );
   }
 
-  async requiredApisAvailable(): Promise<boolean> {
+  requiredApisAvailable(): boolean {
     if (
       !fetch ||
       !Promise ||
@@ -130,13 +127,7 @@ export class Api {
       consoleLogger.info('IndexedDB is not supported by current browswer');
       return false;
     }
-    try {
-      await validateIndexedDBOpenable();
-      return true;
-    } catch (error) {
-      consoleLogger.info(`Environment doesn't support IndexedDB: ${error}`);
-      return false;
-    }
+    return true;
   }
 
   setupObserver(
diff --git a/packages/performance/src/services/perf_logger.test.ts b/packages/performance/src/services/perf_logger.test.ts
@@ -113,9 +113,7 @@ describe('Performance Monitoring > perf_logger', () => {
     });
 
     it('does not log an event if cookies are disabled in the browser', () => {
-      stub(Api.prototype, 'requiredApisAvailable').returns(
-        Promise.resolve(false)
-      );
+      stub(Api.prototype, 'requiredApisAvailable').returns(false);
       stub(attributeUtils, 'getVisibilityState').returns(VISIBILITY_STATE);
       const trace = new Trace(TRACE_NAME);
       trace.record(START_TIME, DURATION);
diff --git a/packages/performance/src/services/perf_logger.ts b/packages/performance/src/services/perf_logger.ts
@@ -111,16 +111,9 @@ export function logTrace(trace: Trace): void {
     return;
   }
   // Do not log if required apis are not available.
-  Api.getInstance()
-    .requiredApisAvailable()
-    .then(isAvailable => {
-      if (!isAvailable) {
-        return;
-      }
-    })
-    .catch(() => {
-      return;
-    });
+  if (!Api.getInstance().requiredApisAvailable()) {
+    return;
+  }
 
   // Only log the page load auto traces if page is visible.
   if (trace.isAuto && getVisibilityState() !== VisibilityState.VISIBLE) {
