Skip to content

Commit

Permalink
Ensure custom views cancel coroutine scopes in onDetachedFromWindow (#…
Browse files Browse the repository at this point in the history
…5551)

Task/Issue URL: https://app.asana.com/0/1198194956794324/1209242164061666/f

### Description
Ensure all custom views cancel coroutineScope

### Steps to test this PR
Smoke tests features that related to the custom view changes
  • Loading branch information
aitorvs authored Jan 29, 2025
1 parent 6a27773 commit b682075
Show file tree
Hide file tree
Showing 31 changed files with 340 additions and 315 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,20 @@

package com.duckduckgo.mobile.android.vpn.ui.newtab

import android.annotation.SuppressLint
import android.content.Context
import android.util.AttributeSet
import android.view.View
import android.widget.LinearLayout
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.findViewTreeLifecycleOwner
import androidx.lifecycle.findViewTreeViewModelStoreOwner
import androidx.lifecycle.lifecycleScope
import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
import com.duckduckgo.anvil.annotations.InjectWith
import com.duckduckgo.anvil.annotations.PriorityKey
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.ViewViewModelFactory
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.di.scopes.AppScope
Expand All @@ -42,9 +44,7 @@ import com.duckduckgo.newtabpage.api.NewTabPageSectionSettingsPlugin
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.android.support.AndroidSupportInjection
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

