Address Glean reviewer feedback

 * ensure uload enabled setting is respected before the initialization.
 * Include test to demonstrate this.
 * change the emphasis of Feature Flag so we can deprecate the legacy telemetry service.
 * Move telemetry service registeration dispatch events to `injectContext` so as to be switchable with Feature Flag.

app/src/main/java/mozilla/lockbox/LockboxApplication.kt

@@ -100,15 +100,16 @@ open class LockboxApplication : Application() {
     }
 
     private fun injectContext() {
-        val contextStoreList: List<ContextStore> = listOf(
+        val contextStoreList: List<ContextStore> = listOfNotNull(
             FingerprintStore.shared,
             SettingStore.shared,
             SecurePreferences.shared,
             ClipboardStore.shared,
             NetworkStore.shared,
             TimingSupport.shared,
             AccountStore.shared,
-            if (FeatureFlags.USE_GLEAN) GleanTelemetryStore.shared else TelemetryStore.shared,
+            if (FeatureFlags.INCLUDE_DEPRECATED_TELEMETRY) TelemetryStore.shared else null,
+            GleanTelemetryStore.shared,
             SentryStore.shared,
             PublicSuffixSupport.shared
         )

app/src/main/java/mozilla/lockbox/LockboxAutofillService.kt

@@ -30,9 +30,9 @@ import mozilla.lockbox.extensions.filterNotNull
 import mozilla.lockbox.flux.Dispatcher
 import mozilla.lockbox.store.AccountStore
 import mozilla.lockbox.store.AutofillStore
-import mozilla.lockbox.store.ContextStore
 import mozilla.lockbox.store.DataStore
 import mozilla.lockbox.store.GleanTelemetryStore
+import mozilla.lockbox.store.SettingStore
 import mozilla.lockbox.store.TelemetryStore
 import mozilla.lockbox.support.FxASyncDataStoreSupport
 import mozilla.lockbox.support.Constant
@@ -47,8 +47,10 @@ import mozilla.lockbox.support.isDebug
 class LockboxAutofillService(
     private val accountStore: AccountStore = AccountStore.shared,
     private val dataStore: DataStore = DataStore.shared,
+    private val settingStore: SettingStore = SettingStore.shared,
     private val securePreferences: SecurePreferences = SecurePreferences.shared,
     private val fxaSupport: FxASyncDataStoreSupport = FxASyncDataStoreSupport.shared,
+    private val gleanTelemetryStore: GleanTelemetryStore = GleanTelemetryStore.shared,
     private val autofillStore: AutofillStore = AutofillStore.shared,
     val dispatcher: Dispatcher = Dispatcher.shared
 ) : AutofillService() {
@@ -150,10 +152,10 @@ class LockboxAutofillService(
     private fun intializeService() {
         isRunning = true
 
-        val telemetryStore: ContextStore = if (FeatureFlags.USE_GLEAN) GleanTelemetryStore.shared else TelemetryStore.shared
-
-        val contextInjectables = arrayOf(
-            telemetryStore,
+        val contextInjectables = listOfNotNull(
+            settingStore,
+            gleanTelemetryStore,
+            if (FeatureFlags.INCLUDE_DEPRECATED_TELEMETRY) TelemetryStore.shared else null,
             securePreferences,
             accountStore,
             fxaSupport

app/src/main/java/mozilla/lockbox/store/GleanTelemetryStore.kt

@@ -37,12 +37,11 @@ class GleanTelemetryStore(
     private val compositeDisposable = CompositeDisposable()
 
     override fun injectContext(context: Context) {
-        gleanWrapper.initialize(context, BuildConfig.BUILD_TYPE)
-
         settingStore.sendUsageData
             .subscribe {
                 gleanWrapper.uploadEnabled = it
             }
             .addTo(compositeDisposable)
+        gleanWrapper.initialize(context, BuildConfig.BUILD_TYPE)
     }
 }

app/src/main/java/mozilla/lockbox/store/TelemetryStore.kt

@@ -12,6 +12,7 @@
 package mozilla.lockbox.store
 
 import android.content.Context
+import androidx.annotation.VisibleForTesting
 import io.reactivex.disposables.CompositeDisposable
 import io.reactivex.rxkotlin.addTo
 import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient
@@ -91,7 +92,8 @@ open class TelemetryStore(
 
     internal val compositeDisposable = CompositeDisposable()
 
-    init {
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    fun register() {
         dispatcher.register
             .filterByType(TelemetryAction::class.java)
             .subscribe {
@@ -108,6 +110,7 @@ open class TelemetryStore(
     }
 
     override fun injectContext(context: Context) {
+        register()
         wrapper.lateinitContext(context)
         settingStore
             .sendUsageData

app/src/main/java/mozilla/lockbox/support/FeatureFlags.kt

@@ -49,17 +49,18 @@ object FeatureFlags {
     }
 
     /**
-     * Use the Glean telemetry store.
+     * Include the legacy telemetry service. As part of our migration to the Glean, we will want
+     * to keep the legacy service running in parallel.
      *
-     * If false, the legacy telemetry-service is used.
-     * If true, the glean telemetry service is used.
+     * If true, the legacy telemetry-service is used.
+     * If false, the legacy telemetry-service is not.
      *
      * Either way, the user can opt out of this from the settings.
      */
-    val USE_GLEAN = when {
+    val INCLUDE_DEPRECATED_TELEMETRY = when {
         isDebug -> true
-        isTesting -> false
-        isRelease -> false
-        else -> false
+        isTesting -> true
+        isRelease -> true
+        else -> true
     }
 }

app/src/test/java/mozilla/lockbox/store/GleanTelemetryStoreTest.kt

@@ -7,14 +7,21 @@
 package mozilla.lockbox.store
 
 import android.content.Context
+import android.content.SharedPreferences
+import android.view.autofill.AutofillManager
 import androidx.test.core.app.ApplicationProvider
 import io.reactivex.subjects.ReplaySubject
 import mozilla.lockbox.flux.Dispatcher
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.anyBoolean
+import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.ArgumentMatchers.anyString
+import org.mockito.ArgumentMatchers.eq
 import org.mockito.Mock
 import org.powermock.api.mockito.PowerMockito.`when`
 import org.powermock.api.mockito.PowerMockito.mock
@@ -65,4 +72,65 @@ class GleanTelemetryStoreTest {
         sendUsageDataStub.onNext(true)
         assertTrue(telemetryWrapper.uploadEnabled)
     }
+
+    @Test
+    fun `ensure upload enabled is called before initialize`() {
+        // We spend quite a lot of effort here to convince ourselves that the user's preference
+        // for sending usage data is respected before initializing glean.
+        // If this test fails, then either we're losing ping data or we're uploading ping data
+        // when the user has explicitly said not to.
+
+        // mock all this out for the setting store, so we can use the Rx machinery it uses.
+        val context = mock(Context::class.java)
+        `when`(context.getSystemService(eq(AutofillManager::class.java))).thenReturn(mock(AutofillManager::class.java))
+        val prefs = mock(SharedPreferences::class.java)
+        `when`(prefs.contains(eq(SettingStore.Keys.DEVICE_SECURITY_PRESENT))).thenReturn(true)
+        `when`(prefs.contains(eq(SettingStore.Keys.SEND_USAGE_DATA))).thenReturn(true)
+        `when`(context.getSharedPreferences(anyString(), anyInt())).thenReturn(prefs)
+
+        val fingerprintStore = mock(FingerprintStore::class.java)
+        `when`(fingerprintStore.isDeviceSecure).thenReturn(true)
+
+        class DummyGleanWrapper : GleanWrapper() {
+            var initializationOrder = arrayListOf<String>()
+
+            override var uploadEnabled: Boolean = false
+                set(value) {
+                    initializationOrder.add("uploadEnabled")
+                    field = value
+                }
+
+            override fun initialize(context: Context, channel: String) {
+                initializationOrder.add("initialize")
+            }
+        }
+
+        fun testWithPref(enabled: Boolean) {
+            `when`(
+                prefs.getBoolean(
+                    eq(SettingStore.Keys.SEND_USAGE_DATA),
+                    anyBoolean()
+                )
+            ).thenReturn(enabled)
+
+            val telemetryWrapper = DummyGleanWrapper()
+
+            val settingStore = SettingStore(fingerprintStore = fingerprintStore)
+            val gleanTelemetryStore = GleanTelemetryStore(telemetryWrapper, settingStore)
+
+            // These should appear in the same order as they do in `injectContext` in
+            // `LockwiseApplication` and `initializeService` in `LockwiseAutofillService`.
+            settingStore.injectContext(context)
+            gleanTelemetryStore.injectContext(context)
+
+            assertEquals(
+                "uploadEnabled, initialize",
+                telemetryWrapper.initializationOrder.joinToString(", ")
+            )
+            assertEquals(enabled, telemetryWrapper.uploadEnabled)
+        }
+
+        testWithPref(false)
+        testWithPref(true)
+    }
 }

app/src/test/java/mozilla/lockbox/store/TelemetryStoreTest.kt

@@ -88,6 +88,7 @@ class TelemetryStoreTest : DisposingTest() {
         val uploadObserver = createTestObserver<Int>()
         wrapper.eventsSubject.subscribe(eventsObserver)
         wrapper.uploadSubject.subscribe(uploadObserver)
+        subject.register()
 
         var action: TelemetryAction = LifecycleAction.Foreground
         dispatcher.dispatch(action)