From f66312963281cdc790dafca90a07e32e2dca8294 Mon Sep 17 00:00:00 2001 From: ekager Date: Mon, 4 May 2020 23:29:33 -0700 Subject: [PATCH] For #6313 - On first load, hides engineView until firstContentfulPaint --- .../mozilla/fenix/ui/robots/BrowserRobot.kt | 19 ++--- .../java/org/mozilla/fenix/FeatureFlags.kt | 5 ++ .../fenix/browser/BaseBrowserFragment.kt | 73 ++++++++++++++----- .../mozilla/fenix/browser/BrowserAnimator.kt | 38 ++-------- .../fenix/settings/SecretSettingsFragment.kt | 6 ++ .../java/org/mozilla/fenix/utils/Settings.kt | 6 ++ app/src/main/res/values/preference_keys.xml | 2 + app/src/main/res/values/static_strings.xml | 2 + .../res/xml/secret_settings_preferences.xml | 5 ++ 9 files changed, 100 insertions(+), 56 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt index 964f65d45..5d6732cce 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt @@ -44,12 +44,6 @@ import org.mozilla.fenix.helpers.click import org.mozilla.fenix.helpers.ext.waitNotNull class BrowserRobot { - - fun verifyBrowserScreen() { - onView(ViewMatchers.withResourceName("browserLayout")) - .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) - } - fun verifyCurrentPrivateSession(context: Context) { val session = context.components.core.sessionManager.selectedSession assertTrue("Current session is private", session?.private!!) @@ -84,6 +78,10 @@ class BrowserRobot { */ fun verifyPageContent(expectedText: String) { + mDevice.waitNotNull( + Until.findObject(By.res("org.mozilla.fenix.debug:id/engineView")), + waitingTime + ) assertTrue(mDevice.findObject(UiSelector().text(expectedText)).waitForExists(waitingTime)) } @@ -145,7 +143,8 @@ class BrowserRobot { fun verifyEnhancedTrackingProtectionSwitch() = assertEnhancedTrackingProtectionSwitch() - fun clickEnhancedTrackingProtectionSwitchOffOn() = onView(withResourceName("switch_widget")).click() + fun clickEnhancedTrackingProtectionSwitchOffOn() = + onView(withResourceName("switch_widget")).click() fun verifyProtectionSettingsButton() = assertProtectionSettingsButton() @@ -191,7 +190,8 @@ class BrowserRobot { fun clickEnhancedTrackingProtectionPanel() = enhancedTrackingProtectionPanel().click() - fun verifyEnhancedTrackingProtectionPanelNotVisible() = assertEnhancedTrackingProtectionPanelNotVisible() + fun verifyEnhancedTrackingProtectionPanelNotVisible() = + assertEnhancedTrackingProtectionPanelNotVisible() fun clickContextOpenLinkInNewTab() { val mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) @@ -423,7 +423,8 @@ fun navURLBar() = onView(withId(R.id.mozac_browser_toolbar_url_view)) private fun assertNavURLBar() = navURLBar() .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -fun enhancedTrackingProtectionPanel() = onView(withId(R.id.mozac_browser_toolbar_tracking_protection_indicator)) +fun enhancedTrackingProtectionPanel() = + onView(withId(R.id.mozac_browser_toolbar_tracking_protection_indicator)) private fun assertEnhancedTrackingProtectionPanelNotVisible() { enhancedTrackingProtectionPanel() diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index d3eb4bdc6..06580a6b9 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -43,4 +43,9 @@ object FeatureFlags { * Enables the new search experience */ val newSearchExperience = Config.channel.isDebug + + /** + * Enables wait til first contentful paint + */ + val waitUntilPaintToDraw = Config.channel.isDebug } diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 14ab0713c..266bc7184 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -144,9 +144,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session private val fullScreenFeature = ViewBoundFeatureWrapper() private val swipeRefreshFeature = ViewBoundFeatureWrapper() private val webchannelIntegration = ViewBoundFeatureWrapper() - private val sitePermissionWifiIntegration = ViewBoundFeatureWrapper() + private val sitePermissionWifiIntegration = + ViewBoundFeatureWrapper() private val secureWindowFeature = ViewBoundFeatureWrapper() - private var fullScreenMediaFeature = ViewBoundFeatureWrapper() + private var fullScreenMediaFeature = + ViewBoundFeatureWrapper() private var pipFeature: PictureInPictureFeature? = null var customTabSessionId: String? = null @@ -202,7 +204,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session engineView = WeakReference(engineView), swipeRefresh = WeakReference(swipeRefresh), viewLifecycleScope = WeakReference(viewLifecycleOwner.lifecycleScope), - arguments = requireArguments() + arguments = requireArguments(), + firstContentfulHappened = ::didFirstContentfulHappen ).apply { beginAnimateInIfNecessary() } @@ -412,8 +415,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } } - resumeDownloadDialogState(sessionManager.selectedSession?.id, - store, view, context, toolbarHeight) + resumeDownloadDialogState( + sessionManager.selectedSession?.id, + store, view, context, toolbarHeight + ) downloadsFeature.set( downloadFeature, @@ -506,7 +511,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session onNeedToRequestPermissions = { permissions -> requestPermissions(permissions, REQUEST_CODE_APP_PERMISSIONS) }, - onShouldShowRequestPermissionRationale = { shouldShowRequestPermissionRationale(it) }), + onShouldShowRequestPermissionRationale = { + shouldShowRequestPermissionRationale( + it + ) + }), owner = this, view = view ) @@ -568,6 +577,21 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session .collect { tab -> pipModeChanged(tab) } } + if (context.settings().waitToShowPageUntilFirstPaint) { + store.flowScoped(viewLifecycleOwner) { flow -> + flow.mapNotNull { state -> + state.findTabOrCustomTabOrSelectedTab( + customTabSessionId + ) + } + .ifChanged { it.content.firstContentfulPaint } + .collect { + engineView?.asView()?.isVisible = + it.content.firstContentfulPaint || it.content.progress == 100 + } + } + } + @Suppress("ConstantConditionIf") if (FeatureFlags.pullToRefreshEnabled) { val primaryTextColor = @@ -687,7 +711,12 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session val context = requireContext() val behavior = when (context.settings().toolbarPosition) { ToolbarPosition.BOTTOM -> EngineViewBottomBehavior(context, null) - ToolbarPosition.TOP -> SwipeRefreshScrollingViewBehavior(context, null, engineView, browserToolbarView) + ToolbarPosition.TOP -> SwipeRefreshScrollingViewBehavior( + context, + null, + engineView, + browserToolbarView + ) } (swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams).behavior = behavior @@ -751,12 +780,13 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session super.onStop() initUIJob?.cancel() - requireComponents.core.store.state.findTabOrCustomTabOrSelectedTab(customTabSessionId)?.let { session -> - // If we didn't enter PiP, exit full screen on stop - if (!session.content.pictureInPictureEnabled && fullScreenFeature.onBackPressed()) { - fullScreenChanged(false) + requireComponents.core.store.state.findTabOrCustomTabOrSelectedTab(customTabSessionId) + ?.let { session -> + // If we didn't enter PiP, exit full screen on stop + if (!session.content.pictureInPictureEnabled && fullScreenFeature.onBackPressed()) { + fullScreenChanged(false) + } } - } } @CallSuper @@ -950,7 +980,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session .setAction(getString(R.string.edit_bookmark_snackbar_action)) { nav( R.id.browserFragment, - BrowserFragmentDirections.actionGlobalBookmarkEditFragment(guid, true) + BrowserFragmentDirections.actionGlobalBookmarkEditFragment( + guid, + true + ) ) } .show() @@ -988,10 +1021,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session // Close find in page bar if opened findInPageIntegration.onBackPressed() FenixSnackbar.make( - view = requireView().browserLayout, - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = false - ) + view = requireView().browserLayout, + duration = Snackbar.LENGTH_SHORT, + isDisplayedWithBrowserToolbar = false + ) .setText(getString(R.string.full_screen_notification)) .show() activity?.enterToImmersiveMode() @@ -1014,6 +1047,12 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session } } + private fun didFirstContentfulHappen() = + if (!requireContext().settings().waitToShowPageUntilFirstPaint) true else + context?.components?.core?.store?.state?.findTabOrCustomTabOrSelectedTab( + customTabSessionId + )?.content?.firstContentfulPaint ?: false + /* * Dereference these views when the fragment view is destroyed to prevent memory leaks */ diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt index 02a94f2d1..c79c53a54 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt @@ -34,7 +34,8 @@ class BrowserAnimator( private val engineView: WeakReference, private val swipeRefresh: WeakReference, private val viewLifecycleScope: WeakReference, - private val arguments: Bundle + private val arguments: Bundle, + private val firstContentfulHappened: () -> Boolean ) { private val unwrappedEngineView: EngineView? @@ -53,22 +54,9 @@ class BrowserAnimator( } doOnEnd { - unwrappedEngineView?.asView()?.visibility = View.VISIBLE - unwrappedSwipeRefresh?.background = null - arguments.putBoolean(SHOULD_ANIMATE_FLAG, false) - } - - interpolator = DecelerateInterpolator() - duration = ANIMATION_DURATION - } - - private val browserFadeInValueAnimator = ValueAnimator.ofFloat(0f, END_ANIMATOR_VALUE).apply { - addUpdateListener { - unwrappedSwipeRefresh?.alpha = it.animatedFraction - } - - doOnEnd { - unwrappedEngineView?.asView()?.visibility = View.VISIBLE + if (firstContentfulHappened()) { + unwrappedEngineView?.asView()?.visibility = View.VISIBLE + } unwrappedSwipeRefresh?.background = null arguments.putBoolean(SHOULD_ANIMATE_FLAG, false) } @@ -93,20 +81,10 @@ class BrowserAnimator( } } else { unwrappedSwipeRefresh?.alpha = 1f - unwrappedEngineView?.asView()?.visibility = View.VISIBLE - unwrappedSwipeRefresh?.background = null - } - } - - /** - * Triggers the *zoom out* browser animation to run. - */ - fun beginAnimateOut() { - viewLifecycleScope.get()?.launch(Dispatchers.Main) { - captureEngineViewAndDrawStatically { - unwrappedEngineView?.asView()?.visibility = View.GONE - browserZoomInValueAnimator.reverse() + if (firstContentfulHappened()) { + unwrappedEngineView?.asView()?.visibility = View.VISIBLE } + unwrappedSwipeRefresh?.background = null } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/SecretSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SecretSettingsFragment.kt index e49dcf46b..c3e663104 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SecretSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SecretSettingsFragment.kt @@ -30,5 +30,11 @@ class SecretSettingsFragment : PreferenceFragmentCompat() { isChecked = context.settings().useNewSearchExperience onPreferenceChangeListener = SharedPreferenceUpdater() } + + requirePreference(R.string.pref_key_wait_first_paint).apply { + isVisible = FeatureFlags.waitUntilPaintToDraw + isChecked = context.settings().waitToShowPageUntilFirstPaint + onPreferenceChangeListener = SharedPreferenceUpdater() + } } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 631920d86..8ab5be5ec 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -103,6 +103,12 @@ class Settings(private val appContext: Context) : PreferencesHolder { featureFlag = FeatureFlags.newSearchExperience ) + var waitToShowPageUntilFirstPaint by featureFlagPreference( + appContext.getPreferenceKey(R.string.pref_key_wait_first_paint), + default = false, + featureFlag = FeatureFlags.waitUntilPaintToDraw + ) + var forceEnableZoom by booleanPreference( appContext.getPreferenceKey(R.string.pref_key_accessibility_force_enable_zoom), default = false diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 1edfecb58..700697231 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -175,6 +175,8 @@ pref_key_use_new_search_experience + pref_key_wait_first_paint + pref_key_debug_settings pref_key_open_tabs_count diff --git a/app/src/main/res/values/static_strings.xml b/app/src/main/res/values/static_strings.xml index 91fb0f67f..e51388ca3 100644 --- a/app/src/main/res/values/static_strings.xml +++ b/app/src/main/res/values/static_strings.xml @@ -34,6 +34,8 @@ Secret Settings Use New Search Experience + + Wait Until First Paint To Show Page Content link diff --git a/app/src/main/res/xml/secret_settings_preferences.xml b/app/src/main/res/xml/secret_settings_preferences.xml index 6e65b1d3c..e8e2f95d9 100644 --- a/app/src/main/res/xml/secret_settings_preferences.xml +++ b/app/src/main/res/xml/secret_settings_preferences.xml @@ -9,4 +9,9 @@ android:key="@string/pref_key_use_new_search_experience" android:title="@string/preferences_debug_settings_use_new_search_experience" app:iconSpaceReserved="false" /> +