Delay initialization of Push

[jonalmeida]

Perf folks: before working on this issue, check in with Jonathan on the status: this issue may be worked on by Jonathan and it may also be blocked on other changes.

Priority: 5-10% of startup spent here. Last big chunk of perf wins before jumping into view inflation.

---

Some thoughts from a conversation with @hawkinsw from reviewing #6712.

Since the consumers for Push are FxaAccountManager and Engine, we were initializing push immediately since that was required by the Firebase SDK.

What Will rightfully noticed, is that maybe we can delay this initialization to be done much later since the consumers are not lazily initialized through UI interactions or activity creation.

This needs investigations (mostly from me) before we do this:

• Where can be move to be initialized to that isn't immediately on startup, but soon enough to receive push messages on "startup".
• Can we safely do this since the black box is the Firebase SDK which assumes initialization is always done in Application#onCreate.
• Will we cause any race conditions from our initialization, i.e. Push init'd and receives a message before our consumers have init'd.

┆Issue is synchronized with this Jira Task

[mcomella]

Triage: placed in the waiting column because Jonathan is working on this.

[jonalmeida]

To keep a record for the ticket:

From what I understand, FxaAccountManager is a heavy start-up which happens because of push.

We experimented with moving Push to a separate component class that lazy init's FxaAccountManager when needed, but this had one hitch in that it broke the feature since FxA needs to register for push subscriptions the first time, before push starts to notify FxA for updates.

Diff of a test patch

diff --git a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
index ce8e8682..2f285952 100644
--- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
@@ -11,19 +11,14 @@ import androidx.annotation.VisibleForTesting.PRIVATE
 import androidx.lifecycle.ProcessLifecycleOwner
 import mozilla.components.browser.storage.sync.PlacesBookmarksStorage
 import mozilla.components.browser.storage.sync.PlacesHistoryStorage
-import mozilla.components.concept.push.Bus
 import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.DeviceCapability
 import mozilla.components.concept.sync.DeviceEvent
 import mozilla.components.concept.sync.DeviceEventsObserver
-import mozilla.components.concept.sync.DevicePushSubscription
 import mozilla.components.concept.sync.DeviceType
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.feature.push.AutoPushFeature
-import mozilla.components.feature.push.AutoPushSubscription
-import mozilla.components.feature.push.PushConfig
-import mozilla.components.feature.push.PushSubscriptionObserver
 import mozilla.components.feature.push.PushType
 import mozilla.components.lib.crash.CrashReporter
 import mozilla.components.lib.dataprotect.SecureAbove22Preferences
@@ -57,7 +52,8 @@ class BackgroundServices(
     historyStorage: PlacesHistoryStorage,
     bookmarkStorage: PlacesBookmarksStorage,
     passwordsStorage: SyncableLoginsStore,
-    secureAbove22Preferences: SecureAbove22Preferences
+    secureAbove22Preferences: SecureAbove22Preferences,
+    private val push: Push
 ) {
     // // A malformed string is causing crashes.
     // This will be removed when the string is fixed. See #5552
@@ -96,10 +92,6 @@ class BackgroundServices(
             syncPeriodInMinutes = 240L) // four hours
     }
 
