1
0
Fork 0

For #9100: Follow-up to fix memory leak in NotificationSessionObserver

The observer was moved and is now bound to the activity and its
context. If the activity is re-created we leak the observer and
therefore the activity itself.

With this we make sure to stop the observer and also don't use
the activity context to begin with.
master
Christian Sadilek 2020-06-25 13:41:35 -04:00
parent c77ddd8d26
commit 64440409b0
5 changed files with 50 additions and 10 deletions

View File

@ -107,6 +107,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
private var isVisuallyComplete = false
private var visualCompletenessQueue: RunWhenReadyQueue? = null
private var privateNotificationObserver: NotificationSessionObserver? = null
private var isToolbarInflated = false
@ -156,8 +157,9 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
sessionObserver = UriOpenedObserver(this)
checkPrivateShortcutEntryPoint(intent)
val privateNotificationObserver = NotificationSessionObserver(this)
privateNotificationObserver.start()
privateNotificationObserver = NotificationSessionObserver(applicationContext).also {
it.start()
}
if (isActivityColdStarted(intent, savedInstanceState)) {
externalSourceIntentProcessors.any { it.process(intent, navHost.navController, this.intent) }
@ -218,6 +220,11 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
BrowsersCache.resetAll()
}
override fun onDestroy() {
super.onDestroy()
privateNotificationObserver?.stop()
}
/**
* Handles intents received when the activity is open.
*/

View File

@ -20,25 +20,22 @@ import org.mozilla.fenix.ext.components
* indicating that a private tab is open.
*/
class NotificationSessionObserver(
private val context: Context,
private val applicationContext: Context,
private val notificationService: SessionNotificationService.Companion = SessionNotificationService
) {
private var scope: CoroutineScope? = null
private var started = false
@ExperimentalCoroutinesApi
fun start() {
scope = context.components.core.store.flowScoped { flow ->
scope = applicationContext.components.core.store.flowScoped { flow ->
flow.map { state -> state.privateTabs.isNotEmpty() }
.ifChanged()
.collect { hasPrivateTabs ->
if (hasPrivateTabs) {
notificationService.start(context, isStartedFromPrivateShortcut)
started = true
} else if (started) {
notificationService.stop(context)
started = false
notificationService.start(applicationContext, isStartedFromPrivateShortcut)
} else if (SessionNotificationService.started) {
notificationService.stop(applicationContext)
}
}
}

View File

@ -138,6 +138,7 @@ class SessionNotificationService : Service() {
private const val ACTION_START = "start"
private const val ACTION_ERASE = "erase"
internal var started = false
internal fun start(
context: Context,
@ -154,6 +155,8 @@ class SessionNotificationService : Service() {
ThreadUtils.postToMainThread(Runnable {
context.startService(intent)
})
started = true
}
internal fun stop(context: Context) {
@ -164,6 +167,8 @@ class SessionNotificationService : Service() {
ThreadUtils.postToMainThread(Runnable {
context.stopService(intent)
})
started = false
}
}
}

View File

@ -1,3 +1,7 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.session
import android.content.Context

View File

@ -0,0 +1,27 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.session
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@RunWith(FenixRobolectricTestRunner::class)
class SessionNotificationServiceTest {
@Test
fun `Service keeps tracked of started state`() {
assertFalse(SessionNotificationService.started)
SessionNotificationService.start(testContext, false)
assertTrue(SessionNotificationService.started)
SessionNotificationService.stop(testContext)
assertFalse(SessionNotificationService.started)
}
}