From 6b9dc4c9582633188a2825fa134fd905b50b363e Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Wed, 1 Dec 2021 15:23:01 -0500 Subject: [PATCH] [fenix] Introduce timeout for capturing thumbnail in BrowserAnimator --- .../mozilla/fenix/browser/BrowserAnimator.kt | 23 +++++++++++++++---- .../DefaultBrowserToolbarControllerTest.kt | 4 ++-- ...DefaultBrowserToolbarMenuControllerTest.kt | 4 ++-- 3 files changed, 23 insertions(+), 8 deletions(-) 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 d9e978105f..bfb9c8bc1d 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt @@ -12,6 +12,7 @@ import androidx.core.graphics.drawable.toDrawable import androidx.fragment.app.Fragment import androidx.lifecycle.LifecycleCoroutineScope import androidx.navigation.NavOptions +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import mozilla.components.concept.engine.EngineView import org.mozilla.fenix.R @@ -50,16 +51,25 @@ class BrowserAnimator( * make transitions from the browser fragment to the home fragment * smoother. So, in addition, we are currently also using it as a * workaround to prevent flashes during those navigations. + * + * @param timeoutInMs timeout in milliseconds after which the operation + * should be cancelled. + * @param onComplete callback invoked when operation is completed or + * cancelled, see [timeoutInMs]. The boolean passed to the lambda + * indicates whether or not the operation was successful i.e., true, + * if the bitmap was successfully set as a background, otherwise false. */ - fun captureEngineViewAndDrawStatically(onComplete: () -> Unit) { + fun captureEngineViewAndDrawStatically(timeoutInMs: Long = 250L, onComplete: (Boolean) -> Unit) { unwrappedEngineView?.asView()?.context.let { viewLifecycleScope.get()?.launch { + var cancelled = false + var completed = false // isAdded check is necessary because of a bug in viewLifecycleOwner. See AC#3828 if (!fragment.isAdded()) { return@launch } unwrappedEngineView?.captureThumbnail { bitmap -> - if (!fragment.isAdded()) { + if (!fragment.isAdded() || cancelled) { return@captureThumbnail } @@ -70,8 +80,13 @@ class BrowserAnimator( } unwrappedEngineView?.asView()?.visibility = View.GONE - - onComplete() + completed = true + onComplete(bitmap != null) + } + delay(timeoutInMs) + if (!completed) { + cancelled = true + onComplete(false) } } } diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt index d583b9db1b..e1557a674e 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarControllerTest.kt @@ -103,9 +103,9 @@ class DefaultBrowserToolbarControllerTest { } every { - browserAnimator.captureEngineViewAndDrawStatically(any()) + browserAnimator.captureEngineViewAndDrawStatically(any(), any()) } answers { - firstArg<() -> Unit>()() + secondArg<(Boolean) -> Unit>()(true) } tabCounterClicked = false diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index de3e419c8f..a5fd88abea 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -118,8 +118,8 @@ class DefaultBrowserToolbarMenuControllerTest { } every { settings.topSitesMaxLimit } returns 16 - val onComplete = slot<() -> Unit>() - every { browserAnimator.captureEngineViewAndDrawStatically(capture(onComplete)) } answers { onComplete.captured.invoke() } + val onComplete = slot<(Boolean) -> Unit>() + every { browserAnimator.captureEngineViewAndDrawStatically(any(), capture(onComplete)) } answers { onComplete.captured.invoke(true) } selectedTab = createTab("https://www.mozilla.org", id = "1") browserStore = BrowserStore(