Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
For #11527 - Allow the decor view time to process the incoming insets
Browse files Browse the repository at this point in the history 
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 6f60a8a)

# Conflicts:
#	docs/changelog.md
  • Loading branch information
@Mugurell
Mugurell authored and mergify-bot committed Feb 8, 2022
1 parent e0d0c84 commit 46551eb
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 50 deletions.
24 changes: 4 additions & 20 deletions components/support/ktx/src/main/java/mozilla/components/support/ktx/android/view/Activity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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.
*
Expand Down
32 changes: 2 additions & 30 deletions ...nts/support/ktx/src/test/java/mozilla/components/support/ktx/android/view/ActivityTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand All @@ -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())
}

Expand All @@ -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)
Expand All @@ -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())
}
}
23 changes: 23 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

2 comments on commit 46551eb

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to fetch task artifact public/github/customCheckRunText.md for GitHub integration.
Make sure the artifact exists on the worker or other location.

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh! Looks like an error! Details

Failed to fetch task artifact public/github/customCheckRunText.md for GitHub integration.
Make sure the artifact exists on the worker or other location.

Please sign in to comment.