Expand All @@ -58,9 +58,12 @@ class AppTrackingProtectionNewTabSettingView @JvmOverloads constructor(
@Inject
lateinit var viewModelFactory: ViewViewModelFactory

@Inject
lateinit var dispatchers: DispatcherProvider

private val binding: ViewApptpSettingsItemBinding by viewBinding()

private var coroutineScope: CoroutineScope? = null
private val conflatedJob = ConflatedJob()

private val viewModel: AppTrackingProtectionNewTabSettingsViewModel by lazy {
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[AppTrackingProtectionNewTabSettingsViewModel::class.java]
Expand All @@ -71,12 +74,14 @@ class AppTrackingProtectionNewTabSettingView @JvmOverloads constructor(
super.onAttachedToWindow()
findViewTreeLifecycleOwner()?.lifecycle?.addObserver(viewModel)

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

viewModel.viewState
conflatedJob += viewModel.viewState
.onEach { render(it) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)
}

override fun onDetachedFromWindow() {
conflatedJob.cancel()
super.onDetachedFromWindow()
}

private fun render(viewState: ViewState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.duckduckgo.mobile.android.vpn.ui.newtab

import android.annotation.SuppressLint
import android.content.Context
import android.util.AttributeSet
import android.view.View
Expand All @@ -28,6 +27,8 @@ import com.duckduckgo.anvil.annotations.InjectWith
import com.duckduckgo.common.ui.view.gone
import com.duckduckgo.common.ui.view.show
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.ViewViewModelFactory
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.di.scopes.ViewScope
Expand All @@ -46,9 +47,7 @@ import com.duckduckgo.newtabpage.api.NewTabPageSection
import com.duckduckgo.newtabpage.api.NewTabPageSectionPlugin
import dagger.android.support.AndroidSupportInjection
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

Expand All @@ -69,30 +68,35 @@ class AppTrackingProtectionStateView @JvmOverloads constructor(
@Inject
lateinit var viewModelFactory: ViewViewModelFactory

@Inject
lateinit var dispatchers: DispatcherProvider

private val viewModel: PrivacyReportViewModel by lazy {
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[PrivacyReportViewModel::class.java]
}

private var coroutineScope: CoroutineScope? = null
private val conflatedJob = ConflatedJob()

private val binding: FragmentDeviceShieldCtaBinding by viewBinding()

override fun onAttachedToWindow() {
AndroidSupportInjection.inject(this)
super.onAttachedToWindow()

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

viewModel.viewStateFlow
conflatedJob += viewModel.viewStateFlow
.onEach { viewState -> renderViewState(viewState) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)

deviceShieldPixels.didShowNewTabSummary()

configureViewReferences()
}

override fun onDetachedFromWindow() {
conflatedJob.cancel()
super.onDetachedFromWindow()
}

private fun configureViewReferences() {
binding.deviceShieldCtaLayout.setOnClickListener {
deviceShieldPixels.didPressNewTabSummary()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.duckduckgo.app.browser.favicon.setting

import android.annotation.SuppressLint
import android.content.*
import android.util.*
import android.widget.*
Expand All @@ -26,13 +25,11 @@ import com.duckduckgo.app.browser.databinding.ViewSyncFaviconsFetchingBinding
import com.duckduckgo.app.browser.favicon.setting.FaviconFetchingViewModel.ViewState
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.*
import com.duckduckgo.saved.sites.impl.databinding.*
import dagger.android.support.*
import javax.inject.*
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
Expand All @@ -47,7 +44,8 @@ class FaviconFetchingSyncSetting @JvmOverloads constructor(
@Inject
lateinit var viewModelFactory: FaviconFetchingViewModel.Factory

private var coroutineScope: CoroutineScope? = null
@Inject
lateinit var dispatchers: DispatcherProvider

private var job: ConflatedJob = ConflatedJob()

Expand All @@ -65,19 +63,14 @@ class FaviconFetchingSyncSetting @JvmOverloads constructor(
viewModel.onFaviconFetchingSettingChanged(isChecked)
}

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

job += viewModel.viewState()
.onEach { render(it) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)
}

override fun onDetachedFromWindow() {
super.onDetachedFromWindow()
coroutineScope?.cancel()
job.cancel()
coroutineScope = null
}

private fun render(it: ViewState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

package com.duckduckgo.app.browser.indonesiamessage

import android.annotation.SuppressLint
import android.content.Context
import android.util.AttributeSet
import android.view.View
import android.widget.FrameLayout
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.findViewTreeLifecycleOwner
import androidx.lifecycle.findViewTreeViewModelStoreOwner
import androidx.lifecycle.lifecycleScope
import com.duckduckgo.anvil.annotations.ContributesActivePlugin
import com.duckduckgo.anvil.annotations.InjectWith
import com.duckduckgo.app.browser.R
Expand All @@ -33,16 +33,16 @@ import com.duckduckgo.common.ui.view.MessageCta.Message
import com.duckduckgo.common.ui.view.gone
import com.duckduckgo.common.ui.view.show
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.ViewViewModelFactory
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.di.scopes.ViewScope
import com.duckduckgo.newtabpage.api.NewTabPageSection
import com.duckduckgo.newtabpage.api.NewTabPageSectionPlugin
import dagger.android.support.AndroidSupportInjection
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach

Expand All @@ -56,26 +56,31 @@ class IndonesiaNewTabSectionView @JvmOverloads constructor(
@Inject
lateinit var viewModelFactory: ViewViewModelFactory

private var coroutineScope: CoroutineScope? = null
@Inject
lateinit var dispatchers: DispatcherProvider

private val binding: ViewIndonesiaNewTabSectionBinding by viewBinding()

private val viewModel: IndonesiaNewTabSectionViewModel by lazy {
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[IndonesiaNewTabSectionViewModel::class.java]
}

private val conflatedJob = ConflatedJob()

override fun onAttachedToWindow() {
AndroidSupportInjection.inject(this)
super.onAttachedToWindow()

findViewTreeLifecycleOwner()?.lifecycle?.addObserver(viewModel)

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

viewModel.viewState
conflatedJob += viewModel.viewState
.onEach { render(it) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)
}

override fun onDetachedFromWindow() {
conflatedJob.cancel()
super.onDetachedFromWindow()
}

private fun render(viewState: ViewState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.duckduckgo.app.browser.newtab

import android.annotation.SuppressLint
import android.app.PendingIntent
import android.content.ActivityNotFoundException
import android.content.Context
Expand All @@ -29,6 +28,7 @@ import androidx.core.view.isVisible
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.findViewTreeLifecycleOwner
import androidx.lifecycle.findViewTreeViewModelStoreOwner
import androidx.lifecycle.lifecycleScope
import com.duckduckgo.anvil.annotations.InjectWith
import com.duckduckgo.app.browser.HomeBackgroundLogo
import com.duckduckgo.app.browser.databinding.ViewNewTabLegacyBinding
Expand All @@ -50,6 +50,8 @@ import com.duckduckgo.app.tabs.BrowserNav
import com.duckduckgo.common.ui.view.gone
import com.duckduckgo.common.ui.view.show
import com.duckduckgo.common.ui.viewbinding.viewBinding
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.ViewViewModelFactory
import com.duckduckgo.di.scopes.ViewScope
import com.duckduckgo.mobile.android.app.tracking.ui.AppTrackingProtectionScreens.AppTrackerOnboardingActivityWithEmptyParamsParams
Expand All @@ -58,9 +60,6 @@ import com.duckduckgo.navigation.api.GlobalActivityStarter.DeeplinkActivityParam
import com.duckduckgo.remote.messaging.api.RemoteMessage
import dagger.android.support.AndroidSupportInjection
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
Expand Down Expand Up @@ -88,7 +87,8 @@ class NewTabLegacyPageView @JvmOverloads constructor(
@Inject
lateinit var pixel: Pixel

private var coroutineScope: CoroutineScope? = null
@Inject
lateinit var dispatchers: DispatcherProvider

private val binding: ViewNewTabLegacyBinding by viewBinding()

Expand All @@ -98,30 +98,30 @@ class NewTabLegacyPageView @JvmOverloads constructor(
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[NewTabLegacyPageViewModel::class.java]
}

private val conflatedStateJob = ConflatedJob()
private val conflatedCommandJob = ConflatedJob()

override fun onAttachedToWindow() {
AndroidSupportInjection.inject(this)
super.onAttachedToWindow()

findViewTreeLifecycleOwner()?.lifecycle?.addObserver(viewModel)

@SuppressLint("NoHardcodedCoroutineDispatcher")
coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main)

viewModel.viewState
conflatedStateJob += viewModel.viewState
.onEach { render(it) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)

viewModel.commands()
conflatedCommandJob += viewModel.commands()
.onEach { processCommands(it) }
.launchIn(coroutineScope!!)
.launchIn(findViewTreeLifecycleOwner()?.lifecycleScope!!)
}

override fun onDetachedFromWindow() {
super.onDetachedFromWindow()

findViewTreeLifecycleOwner()?.lifecycle?.removeObserver(viewModel)
coroutineScope?.cancel()
coroutineScope = null
conflatedStateJob.cancel()
conflatedCommandJob.cancel()
}

private fun render(viewState: ViewState) {
Expand Down
Loading

0 comments on commit b682075

Please sign in to comment.