-    private val pushService by lazy { FirebasePush() }
-
-    val push by lazy { makePushConfig()?.let { makePush(it) } }
-
     init {
         // Make the "history", "bookmark", and "passwords" stores accessible to workers spawned by the sync manager.
         GlobalSyncableStoreProvider.configureStore(SyncEngine.History to historyStorage)
@@ -125,34 +117,10 @@ class BackgroundServices(
 
     val accountAbnormalities = AccountAbnormalities(context, crashReporter)
 
-    private val pushAccountObserver by lazy { push?.let { PushAccountObserver(it) } }
+    private val pushAccountObserver by lazy { PushAccountObserver(lazy { push.pushFeature }) }
 
     val accountManager = makeAccountManager(context, serverConfig, deviceConfig, syncConfig)
 
-    @VisibleForTesting(otherwise = PRIVATE)
-    fun makePush(pushConfig: PushConfig): AutoPushFeature {
-        return AutoPushFeature(
-            context = context,
-            service = pushService,
-            config = pushConfig
-        )
-    }
-
-    @VisibleForTesting(otherwise = PRIVATE)
-    fun makePushConfig(): PushConfig? {
-        val logger = Logger("PushConfig")
-        val projectIdKey = context.getString(R.string.pref_key_push_project_id)
-        val resId = context.resources.getIdentifier(projectIdKey, "string", context.packageName)
-        if (resId == 0) {
-            logger.warn("No firebase configuration found; cannot support push service.")
-            return null
-        }
-
-        logger.debug("Creating push configuration for autopush.")
-        val projectId = context.resources.getString(resId)
-        return PushConfig(projectId)
-    }
-
     @VisibleForTesting(otherwise = PRIVATE)
     fun makeAccountManager(
         context: Context,
@@ -189,39 +157,9 @@ class BackgroundServices(
         accountManager.register(accountAbnormalities)
 
         // Enable push if it's configured.
-        push?.let { autoPushFeature ->
+        push.config?.let {
             // Register the push account observer so we know how to update our push subscriptions.
-            accountManager.register(pushAccountObserver!!)
-
-            val logger = Logger("AutoPushFeature")
-
-            // Notify observers for Services' messages.
-            autoPushFeature.registerForPushMessages(
-                PushType.Services,
-                object : Bus.Observer<PushType, String> {
-                    override fun onEvent(type: PushType, message: String) {
-                        accountManager.authenticatedAccount()?.deviceConstellation()
-                            ?.processRawEventAsync(message)
-                    }
-                })
-
-            // Notify observers for subscription changes.
-            autoPushFeature.registerForSubscriptions(object : PushSubscriptionObserver {
-                override fun onSubscriptionAvailable(subscription: AutoPushSubscription) {
-                    // Update for only the services subscription.
-                    if (subscription.type == PushType.Services) {
-                        logger.info("New push subscription received for FxA")
-                        accountManager.authenticatedAccount()?.deviceConstellation()
-                            ?.setDevicePushSubscriptionAsync(
-                                DevicePushSubscription(
-                                    endpoint = subscription.endpoint,
-                                    publicKey = subscription.publicKey,
-                                    authKey = subscription.authKey
-                                )
-                            )
-                    }
-                }
-            })
+            accountManager.register(pushAccountObserver)
         }
         accountAbnormalities.accountManagerInitializedAsync(
             accountManager,
@@ -280,30 +218,3 @@ class TelemetryAccountObserver(
         context.settings().fxaSignedIn = false
     }
 }
-
-/**
- * When we login/logout of FxA, we need to update our push subscriptions to match the newly
- * logged in account.
- *
- * We added the push service to the AccountManager observer so that we can control when the
- * service will start/stop. Firebase was added when landing the push service to ensure it works
- * as expected without causing any (as many) side effects.
- *
- * In order to use Firebase with Leanplum and other marketing features, we need it always
- * running so we cannot leave this code in place when we implement those features.
- *
- * We should have this removed when we are more confident
- * of the send-tab/push feature: https://github.com/mozilla-mobile/fenix/issues/4063
- */
-@VisibleForTesting(otherwise = PRIVATE)
-class PushAccountObserver(private val push: AutoPushFeature) : AccountObserver {
-    override fun onLoggedOut() {
-        push.unsubscribeForType(PushType.Services)
-    }
-
-    override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
-        if (authType != AuthType.Existing) {
-            push.subscribeForType(PushType.Services)
-        }
-    }
-}
diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt
index 5fb17e15..1f827ed2 100644
--- a/app/src/main/java/org/mozilla/fenix/components/Components.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt
@@ -21,7 +21,8 @@ class Components(private val context: Context) {
             core.historyStorage,
             core.bookmarksStorage,
             core.passwordsStorage,
-            core.getSecureAbove22Preferences()
+            core.getSecureAbove22Preferences(),
+            push
         )
     }
     val services by lazy { Services(context, backgroundServices.accountManager) }
@@ -47,6 +48,7 @@ class Components(private val context: Context) {
             core.customTabsStore
         )
     }
+    val push by lazy { Push(context, lazy { backgroundServices.accountManager }) }
     val analytics by lazy { Analytics(context) }
     val publicSuffixList by lazy { PublicSuffixList(context) }
     val clipboardHandler by lazy { ClipboardHandler(context) }
diff --git a/app/src/main/java/org/mozilla/fenix/components/Push.kt b/app/src/main/java/org/mozilla/fenix/components/Push.kt
new file mode 100644
index 00000000..292ab1d5
--- /dev/null
+++ b/app/src/main/java/org/mozilla/fenix/components/Push.kt
@@ -0,0 +1,106 @@
+package org.mozilla.fenix.components
+
+import android.content.Context
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.VisibleForTesting.PRIVATE
+import mozilla.components.concept.push.Bus
+import mozilla.components.concept.sync.AccountObserver
+import mozilla.components.concept.sync.AuthType
+import mozilla.components.concept.sync.DeviceEvent
+import mozilla.components.concept.sync.DevicePushSubscription
+import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.feature.push.AutoPushFeature
+import mozilla.components.feature.push.AutoPushSubscription
+import mozilla.components.feature.push.PushConfig
+import mozilla.components.feature.push.PushSubscriptionObserver
+import mozilla.components.feature.push.PushType
+import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.support.base.log.logger.Logger
+import org.mozilla.fenix.R
+
+class Push(val context: Context, val accountManager: Lazy<FxaAccountManager>) {
+
+    val logger = Logger("Push")
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    val config: PushConfig? by lazy {
+        val logger = Logger("PushConfig")
+        val projectIdKey = context.getString(R.string.pref_key_push_project_id)
+        val resId = context.resources.getIdentifier(projectIdKey, "string", context.packageName)
+        if (resId == 0) {
+            logger.warn("No firebase configuration found; cannot support push service.")
+            return@lazy null
+        }
+
+        logger.debug("Creating push configuration for autopush.")
+        val projectId = context.resources.getString(resId)
+        PushConfig(projectId)
+    }
+
+    val pushFeature by lazy {
+        config?.let {
+            AutoPushFeature(
+                context = context,
+                service = service,
+                config = it
+            ).also { feature ->
+                // Notify observers for Services' messages.
+                feature.registerForPushMessages(
+                    PushType.Services,
+                    object : Bus.Observer<PushType, String> {
+                        override fun onEvent(type: PushType, message: String) {
+                            accountManager.value.authenticatedAccount()?.deviceConstellation()
+                                ?.processRawEventAsync(message)
+                        }
+                    })
+
+                // Notify observers for subscription changes.
+                feature.registerForSubscriptions(object : PushSubscriptionObserver {
+                    override fun onSubscriptionAvailable(subscription: AutoPushSubscription) {
+                        // Update for only the services subscription.
+                        if (subscription.type == PushType.Services) {
+                            logger.info("New push subscription received for FxA")
+                            accountManager.value.authenticatedAccount()?.deviceConstellation()
+                                ?.setDevicePushSubscriptionAsync(
+                                    DevicePushSubscription(
+                                        endpoint = subscription.endpoint,
+                                        publicKey = subscription.publicKey,
+                                        authKey = subscription.authKey
+                                    )
+                                )
+                        }
+                    }
+                })
+            }
+        }
+    }
+
+    private val service by lazy { FirebasePush() }
+}
+
+/**
+ * When we login/logout of FxA, we need to update our push subscriptions to match the newly
+ * logged in account.
+ *
+ * We added the push service to the AccountManager observer so that we can control when the
+ * service will start/stop. Firebase was added when landing the push service to ensure it works
+ * as expected without causing any (as many) side effects.
+ *
+ * In order to use Firebase with Leanplum and other marketing features, we need it always
+ * running so we cannot leave this code in place when we implement those features.
+ *
+ * We should have this removed when we are more confident
+ * of the send-tab/push feature: https://github.com/mozilla-mobile/fenix/issues/4063
+ */
+@VisibleForTesting(otherwise = PRIVATE)
+class PushAccountObserver(private val push: Lazy<AutoPushFeature?>) : AccountObserver {
+    override fun onLoggedOut() {
+        push.value?.unsubscribeForType(PushType.Services)
+    }
+
+    override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
+        if (authType != AuthType.Existing) {
+            push.value?.subscribeForType(PushType.Services)
+        }
+    }
+}

What I've been meaning to experiment with:

1. Remove the push initialization in the application class.
2. Execute it on the first access of the feature.

val pushService by lazy {
  FirebasePush()
}

val pushFeature by lazy {
  AutoPushFeature(pushService).also { push ->
    // Only happens once on the first access.
    push.initialize()
  }
}

val accountManager by lazy {
  FxaAccountManager().also {

    // This is where push is needed, so our first initialize might be here.
    FxaPushSupportFeature(push).init()
  }
}

// Future
val engine by lazy {
  GeckoEngine().also {
    // This is where push is needed, so our first initialize might be here.
    it.setWebPushDelegate(push)
  }
}

For LeanPlum integration, that needs the PushService (FirebasePush), so we can initialize the push service and provide it to LP without init'ing the push feature and therefore FxA.

What this gives us is that we don't have to initialize fxa via push, but instead the first use of fxa (or GV for that matter) will initialize push and if the PushService hasn't been init'd by LeanPlum yet, we can do that then as well.

This also means we don't have to necessarily move Push to a separate component class for perf, but we really should either way because BackgroundServices is too chunky.

[jonalmeida]

@mcomella we have a patch that allows push to receive messages on startup and will lazily initialize FxaAccountManager if a message is delivered for it and it hasn't already been init'd.

We're going to land this first in Reference Browser as that has FxA push support and GV WebPush there together. If all looks good (and so far it does :), we can use the same patch for Fenix.

The patch for reference: jonalmeida/reference-browser@245d635

[mcomella]

@jonalmeida Does this need a QA label?

[jonalmeida]

Probably not since QA can't test perf fixes.