Skip to content

Commit 7764746

Browse files
mmermerkayammermerkaya
authored
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
1 parent dfd09bc commit 7764746

File tree

7 files changed

+182
-60
lines changed

7 files changed

+182
-60
lines changed

‎integration/shared/validator.js‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,16 @@ function validateNamespace(definition, candidate) {
8888
if (
8989
definitionChunk.__type === 'function' &&
9090
definitionChunk.__return &&
91-
typeof candidateChunk === 'function' &&
92-
candidateChunk()
91+
typeof candidateChunk === 'function'
9392
) {
93+
try {
94+
candidateChunk();
95+
} catch (e) {
96+
it(`Throws because current browser is unsupported`, () => {
97+
__expect(e.code).to.have.string('unsupported-browser');
98+
});
99+
return;
100+
}
94101
validateNamespace(definitionChunk.__return, candidateChunk());
95102
}
96103
});

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,16 @@ describe('Firebase Functions > Call', () => {
6868
// TODO(klimt): Move this to the cross-platform tests and delete this file,
6969
// once instance id works there.
7070
it('instance id', async () => {
71+
if (!('serviceWorker' in navigator)) {
72+
// Current platform does not support messaging, skip test.
73+
return;
74+
}
75+
7176
// Stub out the messaging method get an instance id token.
72-
const messaging = (firebase as any).messaging(app);
73-
const stub = sinon.stub(messaging, 'getToken');
74-
stub.returns(Promise.resolve('iid'));
77+
const messaging = firebase.messaging(app);
78+
const stub = sinon
79+
.stub(messaging, 'getToken')
80+
.returns(Promise.resolve('iid'));
7581

7682
const func = functions.httpsCallable('instanceIdTest');
7783
const result = await func({});

‎packages/messaging/index.ts‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,34 @@ import {
1919
_FirebaseNamespace,
2020
FirebaseServiceFactory
2121
} from '@firebase/app-types/private';
22+
2223
import { SWController } from './src/controllers/sw-controller';
2324
import { WindowController } from './src/controllers/window-controller';
25+
import { ERROR_CODES, errorFactory } from './src/models/errors';
2426

2527
import * as types from '@firebase/messaging-types';
2628

29+
export interface MessagingServiceFactory extends FirebaseServiceFactory {
30+
isSupported?(): boolean;
31+
}
32+
2733
export function registerMessaging(instance: _FirebaseNamespace): void {
2834
const messagingName = 'messaging';
29-
const factoryMethod: FirebaseServiceFactory = app => {
35+
36+
const factoryMethod: MessagingServiceFactory = app => {
37+
if (!isSupported()) {
38+
throw errorFactory.create(ERROR_CODES.UNSUPPORTED_BROWSER);
39+
}
40+
3041
if (self && 'ServiceWorkerGlobalScope' in self) {
42+
// Running in ServiceWorker context
3143
return new SWController(app);
44+
} else {
45+
// Assume we are in the window context.
46+
return new WindowController(app);
3247
}
33-
34-
// Assume we are in the window context.
35-
return new WindowController(app);
3648
};
49+
factoryMethod.isSupported = isSupported;
3750

3851
const namespaceExports = {
3952
// no-inline
@@ -63,3 +76,39 @@ declare module '@firebase/app-types' {
6376
messaging?(): types.FirebaseMessaging;
6477
}
6578
}
79+
80+
function isSupported(): boolean {
81+
if (self && 'ServiceWorkerGlobalScope' in self) {
82+
// Running in ServiceWorker context
83+
return isSWControllerSupported();
84+
} else {
85+
// Assume we are in the window context.
86+
return isWindowControllerSupported();
87+
}
88+
}
89+
90+
/**
91+
* Checks to see if the required APIs exist.
92+
*/
93+
function isWindowControllerSupported(): boolean {
94+
return (
95+
'serviceWorker' in navigator &&
96+
'PushManager' in window &&
97+
'Notification' in window &&
98+
'fetch' in window &&
99+
ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
100+
PushSubscription.prototype.hasOwnProperty('getKey')
101+
);
102+
}
103+
104+
/**
105+
* Checks to see if the required APIs exist within SW Context.
106+
*/
107+
function isSWControllerSupported(): boolean {
108+
return (
109+
'PushManager' in self &&
110+
'Notification' in self &&
111+
ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
112+
PushSubscription.prototype.hasOwnProperty('getKey')
113+
);
114+
}

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,6 @@ export class WindowController extends ControllerInterface {
7474
* permission isn't granted.
7575
*/
7676
getToken(): Promise<string | null> {
77-
// Check that the required API's are available
78-
if (!this.isSupported_()) {
79-
return Promise.reject(
80-
errorFactory.create(ERROR_CODES.UNSUPPORTED_BROWSER)
81-
);
82-
}
83-
8477
return this.manifestCheck_().then(() => {
8578
return super.getToken();
8679
});
@@ -355,10 +348,6 @@ export class WindowController extends ControllerInterface {
355348
// Visible for testing
356349
// TODO: Make private
357350
setupSWMessageListener_(): void {
358-
if (!('serviceWorker' in navigator)) {
359-
return;
360-
}
361-
362351
navigator.serviceWorker.addEventListener(
363352
'message',
364353
event => {
@@ -384,21 +373,4 @@ export class WindowController extends ControllerInterface {
384373
false
385374
);
386375
}
387-
388-
/**
389-
* Checks to see if the required API's are valid or not.
390-
* @return Returns true if the desired APIs are available.
391-
*/
392-
// Visible for testing
393-
// TODO: Make private
394-
isSupported_(): boolean {
395-
return (
396-
'serviceWorker' in navigator &&
397-
'PushManager' in window &&
398-
'Notification' in window &&
399-
'fetch' in window &&
400-
ServiceWorkerRegistration.prototype.hasOwnProperty('showNotification') &&
401-
PushSubscription.prototype.hasOwnProperty('getKey')
402-
);
403-
}
404376
}

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,6 @@ describe('Firebase Messaging > *Controller.getToken()', () => {
130130
return cleanUp();
131131
});
132132

133-
it('should throw on unsupported browsers', () => {
134-
sandbox
135-
.stub(WindowController.prototype, 'isSupported_')
136-
.callsFake(() => false);
137-
138-
const messagingService = new WindowController(app);
139-
return messagingService.getToken().then(
140-
() => {
141-
throw new Error('Expected getToken to throw ');
142-
},
143-
err => {
144-
assert.equal('messaging/' + ERROR_CODES.UNSUPPORTED_BROWSER, err.code);
145-
}
146-
);
147-
});
148-
149133
it('should handle a failure to get registration', () => {
150134
sandbox
151135
.stub(ControllerInterface.prototype, 'getNotificationPermission_')

‎packages/messaging/test/index.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* Copyright 2018 Google Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { expect } from 'chai';
18+
import { sandbox, SinonSandbox, SinonStub } from 'sinon';
19+
20+
import { FirebaseApp } from '@firebase/app-types';
21+
import { _FirebaseNamespace } from '@firebase/app-types/private';
22+
23+
import { MessagingServiceFactory, registerMessaging } from '../index';
24+
import { ERROR_CODES } from '../src/models/errors';
25+
26+
import { SWController } from '../src/controllers/sw-controller';
27+
import { WindowController } from '../src/controllers/window-controller';
28+
import { makeFakeApp } from './testing-utils/make-fake-app';
29+
30+
describe('Firebase Messaging > registerMessaging', () => {
31+
let sinonSandbox: SinonSandbox;
32+
let registerService: SinonStub;
33+
let fakeFirebase: _FirebaseNamespace;
34+
35+
beforeEach(() => {
36+
sinonSandbox = sandbox.create();
37+
registerService = sinonSandbox.stub();
38+
39+
fakeFirebase = {
40+
INTERNAL: { registerService }
41+
} as any;
42+
});
43+
44+
afterEach(() => {
45+
sinonSandbox.restore();
46+
});
47+
48+
it('calls registerService', () => {
49+
registerMessaging(fakeFirebase);
50+
expect(registerService.callCount).to.equal(1);
51+
});
52+
53+
describe('factoryMethod', () => {
54+
let factoryMethod: MessagingServiceFactory;
55+
let fakeApp: FirebaseApp;
56+
57+
beforeEach(() => {
58+
registerMessaging(fakeFirebase);
59+
factoryMethod = registerService.getCall(0).args[1];
60+
61+
fakeApp = makeFakeApp({
62+
messagingSenderId: '1234567890'
63+
});
64+
});
65+
66+
describe('isSupported', () => {
67+
it('is a static method on factoryMethod', () => {
68+
expect(factoryMethod.isSupported).to.be.a('function');
69+
});
70+
});
71+
72+
describe('in Service Worker context', () => {
73+
beforeEach(() => {
74+
// self.ServiceWorkerGlobalScope exists
75+
// can't stub a non-existing property, so no sinon.stub().
76+
(self as any).ServiceWorkerGlobalScope = {};
77+
});
78+
79+
afterEach(() => {
80+
delete (self as any).ServiceWorkerGlobalScope;
81+
});
82+
83+
it('returns a SWController', () => {
84+
const firebaseService = factoryMethod(fakeApp);
85+
expect(firebaseService).to.be.instanceOf(SWController);
86+
});
87+
});
88+
89+
describe('in Window context', () => {
90+
it('throws if required globals do not exist', () => {
91+
// Empty navigator, no navigator.serviceWorker ¯\_(ツ)_/¯
92+
sinonSandbox.stub(window, 'navigator').value({});
93+
94+
try {
95+
factoryMethod(fakeApp);
96+
} catch (e) {
97+
expect(e.code).to.equal(
98+
'messaging/' + ERROR_CODES.UNSUPPORTED_BROWSER
99+
);
100+
return;
101+
}
102+
throw new Error('Expected getToken to throw ');
103+
});
104+
105+
it('returns a WindowController', () => {
106+
const firebaseService = factoryMethod(fakeApp);
107+
expect(firebaseService).to.be.instanceOf(WindowController);
108+
});
109+
});
110+
});
111+
});

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -349,13 +349,6 @@ describe('Firebase Messaging > *WindowController', () => {
349349
});
350350

351351
describe('setupSWMessageListener_()', () => {
352-
it('should not do anything is no service worker support', () => {
353-
sandbox.stub(window, 'navigator').value({});
354-
355-
const controller = new WindowController(app);
356-
controller.setupSWMessageListener_();
357-
});
358-
359352
it('should add listener when supported', () => {
360353
const spy = sandbox.spy();
361354
sandbox.stub(navigator, 'serviceWorker').value({

0 commit comments

Comments
 (0)