From d6952cb2aa01a8b4e1f654840ea0d4efff8327f0 Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Thu, 23 Jun 2022 14:06:19 -0700 Subject: [PATCH] Fixes #25545: wait to draw Pocket until after first frame --- app/src/main/java/org/mozilla/fenix/FeatureFlags.kt | 2 +- .../mozilla/fenix/components/appstate/AppAction.kt | 5 +++++ .../mozilla/fenix/components/appstate/AppState.kt | 2 ++ .../fenix/components/appstate/AppStoreReducer.kt | 3 +++ .../java/org/mozilla/fenix/home/HomeFragment.kt | 5 ++++- .../fenix/home/pocket/PocketCategoriesViewHolder.kt | 5 +++++ .../fenix/home/pocket/PocketStoriesViewHolder.kt | 13 +++++++++++++ app/src/main/res/layout/top_placeholder_item.xml | 6 +++++- 8 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 5fb1dfe4aa..7ca2f7a221 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -87,7 +87,7 @@ object FeatureFlags { /** * Enables the Task Continuity enhancements. */ - val taskContinuityFeature = Config.channel.isDebug + val taskContinuityFeature = Config.channel.isNightlyOrDebug /** * Enables the Unified Search feature. diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index 5bbec5f2e7..d15d0764d8 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -27,6 +27,11 @@ import org.mozilla.fenix.gleanplumb.MessagingState */ sealed class AppAction : Action { data class UpdateInactiveExpanded(val expanded: Boolean) : AppAction() + + /** + * Updates whether the first frame of the homescreen has been [drawn]. + */ + data class UpdateFirstFrameDrawn(val drawn: Boolean) : AppAction() data class AddNonFatalCrash(val crash: NativeCodeCrash) : AppAction() data class RemoveNonFatalCrash(val crash: NativeCodeCrash) : AppAction() object RemoveAllNonFatalCrashes : AppAction() diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt index 8eaf79f193..be5338b228 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt @@ -28,6 +28,7 @@ import org.mozilla.fenix.gleanplumb.MessagingState * * @property inactiveTabsExpanded A flag to know if the Inactive Tabs section of the Tabs Tray * should be expanded when the tray is opened. + * @property firstFrameDrawn Flag indicating whether the first frame of the homescreen has been drawn. * @property nonFatalCrashes List of non-fatal crashes that allow the app to continue being used. * @property collections The list of [TabCollection] to display in the [HomeFragment]. * @property expandedCollections A set containing the ids of the [TabCollection] that are expanded @@ -50,6 +51,7 @@ import org.mozilla.fenix.gleanplumb.MessagingState */ data class AppState( val inactiveTabsExpanded: Boolean = false, + val firstFrameDrawn: Boolean = false, val nonFatalCrashes: List = emptyList(), val collections: List = emptyList(), val expandedCollections: Set = emptySet(), diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index d0fa7faff3..497dfa2a29 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -26,6 +26,9 @@ internal object AppStoreReducer { fun reduce(state: AppState, action: AppAction): AppState = when (action) { is AppAction.UpdateInactiveExpanded -> state.copy(inactiveTabsExpanded = action.expanded) + is AppAction.UpdateFirstFrameDrawn -> { + state.copy(firstFrameDrawn = action.drawn) + } is AppAction.AddNonFatalCrash -> state.copy(nonFatalCrashes = state.nonFatalCrashes + action.crash) is AppAction.RemoveNonFatalCrash -> diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index d1d6f01927..de8e1adbd7 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -28,6 +28,7 @@ import androidx.constraintlayout.widget.ConstraintSet.TOP import androidx.coordinatorlayout.widget.CoordinatorLayout import androidx.core.content.ContextCompat import androidx.core.view.isVisible +import androidx.core.view.doOnPreDraw import androidx.core.view.updateLayoutParams import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels @@ -385,11 +386,13 @@ class HomeFragment : Fragment() { displayWallpaperIfEnabled() + binding.root.doOnPreDraw { + requireComponents.appStore.dispatch(AppAction.UpdateFirstFrameDrawn(drawn = true)) + } // DO NOT MOVE ANYTHING BELOW THIS addMarker CALL! requireComponents.core.engine.profiler?.addMarker( MarkersFragmentLifecycleCallbacks.MARKER_NAME, profilerStartTime, "HomeFragment.onCreateView", ) - return binding.root } diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt index 658a396c97..6f33b21b3e 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt @@ -49,11 +49,16 @@ class PocketCategoriesViewHolder( composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) + val homeScreenReady = components.appStore + .observeAsComposableState { state -> state.firstFrameDrawn }.value ?: false + val categories = components.appStore .observeAsComposableState { state -> state.pocketStoriesCategories }.value val categoriesSelections = components.appStore .observeAsComposableState { state -> state.pocketStoriesCategoriesSelections }.value + // See the detailed comment in PocketStoriesViewHolder for reasoning behind this change. + if (!homeScreenReady) return Column { Spacer(Modifier.height(24.dp)) diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt index d7e81a713d..ec73174d0f 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt @@ -52,9 +52,22 @@ class PocketStoriesViewHolder( override fun Content() { val horizontalPadding = dimensionResource(R.dimen.home_item_horizontal_margin) + val homeScreenReady = components.appStore + .observeAsComposableState { state -> state.firstFrameDrawn }.value ?: false + val stories = components.appStore .observeAsComposableState { state -> state.pocketStories }.value + /* This was originally done to address this perf issue: + * https://github.com/mozilla-mobile/fenix/issues/25545 for details. + * It was determined that Pocket content was becoming available before the first frame was + * rendered more regularly. Including Pocket in the first render pass significantly + * increases time-to-render in lower-end devices. By waiting until the first frame has + * rendered, the perceived performance should increase since the app becomes active more + * quickly. This was intended as a workaround until the Compose upgrade was completed and a + * more robust solution could be investigated. + */ + if (!homeScreenReady) return LaunchedEffect(stories) { // We should report back when a certain story is actually being displayed. // Cannot do it reliably so for now we'll just mass report everything as being displayed. diff --git a/app/src/main/res/layout/top_placeholder_item.xml b/app/src/main/res/layout/top_placeholder_item.xml index fc1596dbcb..a5919395c2 100644 --- a/app/src/main/res/layout/top_placeholder_item.xml +++ b/app/src/main/res/layout/top_placeholder_item.xml @@ -2,6 +2,10 @@ + + + android:layout_height="0dp" + android:focusable="true" />