For mozilla-mobile#11527 - Allow the decor view time to process the i…

…ncoming 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.
Mugurell · Jan 17, 2022 · a233802 · a233802
1 parent 2222939
commit a233802
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 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
@@ -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/...nts/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt b/...nts/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<ViewTreeObserver.OnWindowFocusChangeListener>()
-
-        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,34 +106,29 @@ 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
 
         activity.exitImmersiveModeIfNeeded()
 
         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
 
         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
@@ -11,6 +11,9 @@ 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)
 
+* **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.