Check browser support before instantiating controllers (#700)

* Check browser support before instantiating controllers

Prevents issues such as #658 by making sure we don't call any unsupported methods by accident.

Also adds some tests for index.ts.

* Skip tests on unsupported browsers

* Ignore 'unsupported-browser' errors in namespace validation

* Check browser support within SW

* Implement isSupported static method

* [AUTOMATED]: Prettier Code Styling

integration/shared/validator.js

@@ -88,9 +88,16 @@ function validateNamespace(definition, candidate) {
       if (
         definitionChunk.__type === 'function' &&
         definitionChunk.__return &&
-        typeof candidateChunk === 'function' &&
-        candidateChunk()
+        typeof candidateChunk === 'function'
       ) {
+        try {
+          candidateChunk();
+        } catch (e) {
+          it(`Throws because current browser is unsupported`, () => {
+            __expect(e.code).to.have.string('unsupported-browser');
+          });
+          return;
+        }
         validateNamespace(definitionChunk.__return, candidateChunk());
       }
     });

packages/functions/test/browser/callable.test.ts

@@ -68,10 +68,16 @@ describe('Firebase Functions > Call', () => {
   // TODO(klimt): Move this to the cross-platform tests and delete this file,
   // once instance id works there.
   it('instance id', async () => {
+    if (!('serviceWorker' in navigator)) {
+      // Current platform does not support messaging, skip test.
+      return;
+    }
+
     // Stub out the messaging method get an instance id token.
-    const messaging = (firebase as any).messaging(app);
-    const stub = sinon.stub(messaging, 'getToken');
-    stub.returns(Promise.resolve('iid'));
+    const messaging = firebase.messaging(app);
+    const stub = sinon
+      .stub(messaging, 'getToken')
+      .returns(Promise.resolve('iid'));
 
     const func = functions.httpsCallable('instanceIdTest');
     const result = await func({});

packages/messaging/index.ts

@@ -19,21 +19,34 @@ import {
   _FirebaseNamespace,
   FirebaseServiceFactory
 } from '@firebase/app-types/private';
+
 import { SWController } from './src/controllers/sw-controller';
 import { WindowController } from './src/controllers/window-controller';
+import { ERROR_CODES, errorFactory } from './src/models/errors';
 
 import * as types from '@firebase/messaging-types';
 
+export interface MessagingServiceFactory extends FirebaseServiceFactory {
+  isSupported?(): boolean;
+}
+
 export function registerMessaging(instance: _FirebaseNamespace): void {
   const messagingName = 'messaging';
-  const factoryMethod: FirebaseServiceFactory = app => {
-    if (self && 'ServiceWorkerGlobalScope' in self) {
-      return new SWController(app);
+
+  const factoryMethod: MessagingServiceFactory = app => {
+    if (!isSupported()) {
+      throw errorFactory.create(ERROR_CODES.UNSUPPORTED_BROWSER);
     }
 
-    // Assume we are in the window context.
-    return new WindowController(app);
+    if (self && 'ServiceWorkerGlobalScope' in self) {
+      // Running in ServiceWorker context
+      return new SWController(app);
+    } else {
+      // Assume we are in the window context.
+      return new WindowController(app);
+    }
   };
+  factoryMethod.isSupported = isSupported;
 
   const namespaceExports = {
     // no-inline
@@ -63,3 +76,39 @@ declare module '@firebase/app-types' {
     messaging?(): types.FirebaseMessaging;
   }
 }
+
+function isSupported(): boolean {
+  if (self && 'ServiceWorkerGlobalScope' in self) {
+    // Running in ServiceWorker context
+    return isSWControllerSupported();
+  } else {
+    // Assume we are in the window context.
+    return isWindowControllerSupported();
+  }
+}
+
+/**
+ * Checks to see if the required APIs exist.
+ */
+function isWindowControllerSupported(): boolean {
+  return (
+    'serviceWorker' in navigator &&
+    'PushManager' in window &&
+    'Notification' in window &&
+    'fetch' in window &&
+    ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
+    PushSubscription.prototype.hasOwnProperty('getKey')
+  );
+}
+
+/**
+ * Checks to see if the required APIs exist within SW Context.
+ */
+function isSWControllerSupported(): boolean {
+  return (
+    'PushManager' in self &&
+    'Notification' in self &&
+    ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
+    PushSubscription.prototype.hasOwnProperty('getKey')
+  );
+}

packages/messaging/src/controllers/window-controller.ts

@@ -74,13 +74,6 @@ export class WindowController extends ControllerInterface {
    * permission isn't granted.
    */
   getToken(): Promise<string | null> {
-    // Check that the required API's are available
-    if (!this.isSupported_()) {
-      return Promise.reject(
-        errorFactory.create(ERROR_CODES.UNSUPPORTED_BROWSER)
-      );
-    }
-
     return this.manifestCheck_().then(() => {
       return super.getToken();
     });
@@ -355,10 +348,6 @@ export class WindowController extends ControllerInterface {
   // Visible for testing
   // TODO: Make private
   setupSWMessageListener_(): void {
-    if (!('serviceWorker' in navigator)) {
-      return;
-    }
-
     navigator.serviceWorker.addEventListener(
       'message',
       event => {
@@ -384,21 +373,4 @@ export class WindowController extends ControllerInterface {
       false
     );
   }
-
-  /**
-   * Checks to see if the required API's are valid or not.
-   * @return Returns true if the desired APIs are available.
-   */
-  // Visible for testing
-  // TODO: Make private
-  isSupported_(): boolean {
-    return (
-      'serviceWorker' in navigator &&
-      'PushManager' in window &&
-      'Notification' in window &&
-      'fetch' in window &&
-      ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
-      PushSubscription.prototype.hasOwnProperty('getKey')
-    );
-  }
 }

packages/messaging/test/controller-get-token.test.ts

@@ -130,22 +130,6 @@ describe('Firebase Messaging > *Controller.getToken()', () => {
     return cleanUp();
   });
 
-  it('should throw on unsupported browsers', () => {
-    sandbox
-      .stub(WindowController.prototype, 'isSupported_')
-      .callsFake(() => false);
-
-    const messagingService = new WindowController(app);
-    return messagingService.getToken().then(
-      () => {
-        throw new Error('Expected getToken to throw ');
-      },
-      err => {
-        assert.equal('messaging/' + ERROR_CODES.UNSUPPORTED_BROWSER, err.code);
-      }
-    );
-  });
-
   it('should handle a failure to get registration', () => {
     sandbox
       .stub(ControllerInterface.prototype, 'getNotificationPermission_')

packages/messaging/test/index.test.ts

@@ -0,0 +1,111 @@
+/**
+ * Copyright 2018 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect } from 'chai';
+import { sandbox, SinonSandbox, SinonStub } from 'sinon';
+
+import { FirebaseApp } from '@firebase/app-types';
+import { _FirebaseNamespace } from '@firebase/app-types/private';
+
+import { MessagingServiceFactory, registerMessaging } from '../index';
+import { ERROR_CODES } from '../src/models/errors';
+
+import { SWController } from '../src/controllers/sw-controller';
+import { WindowController } from '../src/controllers/window-controller';
+import { makeFakeApp } from './testing-utils/make-fake-app';
+
+describe('Firebase Messaging > registerMessaging', () => {
+  let sinonSandbox: SinonSandbox;
+  let registerService: SinonStub;
+  let fakeFirebase: _FirebaseNamespace;
+
+  beforeEach(() => {
+    sinonSandbox = sandbox.create();
+    registerService = sinonSandbox.stub();
+
+    fakeFirebase = {
+      INTERNAL: { registerService }
+    } as any;
+  });
+
+  afterEach(() => {
+    sinonSandbox.restore();
+  });
+
+  it('calls registerService', () => {
+    registerMessaging(fakeFirebase);
+    expect(registerService.callCount).to.equal(1);
+  });
+
+  describe('factoryMethod', () => {
+    let factoryMethod: MessagingServiceFactory;
+    let fakeApp: FirebaseApp;
+
+    beforeEach(() => {
+      registerMessaging(fakeFirebase);
+      factoryMethod = registerService.getCall(0).args[1];
+
+      fakeApp = makeFakeApp({
+        messagingSenderId: '1234567890'
+      });
+    });
+
+    describe('isSupported', () => {
+      it('is a static method on factoryMethod', () => {
+        expect(factoryMethod.isSupported).to.be.a('function');
+      });
+    });
+
+    describe('in Service Worker context', () => {
+      beforeEach(() => {
+        // self.ServiceWorkerGlobalScope exists
+        // can't stub a non-existing property, so no sinon.stub().
+        (self as any).ServiceWorkerGlobalScope = {};
+      });
+
+      afterEach(() => {
+        delete (self as any).ServiceWorkerGlobalScope;
+      });
+
+      it('returns a SWController', () => {
+        const firebaseService = factoryMethod(fakeApp);
+        expect(firebaseService).to.be.instanceOf(SWController);
+      });
+    });
+
+    describe('in Window context', () => {
+      it('throws if required globals do not exist', () => {
+        // Empty navigator, no navigator.serviceWorker ¯\_(ツ)_/¯
+        sinonSandbox.stub(window, 'navigator').value({});
+
+        try {
+          factoryMethod(fakeApp);
+        } catch (e) {
+          expect(e.code).to.equal(
+            'messaging/' + ERROR_CODES.UNSUPPORTED_BROWSER
+          );
+          return;
+        }
+        throw new Error('Expected getToken to throw ');
+      });
+
+      it('returns a WindowController', () => {
+        const firebaseService = factoryMethod(fakeApp);
+        expect(firebaseService).to.be.instanceOf(WindowController);
+      });
+    });
+  });
+});

packages/messaging/test/window-controller.test.ts

@@ -349,13 +349,6 @@ describe('Firebase Messaging > *WindowController', () => {
   });
 
   describe('setupSWMessageListener_()', () => {
-    it('should not do anything is no service worker support', () => {
-      sandbox.stub(window, 'navigator').value({});
-
-      const controller = new WindowController(app);
-      controller.setupSWMessageListener_();
-    });
-
     it('should add listener when supported', () => {
       const spy = sandbox.spy();
       sandbox.stub(navigator, 'serviceWorker').value({