For #5182: Loading experiments on startup is slow, remove Fretboard (#7510)

This removes Fretboard. The goal is to reduce cold startup costs associated with loading the experiments on the main thread. We currently have two experiments frameworks in use and should only require one.
2020-01-13 12:38:32 -06:00 · 2020-01-13 12:38:32 -06:00 · 7baf54f566
parent 399df17062
commit 7baf54f566
15 changed files with 29 additions and 225 deletions
--- a/app/build.gradle
+++ b/app/build.gradle
@ -420,7 +420,6 @@ dependencies {

    implementation Deps.mozilla_service_sync_logins
    implementation Deps.mozilla_service_firefox_accounts
-    implementation Deps.mozilla_service_fretboard
    implementation Deps.mozilla_service_glean
    implementation Deps.mozilla_service_experiments

--- a/app/src/androidTest/java/org/mozilla/fenix/Experiments.kt
+++ b/app/src/androidTest/java/org/mozilla/fenix/Experiments.kt
@ -1,33 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.fenix
-
-import android.content.Context
-import mozilla.components.service.fretboard.ExperimentDescriptor
-
-const val EXPERIMENTS_JSON_FILENAME = "experiments.json"
-const val EXPERIMENTS_BASE_URL = "https://firefox.settings.services.mozilla.com/v1"
-const val EXPERIMENTS_BUCKET_NAME = "main"
-// collection name below, see https://bugzilla.mozilla.org/show_bug.cgi?id=1523395 for ownership details
-const val EXPERIMENTS_COLLECTION_NAME = "fenix-experiments"
-
-object Experiments {
-    val AATestDescriptor = ExperimentDescriptor("AAtest")
-    // application services flag to disable the Firefox Sync syncManager
-    val asFeatureSyncDisabled = ExperimentDescriptor("asFeatureSyncDisabled")
-    // application services flag to disable Firefox Accounts pairing button.
-    val asFeatureFxAPairingDisabled = ExperimentDescriptor("asFeatureFxAPairingDisabled")
-    // application services flag to disable Firefox Accounts WebChannel integration.
-    val asFeatureWebChannelsDisabled = ExperimentDescriptor("asFeatureWebChannelsDisabled")
-}
-
-val Context.app: FenixApplication
-    get() = applicationContext as FenixApplication
-
-fun Context.isInExperiment(descriptor: ExperimentDescriptor): Boolean =
-    if (descriptor.name == "asFeatureWebChannelsDisabled")
-        true
-    else
-        app.fretboard.isInExperiment(this, descriptor)
--- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt
+++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt
@ -28,7 +28,8 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor {
        var result: RequestInterceptor.InterceptionResponse? = null

        // WebChannel-driven authentication does not require a separate redirect interceptor.
-        if (context.isInExperiment(Experiments.asFeatureWebChannelsDisabled)) {
+        @Suppress("ConstantConditionIf")
+        if (FeatureFlags.asFeatureWebChannelsDisabled) {
            result = context.components.services.accountsAuthFeature.interceptor.onLoadRequest(
                    engineSession, uri, hasUserGesture, isSameDomain)
        }
--- a/app/src/main/java/org/mozilla/fenix/Experiments.kt
+++ b/app/src/main/java/org/mozilla/fenix/Experiments.kt
@ -1,30 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.fenix
-
-import android.content.Context
-import mozilla.components.service.fretboard.ExperimentDescriptor
-
-const val EXPERIMENTS_JSON_FILENAME = "experiments.json"
-const val EXPERIMENTS_BASE_URL = "https://firefox.settings.services.mozilla.com/v1"
-const val EXPERIMENTS_BUCKET_NAME = "main"
-// collection name below, see https://bugzilla.mozilla.org/show_bug.cgi?id=1523395 for ownership details
-const val EXPERIMENTS_COLLECTION_NAME = "fenix-experiments"
-
-object Experiments {
-    val AATestDescriptor = ExperimentDescriptor("AAtest")
-    // application services flag to disable the Firefox Sync syncManager
-    val asFeatureSyncDisabled = ExperimentDescriptor("asFeatureSyncDisabled")
-    // application services flag to disable Firefox Accounts pairing button.
-    val asFeatureFxAPairingDisabled = ExperimentDescriptor("asFeatureFxAPairingDisabled")
-    // application services flag to disable Firefox Accounts WebChannel integration.
-    val asFeatureWebChannelsDisabled = ExperimentDescriptor("asFeatureWebChannelsDisabled")
-}
-
-val Context.app: FenixApplication
-    get() = applicationContext as FenixApplication
-
-fun Context.isInExperiment(descriptor: ExperimentDescriptor): Boolean =
-    app.fretboard.isInExperiment(this, descriptor)
--- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
+++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
@ -38,4 +38,19 @@ object FeatureFlags {
     * Gives option in Settings to see logins and sync logins
     */
    val logins = Config.channel.isNightlyOrDebug
+
+    /**
+     * Disables FxA Application Services Web Channels feature
+     */
+    const val asFeatureWebChannelsDisabled = false
+
+    /**
+     * Disables FxA Application Services Sync feature
+     */
+    const val asFeatureSyncDisabled = false
+
+    /**
+     * Disables FxA Application Services Pairing feature
+     */
+    const val asFeatureFxAPairingDisabled = false
 }
--- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt
+++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt
@ -20,9 +20,6 @@ import kotlinx.coroutines.runBlocking
 import mozilla.appservices.Megazord
 import mozilla.components.concept.push.PushProcessor
 import mozilla.components.service.experiments.Experiments
-import mozilla.components.service.fretboard.Fretboard
-import mozilla.components.service.fretboard.source.kinto.KintoExperimentSource
-import mozilla.components.service.fretboard.storage.flatfile.FlatFileExperimentStorage
 import mozilla.components.support.base.log.Log
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.log.sink.AndroidLogSink
@ -36,13 +33,10 @@ import org.mozilla.fenix.components.Components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.session.NotificationSessionObserver
 import org.mozilla.fenix.session.VisibilityLifecycleCallback
-import java.io.File

@SuppressLint("Registered")
@Suppress("TooManyFunctions")
 open class FenixApplication : LocaleAwareApplication() {
-    lateinit var fretboard: Fretboard
-    lateinit var experimentLoader: Deferred<Boolean>

    open val components by lazy { Components(this) }

@ -103,10 +97,6 @@ open class FenixApplication : LocaleAwareApplication() {
            }
        }

-        // We want to call this function as early as possible, but only once and
-        // on the main process, as it uses Gecko to fetch experiments from the server.
-        experimentLoader = loadExperiments()
-
        // When the `fenix-test-2019-08-05` experiment is active, record its branch in Glean
        // telemetry. This will be used to validate that the experiment system correctly enrolls
        // clients and segments them into branches. Note that this will not take effect the first
@ -157,26 +147,6 @@ open class FenixApplication : LocaleAwareApplication() {
        // no-op, LeakCanary is disabled by default
    }

-    private fun loadExperiments(): Deferred<Boolean> {
-        val experimentsFile = File(filesDir, EXPERIMENTS_JSON_FILENAME)
-        val experimentSource = KintoExperimentSource(
-            EXPERIMENTS_BASE_URL,
-            EXPERIMENTS_BUCKET_NAME,
-            EXPERIMENTS_COLLECTION_NAME,
-            components.core.client
-        )
-        // TODO add ValueProvider to keep clientID in sync with Glean when ready
-        fretboard = Fretboard(experimentSource, FlatFileExperimentStorage(experimentsFile))
-
-        return GlobalScope.async(Dispatchers.IO) {
-            fretboard.loadExperiments()
-            Logger.debug("Bucket is ${fretboard.getUserBucket(this@FenixApplication)}")
-            Logger.debug("Experiments active: ${fretboard.getExperimentsMap(this@FenixApplication)}")
-            fretboard.updateExperiments()
-            return@async true
-        }
-    }
-
    private fun setupPush() {
        // Sets the PushFeature as the singleton instance for push messages to go to.
        // We need the push feature setup here to deliver messages in the case where the service
@ -290,8 +260,4 @@ open class FenixApplication : LocaleAwareApplication() {
            StrictMode.setVmPolicy(builder.build())
        }
    }
-
-    companion object {
-        private const val ONE_DAY_MILLIS = 24 * 60 * 60 * 1000
-    }
 }
--- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
@ -56,7 +56,6 @@ import mozilla.components.support.base.feature.PermissionsFeature
 import mozilla.components.support.base.feature.UserInteractionHandler
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.ktx.android.view.exitImmersiveModeIfNeeded
-import org.mozilla.fenix.Experiments
 import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.IntentReceiverActivity
@ -85,7 +84,6 @@ import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.sessionsOfType
 import org.mozilla.fenix.ext.settings
-import org.mozilla.fenix.isInExperiment
 import org.mozilla.fenix.settings.SupportUtils
 import org.mozilla.fenix.theme.ThemeManager

@ -436,7 +434,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
                view.swipeRefresh.setOnChildScrollUpCallback { _, _ -> true }
            }

-            if (!requireContext().isInExperiment(Experiments.asFeatureWebChannelsDisabled)) {
+            @Suppress("ConstantConditionIf")
+            if (FeatureFlags.asFeatureWebChannelsDisabled) {
                webchannelIntegration.set(
                    feature = FxaWebChannelFeature(
                        requireContext(),
--- a/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/BackgroundServices.kt
@ -32,13 +32,12 @@ import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider
 import mozilla.components.service.sync.logins.SyncableLoginsStore
 import mozilla.components.support.base.log.logger.Logger
 import org.mozilla.fenix.Config
-import org.mozilla.fenix.Experiments
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
-import org.mozilla.fenix.isInExperiment
 import org.mozilla.fenix.test.Mockable

 /**
@ -78,8 +77,9 @@ class BackgroundServices(
        secureStateAtRest = Config.channel.isNightlyOrDebug
    )
    // If sync has been turned off on the server then disable syncing.
+    @Suppress("ConstantConditionIf")
    @VisibleForTesting(otherwise = PRIVATE)
-    val syncConfig = if (context.isInExperiment(Experiments.asFeatureSyncDisabled)) {
+    val syncConfig = if (FeatureFlags.asFeatureSyncDisabled) {
        null
    } else {
        SyncConfig(
--- a/app/src/main/java/org/mozilla/fenix/components/FxaServer.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/FxaServer.kt
@ -5,8 +5,7 @@ package org.mozilla.fenix.components

 import android.content.Context
 import mozilla.components.service.fxa.ServerConfig
-import org.mozilla.fenix.Experiments
-import org.mozilla.fenix.isInExperiment
+import org.mozilla.fenix.FeatureFlags

 /**
 * Utility to configure Firefox Account servers.
@ -16,7 +15,8 @@ object FxaServer {
    const val CLIENT_ID = "a2270f727f45f648"
    const val REDIRECT_URL = "https://accounts.firefox.com/oauth/success/$CLIENT_ID"

-    fun redirectUrl(context: Context) = if (context.isInExperiment(Experiments.asFeatureWebChannelsDisabled)) {
+    @Suppress("ConstantConditionIf", "UNUSED_PARAMETER")
+    fun redirectUrl(context: Context) = if (FeatureFlags.asFeatureWebChannelsDisabled) {
        REDIRECT_URL
    } else {
        "urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel"
--- a/app/src/main/java/org/mozilla/fenix/components/Services.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/Services.kt
@ -14,13 +14,12 @@ import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature
 import mozilla.components.feature.app.links.AppLinksInterceptor
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.ktx.android.content.hasCamera
-import org.mozilla.fenix.Experiments
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.NavGraphDirections
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.getPreferenceKey
-import org.mozilla.fenix.isInExperiment
 import org.mozilla.fenix.settings.SupportUtils
 import org.mozilla.fenix.test.Mockable

@ -65,8 +64,7 @@ class Services(
     */
    fun launchPairingSignIn(context: Context, navController: NavController) {
        // Do not navigate to pairing UI if camera not available or pairing is disabled
-        if (context.hasCamera() && !context.isInExperiment(Experiments.asFeatureFxAPairingDisabled)
-        ) {
+        if (context.hasCamera() && !FeatureFlags.asFeatureFxAPairingDisabled) {
            val directions = NavGraphDirections.actionGlobalTurnOnSync()
            navController.navigate(directions)
        } else {
--- a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
@ -145,7 +145,6 @@ class SearchFragment : Fragment(), UserInteractionHandler {
        super.onViewCreated(view, savedInstanceState)

        search_scan_button.visibility = if (context?.hasCamera() == true) View.VISIBLE else View.GONE
-        layoutComponents(view.search_layout)

        qrFeature.set(
            QrFeature(
--- a/app/src/main/java/org/mozilla/fenix/search/SearchLayouts.kt
+++ b/app/src/main/java/org/mozilla/fenix/search/SearchLayouts.kt
@ -1,73 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package org.mozilla.fenix.search
-
-import androidx.constraintlayout.widget.ConstraintLayout
-import androidx.constraintlayout.widget.ConstraintLayout.LayoutParams.PARENT_ID
-import androidx.constraintlayout.widget.ConstraintLayout.LayoutParams.TOP
-import androidx.constraintlayout.widget.ConstraintLayout.LayoutParams.BOTTOM
-import androidx.constraintlayout.widget.ConstraintSet
-import androidx.recyclerview.widget.LinearLayoutManager
-import kotlinx.android.synthetic.main.component_awesomebar.*
-import kotlinx.android.synthetic.main.fragment_search.*
-import mozilla.components.support.base.log.logger.Logger
-import org.mozilla.fenix.Experiments.AATestDescriptor
-import org.mozilla.fenix.isInExperiment
-
-internal fun SearchFragment.layoutComponents(layout: ConstraintLayout) {
-    context?.let {
-        when {
-            it.isInExperiment(AATestDescriptor) -> {
-                setInExperimentConstraints(layout)
-            }
-            else -> {
-                setOutOfExperimentConstraints(layout)
-            }
-        }
-    } // we're unattached if context is null
-}
-
-internal fun SearchFragment.setInExperimentConstraints(layout: ConstraintLayout) {
-    Logger.debug("Loading in experiment constraints")
-
-    ConstraintSet().apply {
-        clone(layout)
-
-        // Move the search bar to the bottom of the layout
-        clear(toolbar_wrapper.id, TOP)
-        connect(toolbar_wrapper.id, BOTTOM, pill_wrapper.id, TOP)
-
-        connect(awesomeBar.id, TOP, PARENT_ID, TOP)
-        connect(awesomeBar.id, BOTTOM, toolbar_wrapper.id, TOP)
-        (awesomeBar.layoutManager as? LinearLayoutManager)?.reverseLayout = true
-
-        connect(pill_wrapper.id, BOTTOM, PARENT_ID, BOTTOM)
-
-        applyTo(layout)
-    }
-}
-
-internal fun SearchFragment.setOutOfExperimentConstraints(layout: ConstraintLayout) {
-    Logger.debug("Loading out of experiment constraints")
-
-    ConstraintSet().apply {
-        clone(layout)
-
-        // Move the search bar to the top of the layout
-        connect(toolbar_wrapper.id, TOP, PARENT_ID, TOP)
-        clear(toolbar_wrapper.id, BOTTOM)
-
-        connect(fill_link_from_clipboard.id, TOP, toolbar_wrapper.id, BOTTOM)
-
-        clear(awesomeBar.id, TOP)
-        connect(awesomeBar.id, TOP, search_with_shortcuts.id, BOTTOM)
-        connect(awesomeBar.id, BOTTOM, pill_wrapper.id, TOP)
-        (awesomeBar.layoutManager as? LinearLayoutManager)?.reverseLayout = false
-
-        connect(pill_wrapper.id, BOTTOM, PARENT_ID, BOTTOM)
-
-        applyTo(layout)
-    }
-}
--- a/app/src/main/res/layout/component_awesomebar.xml
+++ b/app/src/main/res/layout/component_awesomebar.xml
@ -11,9 +11,9 @@
    android:fadingEdge="horizontal"
    android:fadingEdgeLength="40dp"
    android:requiresFadingEdge="vertical"
-    app:layout_constraintBottom_toTopOf="@id/search_divider"
+    app:layout_constraintBottom_toTopOf="@id/pill_wrapper"
    app:layout_constraintEnd_toEndOf="parent"
    app:layout_constraintStart_toStartOf="parent"
-    app:layout_constraintTop_toBottomOf="@id/awesomeBar_barrier"
+    app:layout_constraintTop_toBottomOf="@id/search_with_shortcuts"
    mozac:awesomeBarDescriptionTextColor="?secondaryText"
    mozac:awesomeBarTitleTextColor="?primaryText" />
--- a/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/components/BackgroundServicesTest.kt
@ -18,16 +18,11 @@ import mozilla.components.feature.push.PushConfig
 import mozilla.components.service.fxa.DeviceConfig
 import mozilla.components.service.fxa.ServerConfig
 import mozilla.components.service.fxa.SyncConfig
-import mozilla.components.service.fxa.SyncEngine
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.base.observer.ObserverRegistry
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
 import org.junit.Test
-import org.mozilla.fenix.Experiments
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
-import org.mozilla.fenix.isInExperiment

 class BackgroundServicesTest {
    class TestableBackgroundServices(
@ -44,37 +39,6 @@ class BackgroundServicesTest {
        override fun makePush(pushConfig: PushConfig) = mockk<AutoPushFeature>(relaxed = true)
    }

-    @Test
-    fun `experiment flags`() {
-        val context = mockk<Context>(relaxed = true)
-
-        every { context.isInExperiment(eq(Experiments.asFeatureWebChannelsDisabled)) } returns false
-        assertEquals(
-            "urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel",
-            FxaServer.redirectUrl(context)
-        )
-
-        every { context.isInExperiment(eq(Experiments.asFeatureWebChannelsDisabled)) } returns true
-        assertEquals(
-            "https://accounts.firefox.com/oauth/success/a2270f727f45f648",
-            FxaServer.redirectUrl(context)
-        )
-
-        every { context.isInExperiment(eq(Experiments.asFeatureSyncDisabled)) } returns false
-        var backgroundServices = TestableBackgroundServices(context)
-        assertEquals(
-            SyncConfig(
-                setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords),
-                syncPeriodInMinutes = 240L
-            ),
-            backgroundServices.syncConfig
-        )
-
-        every { context.isInExperiment(eq(Experiments.asFeatureSyncDisabled)) } returns true
-        backgroundServices = TestableBackgroundServices(context)
-        assertNull(backgroundServices.syncConfig)
-    }
-
    @Test
    fun `telemetry account observer`() {
        val metrics = mockk<MetricController>()
--- a/buildSrc/src/main/java/Dependencies.kt
+++ b/buildSrc/src/main/java/Dependencies.kt
@ -126,7 +126,6 @@ object Deps {
    const val mozilla_service_sync_logins =
        "org.mozilla.components:service-sync-logins:${Versions.mozilla_android_components}"
    const val mozilla_service_firefox_accounts = "org.mozilla.components:service-firefox-accounts:${Versions.mozilla_android_components}"
-    const val mozilla_service_fretboard = "org.mozilla.components:service-fretboard:${Versions.mozilla_android_components}"
    const val mozilla_service_glean = "org.mozilla.components:service-glean:${Versions.mozilla_android_components}"
    const val mozilla_service_glean_forUnitTests = "org.mozilla.telemetry:glean-forUnitTests:${Versions.mozilla_glean}"
    const val mozilla_service_experiments = "org.mozilla.components:service-experiments:${Versions.mozilla_android_components}"