Fix FCM rxjs Incompatibility (#3221)

* Enable the FCM integration test (IT) for Chrome.

Context: the tests were switched off a couple of years ago due to flakiness. It has not been maintained since then. During the time, Some Selenium API methods used were deprecated and removed; the Firebase projects used are no longer managed or owned by the FCM team. Consequently, the test became unfunctional. In the effort of providing safety for the upcoming FCM releases, this PR is created to fix, deflake, refactor and improve the old tests.

This PR did the following:
- Enabled comprehensive IT for chrome (ver.80). Now we are covering send&foreground recevie for FCM messages (messages with {notification} payload, {data} payload and {notification, data} payload), delete/update token, use default/customized ServiceWorker.

- Defalaked test. The IT is now reasonably stable without retry. Previously we are retrying 1 or 3 times.

- Optimized test. Previously we create a webDriver for each test, which is slow and annoying. Because a window is created and brought to focus and killed frequently, it makes working on other tasks and testing nearly impossible (Probably using a headless browser would work but I haven't found a satisfying solution to have the app in the state of foreground and background which is a requirement for FCM functions). With the way the tests are organized, the IT only spin up a new web driver when necessary. Some data on performance: (old) 'test-send' take 74 seconds (only measured 'test-send' because the other test suites were not functional at the time); (now) 'test-send', 'test-deleteToken', 'test-updateToken', 'test-useDefaultServiceWorker', 'test-useValidManifest' takes in total 33s (10 run average).

- General refactoring. Including refactors on expect blocks, createWebDriver, use const for constants usage, etc. The code should be much easier to understand and maintain.

Future work:
- Enable test on firefox once I get the notification permission working.
- Run the IC against some milestone chrome/firefox version (if it makes sense) to ensure backward compatibility. We should try to avoid #2712 . :)

* [AUTOMATED]: License Headers

* Enable integration test (IT) for FCM.

Context: FCM IT were turned off a couple years ago due to flakiness. It became mostly unfunctional as repo structure change overtime. The goal is to fix and enable IT for more confident developement flow and safer releases.

This CL does the following:
- Fix the IT to be functional again. The IT is derteminated from my experiements (no longer flaky). Therefore, The CL removed the retry mechasim (previously retry 3 times) which makes running IT cheaper and more enjoyable.

- Include IT for test:change for FCM package: the entire IT test suites is resoanblly fast (from my exeperience 1-3 miutes to complete. As it grows larger, maybe it makes run tests in parallel in Saucelab)

Futhure work:
- Enable testing for firefox

* Correct int syntax

js doesn't allow underscore for int

* This file wasn't auto-saved

* Trigger FCM IT

why dot.env(FCM_SECRETS) are not included in env?

* Test Secrets can be accessed w/o dotenv

* Add fcm sercret to workflow

* Update test-changed.yml

* test send (background only)

Because headless chrome doesn't have the concept of foreground and background

* remove dotenv

* feed secrest into test:all workflow

* Update test-all.yml

* background messaging checking

* [AUTOMATED]: License Headers

* rerun

* added waiting

* wait

* Update test-send.js

* Update test-send.js

* Examine wrong sercret

* Update sendMessage.js

* Update sendMessage.js

* Update sendMessage.js

* Update sendMessage.js

* Update sendMessage.js

* update fcm project

* Update package.json

* Update test-send.js

* open new tab for backgournd receive

* removed  test-send

somehow not workingin github workflow?

* Adding Reties

* Change timeout limit

* retry 3 times

* adjust mocha setting

* update

* Enable foreground tesing

* Fix FCM rxjs incompatibility

* Use one listener to handle next and observer for onMessage

* Add changeset to the PR

* Update tough-rings-bake.md

Co-authored-by: Kai Wu <[email protected]>

Author: zwu52

.changeset/tough-rings-bake.md

@@ -0,0 +1,6 @@
+---
+'firebase': patch
+'@firebase/messaging': patch
+---
+
+Added support for `onMessage` so the internal callback can work with [Subscriber](https://rxjs.dev/api/index/class/Subscriber)

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

@@ -1,3 +1,25 @@
+import '../testing/setup';
+
+import * as tokenManagementModule from '../core/token-management';
+
+import {
+  CONSOLE_CAMPAIGN_ANALYTICS_ENABLED,
+  CONSOLE_CAMPAIGN_ID,
+  CONSOLE_CAMPAIGN_NAME,
+  CONSOLE_CAMPAIGN_TIME,
+  DEFAULT_SW_PATH,
+  DEFAULT_SW_SCOPE,
+  DEFAULT_VAPID_KEY
+} from '../util/constants';
+import { InternalMessage, MessageType } from '../interfaces/internal-message';
+import { SinonFakeTimers, SinonSpy, spy, stub, useFakeTimers } from 'sinon';
+import { Spy, Stub } from '../testing/sinon-types';
+
+import { ErrorCode } from '../util/errors';
+import { FakeServiceWorkerRegistration } from '../testing/fakes/service-worker';
+import { FirebaseAnalyticsInternal } from '@firebase/analytics-interop-types';
+import { FirebaseInternalDependencies } from '../interfaces/internal-dependencies';
+import { WindowController } from './window-controller';
 /**
  * @license
  * Copyright 2017 Google LLC
@@ -15,27 +37,7 @@
  * limitations under the License.
  */
 import { expect } from 'chai';
-import { stub, spy, SinonSpy, useFakeTimers, SinonFakeTimers } from 'sinon';
-
-import { FirebaseAnalyticsInternal } from '@firebase/analytics-interop-types';
-import { WindowController } from './window-controller';
 import { getFakeFirebaseDependencies } from '../testing/fakes/firebase-dependencies';
-import { ErrorCode } from '../util/errors';
-import { FirebaseInternalDependencies } from '../interfaces/internal-dependencies';
-import * as tokenManagementModule from '../core/token-management';
-import {
-  DEFAULT_VAPID_KEY,
-  DEFAULT_SW_SCOPE,
-  DEFAULT_SW_PATH,
-  CONSOLE_CAMPAIGN_ANALYTICS_ENABLED,
-  CONSOLE_CAMPAIGN_ID,
-  CONSOLE_CAMPAIGN_NAME,
-  CONSOLE_CAMPAIGN_TIME
-} from '../util/constants';
-import { Stub, Spy } from '../testing/sinon-types';
-import '../testing/setup';
-import { FakeServiceWorkerRegistration } from '../testing/fakes/service-worker';
-import { MessageType, InternalMessage } from '../interfaces/internal-message';
 
 type MessageEventListener = (event: Event) => Promise<void>;
 

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

@@ -15,30 +15,31 @@
  * limitations under the License.
  */
 
-import { getToken, deleteToken } from '../core/token-management';
-import { FirebaseInternalDependencies } from '../interfaces/internal-dependencies';
-import { FirebaseMessaging } from '@firebase/messaging-types';
-import { ERROR_FACTORY, ErrorCode } from '../util/errors';
-import { NextFn, Observer, Unsubscribe } from '@firebase/util';
-import { InternalMessage, MessageType } from '../interfaces/internal-message';
 import {
-  CONSOLE_CAMPAIGN_ID,
   CONSOLE_CAMPAIGN_ANALYTICS_ENABLED,
+  CONSOLE_CAMPAIGN_ID,
   CONSOLE_CAMPAIGN_NAME,
   CONSOLE_CAMPAIGN_TIME,
   DEFAULT_SW_PATH,
   DEFAULT_SW_SCOPE,
   DEFAULT_VAPID_KEY
 } from '../util/constants';
-import { FirebaseApp } from '@firebase/app-types';
+import { ERROR_FACTORY, ErrorCode } from '../util/errors';
+import { InternalMessage, MessageType } from '../interfaces/internal-message';
+import { NextFn, Observer, Unsubscribe } from '@firebase/util';
+import { deleteToken, getToken } from '../core/token-management';
+
 import { ConsoleMessageData } from '../interfaces/message-payload';
-import { isConsoleMessage } from '../helpers/is-console-message';
+import { FirebaseApp } from '@firebase/app-types';
+import { FirebaseInternalDependencies } from '../interfaces/internal-dependencies';
+import { FirebaseMessaging } from '@firebase/messaging-types';
 import { FirebaseService } from '@firebase/app-types/private';
+import { isConsoleMessage } from '../helpers/is-console-message';
 
 export class WindowController implements FirebaseMessaging, FirebaseService {
   private vapidKey: string | null = null;
   private swRegistration?: ServiceWorkerRegistration;
-  private onMessageCallback: NextFn<object> | null = null;
+  private onMessageCallback: NextFn<object> | Observer<object> | null = null;
 
   constructor(
     private readonly firebaseDependencies: FirebaseInternalDependencies
@@ -131,12 +132,8 @@ export class WindowController implements FirebaseMessaging, FirebaseService {
    * message.
    * @return The unsubscribe function for the observer.
    */
-  // TODO: Simplify this to only accept a function and not an Observer.
   onMessage(nextOrObserver: NextFn<object> | Observer<object>): Unsubscribe {
-    this.onMessageCallback =
-      typeof nextOrObserver === 'function'
-        ? nextOrObserver
-        : nextOrObserver.next;
+    this.onMessageCallback = nextOrObserver;
 
     return () => {
       this.onMessageCallback = null;
@@ -193,8 +190,13 @@ export class WindowController implements FirebaseMessaging, FirebaseService {
 
     const { type, payload } = (event.data as InternalMessage).firebaseMessaging;
 
+    // onMessageCallback is either a function or observer/subscriber.
     if (this.onMessageCallback && type === MessageType.PUSH_RECEIVED) {
-      this.onMessageCallback(payload);
+      if (typeof this.onMessageCallback === 'function') {
+        this.onMessageCallback(payload);
+      } else {
+        this.onMessageCallback.next(payload);
+      }
     }
 
     const { data } = payload;