From 93859d8560331a2f69ef0980bef0605ea9b917a9 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 31 Aug 2022 17:46:22 +0300 Subject: [PATCH] For #26723 - Fix WallpapersObserver leaking a view from HomeFragment The observer initialized with a fragment View would outlive the fragment and in that allow for leaking the entire fragment layout. --- .../main/java/org/mozilla/fenix/home/HomeFragment.kt | 8 +++++--- .../java/org/mozilla/fenix/home/HomeFragmentTest.kt | 11 +++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index f66055c66..da38cb2c6 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -121,7 +121,8 @@ import kotlin.math.min @Suppress("TooManyFunctions", "LargeClass") class HomeFragment : Fragment() { private val args by navArgs() - private lateinit var bundleArgs: Bundle + @VisibleForTesting + internal lateinit var bundleArgs: Bundle private var _binding: FragmentHomeBinding? = null private val binding get() = _binding!! @@ -163,7 +164,7 @@ class HomeFragment : Fragment() { private var appBarLayout: AppBarLayout? = null private lateinit var currentMode: CurrentMode @VisibleForTesting - internal lateinit var wallpapersObserver: WallpapersObserver + internal var wallpapersObserver: WallpapersObserver? = null private val topSitesFeature = ViewBoundFeatureWrapper() private val messagingFeature = ViewBoundFeatureWrapper() @@ -418,7 +419,7 @@ class HomeFragment : Fragment() { // Otherwise the portrait wallpaper may remain shown on landscape, // see https://github.com/mozilla-mobile/fenix/issues/26638 runBlockingIncrement { - wallpapersObserver.applyCurrentWallpaper() + wallpapersObserver?.applyCurrentWallpaper() } } } @@ -700,6 +701,7 @@ class HomeFragment : Fragment() { _sessionControlInteractor = null sessionControlView = null appBarLayout = null + wallpapersObserver = null _binding = null bundleArgs.clear() } diff --git a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt index c5e3b56f5..4d6d6e8e3 100644 --- a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentTest.kt @@ -19,6 +19,7 @@ import mozilla.components.feature.top.sites.TopSite import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -156,4 +157,14 @@ class HomeFragmentTest { assertFalse(homeFragment.shouldEnableWallpaper()) } + + @Test + fun `GIVEN the wallpaper feature is active WHEN the fragment view is destroyed THEN cleanup the wallpaper observer`() { + homeFragment.bundleArgs = mockk(relaxed = true) + homeFragment.wallpapersObserver = mockk() + + homeFragment.onDestroyView() + + assertNull(homeFragment.wallpapersObserver) + } }