From 46551eb926d5fd70a4240275b8188e579f171d1a Mon Sep 17 00:00:00 2001 From: Mugurell Date: Mon, 17 Jan 2022 19:24:36 +0200 Subject: [PATCH] For #11527 - Allow the decor view time to process the incoming insets There was a race between how insets are applied when entering immersive mode and enabling immersive mode restore by setting an insets listener which is now resolved by ensuring the decor view has the time needed to process the incoming insets, solution recommended on issuetracker https://issuetracker.google.com/u/2/issues/214012501 . Removed the onWindowFocusChangeListener extension property since by having to offer it through a getter the current implementation would always leak the old one. Fenix wasn't using it when this APIs allowed Fenix to pass such a listener and there was no issue observed so there should be no observable negative impact. (cherry picked from commit 6f60a8a44b9fa24b62c223130d39bc5ea8095ae6) # Conflicts: # docs/changelog.md --- .../support/ktx/android/view/Activity.kt | 24 +++----------- .../support/ktx/android/view/ActivityTest.kt | 32 ++----------------- docs/changelog.md | 23 +++++++++++++ 3 files changed, 29 insertions(+), 50 deletions(-) diff --git a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt index ded74e7fea5..dc5913aa1ef 100644 --- a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt +++ b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt @@ -7,7 +7,6 @@ package mozilla.components.support.ktx.android.view import android.app.Activity import android.os.Build import android.view.View -import android.view.ViewTreeObserver import android.view.WindowInsets.Type.statusBars import android.view.WindowManager import androidx.annotation.VisibleForTesting @@ -19,8 +18,7 @@ import mozilla.components.support.base.log.logger.Logger * Attempts to enter immersive mode - fullscreen with the status bar and navigation buttons hidden. * This will automatically register and use an * - an inset listener: [View.OnApplyWindowInsetsListener] on API 30+ or - * [View.OnSystemUiVisibilityChangeListener] for below APIs - * - a focus listener: [ViewTreeObserver.OnWindowFocusChangeListener] + * - a system visibility listener: [View.OnSystemUiVisibilityChangeListener] for below APIs. * * to restore immersive mode if interactions with various other widgets like the keyboard or dialogs * got the activity out of immersive mode without [exitImmersiveModeIfNeeded] being called. @@ -47,14 +45,13 @@ internal fun Activity.setAsImmersive() { */ @VisibleForTesting internal fun Activity.enableImmersiveModeRestore() { - window.decorView.viewTreeObserver?.addOnWindowFocusChangeListener(onWindowFocusChangeListener) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - window.decorView.setOnApplyWindowInsetsListener { _, insets -> + window.decorView.setOnApplyWindowInsetsListener { view, insets -> if (insets.isVisible(statusBars())) { setAsImmersive() } - insets + // Allow the decor view to have a chance to process the incoming WindowInsets. + view.onApplyWindowInsets(insets) } } else { @Suppress("DEPRECATION") // insets.isVisible(int) is available only starting with API 30 @@ -75,8 +72,6 @@ fun Activity.exitImmersiveModeIfNeeded() { return } - window.decorView.viewTreeObserver?.removeOnWindowFocusChangeListener(onWindowFocusChangeListener) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { window.decorView.setOnApplyWindowInsetsListener(null) } else { @@ -90,17 +85,6 @@ fun Activity.exitImmersiveModeIfNeeded() { } } -/** - * OnWindowFocusChangeListener used to ensure immersive mode is not broken by other views interactions. - */ -@VisibleForTesting -internal val Activity.onWindowFocusChangeListener: ViewTreeObserver.OnWindowFocusChangeListener - get() = ViewTreeObserver.OnWindowFocusChangeListener { hasFocus -> - if (hasFocus) { - setAsImmersive() - } - } - /** * Calls [Activity.reportFullyDrawn] while also preventing crashes under some circumstances. * diff --git a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt index ac2f466af04..45bbc3994c9 100644 --- a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt +++ b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt @@ -54,7 +54,6 @@ class ActivityTest { verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION // verify that the immersive mode restoration is set as expected - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(any()) verify(window.decorView).setOnSystemUiVisibilityChangeListener(any()) } @@ -65,15 +64,13 @@ class ActivityTest { verify(window).addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION - verify(window.decorView.viewTreeObserver, never()).addOnWindowFocusChangeListener(any()) verify(window.decorView, never()).setOnSystemUiVisibilityChangeListener(any()) } @Test - fun `check enableImmersiveModeRestore sets focus and insets listeners`() { + fun `check enableImmersiveModeRestore sets insets listeners`() { activity.enableImmersiveModeRestore() - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(any()) verify(window.decorView).setOnSystemUiVisibilityChangeListener(any()) } @@ -97,26 +94,6 @@ class ActivityTest { verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION } - @Test - fun `check enableImmersiveModeRestore set focus listener has the correct behavior`() { - val focusListenerCaptor = argumentCaptor() - - activity.enableImmersiveModeRestore() - verify(window.decorView.viewTreeObserver).addOnWindowFocusChangeListener(focusListenerCaptor.capture()) - - focusListenerCaptor.value.onWindowFocusChanged(false) - // If the activity is not focused restoration is needed. - // Cannot test if "setAsImmersive()" was called it being an extension function but we can check the effect of that call. - verify(window, never()).addFlags(anyInt()) - verify(decorView, never()).systemUiVisibility = anyInt() - verify(decorView, never()).systemUiVisibility = anyInt() - - focusListenerCaptor.value.onWindowFocusChanged(true) - verify(window).addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) - verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN - verify(decorView).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION - } - @Test fun `check exitImmersiveModeIfNeeded sets the correct flags`() { val attributes = mock(WindowManager.LayoutParams::class.java) @@ -129,7 +106,6 @@ class ActivityTest { verify(decorView, never()).systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN.inv() verify(decorView, never()).systemUiVisibility = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION.inv() verify(decorView, never()).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver, never()).removeOnWindowFocusChangeListener(any()) attributes.flags = WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON @@ -137,13 +113,11 @@ class ActivityTest { verify(window).clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) verify(decorView, times(2)).systemUiVisibility = View.SYSTEM_UI_FLAG_VISIBLE - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) verify(decorView).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) } @Test - fun `check exitImmersiveModeIfNeeded correctly cleanups inset and focus listeners`() { + fun `check exitImmersiveModeIfNeeded correctly cleanups the insets listeners`() { val attributes = mock(WindowManager.LayoutParams::class.java) `when`(window.attributes).thenReturn(attributes) attributes.flags = 0 @@ -151,12 +125,10 @@ class ActivityTest { activity.exitImmersiveModeIfNeeded() verify(decorView, never()).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver, never()).removeOnWindowFocusChangeListener(any()) attributes.flags = WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON activity.exitImmersiveModeIfNeeded() verify(decorView).setOnSystemUiVisibilityChangeListener(null) - verify(window.decorView.viewTreeObserver).removeOnWindowFocusChangeListener(any()) } } diff --git a/docs/changelog.md b/docs/changelog.md index 377bb554b6d..a5631acdcc6 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,29 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +<<<<<<< HEAD +======= +* **support-ktx** + * 🚒 Bug fixed [issue #11527](https://github.com/mozilla-mobile/android-components/issues/11527) - Fix some situations in which the immersive mode wasn't properly applied. + +* **lib/publicsuffixlist** + * ⚠️ **This is a breaking change**: Removed `String.urlToTrimmedHost` extension method. + +* **feature-top-sites** + * ⚠️ **This is a breaking change**: The existing data class `TopSite` has been converted into a sealed class. [#11483](https://github.com/mozilla-mobile/android-components/issues/11483) + * Extend `DefaultTopSitesStorage` to accept a `TopSitesProvider` for fetching top sites. [#11483](https://github.com/mozilla-mobile/android-components/issues/11483) + +* **service-contile** + * Adds a `ContileTopSitesProvider` that implements `TopSitesProvider` for returning top sites from the Contile services API. [#11483](https://github.com/mozilla-mobile/android-components/issues/11483) + +# 97.0.0 +* [Commits](https://github.com/mozilla-mobile/android-components/compare/v96.0.0...v97.0.0) +* [Milestone](https://github.com/mozilla-mobile/android-components/milestone/144?closed=1) +* [Dependencies](https://github.com/mozilla-mobile/android-components/blob/v97.0.0/buildSrc/src/main/java/Dependencies.kt) +* [Gecko](https://github.com/mozilla-mobile/android-components/blob/v97.0.0/buildSrc/src/main/java/Gecko.kt) +* [Configuration](https://github.com/mozilla-mobile/android-components/blob/v97.0.0/.config.yml) + +>>>>>>> 6f60a8a44b (For #11527 - Allow the decor view time to process the incoming insets) * **support-ktx** * 🚒 Bug fixed [issue #11374](https://github.com/mozilla-mobile/android-components/issues/11374) - Restore immersive mode after interacting with other Windows. * ⚠️ **This is a breaking change**: `OnWindowFocusChangeListener` parameter is removed from `Activity.enterToImmersiveMode()`. There was no way to guarantee that the argument knew to handle immersive mode. Now everything is handled internally.