Skip to content
Permalink
Browse files

Fix potential leak due to EpoxyController callbacks

  • Loading branch information...
chrisbanes committed Nov 7, 2019
1 parent 05ae590 commit fb3522d3fbe73962958f5672f75e1929357140ab
@@ -30,7 +30,7 @@ import com.airbnb.epoxy.paging.PagedListEpoxyController

abstract class EntryGridEpoxyController<LI : EntryWithShow<out Entry>> :
PagedListEpoxyController<LI>() {
var state: EntryViewState<LI> by observable(EntryViewState()) { requestForcedModelBuild() }
var state: EntryViewState<LI> by observable(EntryViewState(), ::requestModelBuild)

@Suppress("UselessCallOnCollection")
override fun addModels(models: List<EpoxyModel<*>>) {
@@ -27,15 +27,17 @@ import app.tivi.common.layouts.vertSpacerNormal
import app.tivi.data.Entry
import app.tivi.data.entities.findHighestRatedPoster
import app.tivi.data.resultentities.EntryWithShow
import app.tivi.extensions.observable
import com.airbnb.epoxy.Carousel
import com.airbnb.epoxy.TypedEpoxyController
import com.airbnb.epoxy.EpoxyController
import javax.inject.Inject

class DiscoverEpoxyController @Inject constructor(
private val context: Context,
private val textCreator: DiscoverTextCreator
) : TypedEpoxyController<DiscoverViewState>() {
var callbacks: Callbacks? = null
) : EpoxyController() {
var callbacks: Callbacks? by observable(null, ::requestModelBuild)
var state: DiscoverViewState by observable(DiscoverViewState(), ::requestModelBuild)

interface Callbacks {
fun onTrendingHeaderClicked()
@@ -45,28 +47,28 @@ class DiscoverEpoxyController @Inject constructor(
fun onItemClicked(viewHolderId: Long, item: EntryWithShow<out Entry>)
}

override fun buildModels(viewState: DiscoverViewState) {
val trendingShows = viewState.trendingItems
val popularShows = viewState.popularItems
val recommendedShows = viewState.recommendedItems
override fun buildModels() {
val trendingShows = state.trendingItems
val popularShows = state.popularItems
val recommendedShows = state.recommendedItems

vertSpacerNormal {
id("top_spacer")
}

if (viewState.nextEpisodeWithShowToWatched != null) {
state.nextEpisodeWithShowToWatched?.also { nextEpisodeToWatch ->
header {
id("keep_watching_header")
title(R.string.discover_keep_watching_title)
spanSizeOverride(TotalSpanOverride)
}
discoverNextShowEpisodeToWatch {
id("keep_watching_${viewState.nextEpisodeWithShowToWatched.episode.id}")
id("keep_watching_${nextEpisodeToWatch.episode.id}")
spanSizeOverride(TotalSpanOverride)
episode(viewState.nextEpisodeWithShowToWatched.episode)
season(viewState.nextEpisodeWithShowToWatched.season)
tiviShow(viewState.nextEpisodeWithShowToWatched.show)
posterImage(viewState.nextEpisodeWithShowToWatched.images.findHighestRatedPoster())
episode(nextEpisodeToWatch.episode)
season(nextEpisodeToWatch.season)
tiviShow(nextEpisodeToWatch.show)
posterImage(nextEpisodeToWatch.images.findHighestRatedPoster())
textCreator(textCreator)
clickListener { _ -> callbacks?.onNextEpisodeToWatchClicked() }
}
@@ -80,7 +82,7 @@ class DiscoverEpoxyController @Inject constructor(
header {
id("trending_header")
title(R.string.discover_trending_title)
showProgress(viewState.trendingRefreshing)
showProgress(state.trendingRefreshing)
spanSizeOverride(TotalSpanOverride)
buttonClickListener { _ -> callbacks?.onTrendingHeaderClicked() }
}
@@ -121,7 +123,7 @@ class DiscoverEpoxyController @Inject constructor(
header {
id("recommended_header")
title(R.string.discover_recommended_title)
showProgress(viewState.recommendedRefreshing)
showProgress(state.recommendedRefreshing)
spanSizeOverride(TotalSpanOverride)
buttonClickListener { _ -> callbacks?.onRecommendedHeaderClicked() }
}
@@ -155,7 +157,7 @@ class DiscoverEpoxyController @Inject constructor(
header {
id("popular_header")
title(R.string.discover_popular_title)
showProgress(viewState.popularRefreshing)
showProgress(state.popularRefreshing)
spanSizeOverride(TotalSpanOverride)
buttonClickListener { _ -> callbacks?.onPopularHeaderClicked() }
}
@@ -193,4 +195,8 @@ class DiscoverEpoxyController @Inject constructor(
id("bottom_spacer")
}
}

fun clear() {
callbacks = null
}
}
@@ -176,6 +176,11 @@ class DiscoverFragment : TiviFragmentWithBinding<FragmentDiscoverBinding>() {
authStateMenuItemBinder.bind(state.authState, state.user)

binding.state = state
controller.setData(state)
controller.state = state
}

override fun onDestroyView() {
super.onDestroyView()
controller.clear()
}
}
@@ -105,7 +105,7 @@ class EpisodeDetailsFragment : TiviFragmentWithBinding<FragmentEpisodeDetailsBin
}

override fun invalidate(binding: FragmentEpisodeDetailsBinding) = withState(viewModel) { state ->
binding.state = state
controller.setData(state)
}
binding.state = state
controller.setData(state)
}
}
@@ -37,11 +37,11 @@ import javax.inject.Inject
class FollowedEpoxyController @Inject constructor(
private val textCreator: HomeTextCreator
) : PagedListEpoxyController<FollowedShowEntryWithShow>() {
var viewState by observable(FollowedViewState()) { requestForcedModelBuild() }
var callbacks: Callbacks? by observable(null) { requestForcedModelBuild() }
var state by observable(FollowedViewState(), ::requestModelBuild)
var callbacks: Callbacks? by observable(null, ::requestModelBuild)

override fun addModels(models: List<EpoxyModel<*>>) {
if (viewState.isEmpty) {
if (state.isEmpty) {
emptyState {
id("empty")
spanSizeOverride(TotalSpanOverride)
@@ -52,7 +52,7 @@ class FollowedEpoxyController @Inject constructor(
}
filter {
id("filters")
filter(viewState.filter)
filter(state.filter)
numberShows(models.size)
watcher(object : TextWatcher {
override fun afterTextChanged(s: Editable?) {
@@ -64,7 +64,7 @@ class FollowedEpoxyController @Inject constructor(
override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {}
})

popupMenuListener(SortPopupMenuListener(viewState.sort, viewState.availableSorts))
popupMenuListener(SortPopupMenuListener(state.sort, state.availableSorts))
popupMenuClickListener {
val option = popupMenuItemIdToSortOption(it.itemId)
?: throw IllegalArgumentException("Selected sort option is null")
@@ -88,7 +88,7 @@ class FollowedEpoxyController @Inject constructor(
tiviShow(item.show)
posterImage(item.images.findHighestRatedPoster())
posterTransitionName("show_${item.show.homepage}")
selected(item.show.id in viewState.selectedShowIds)
selected(item.show.id in state.selectedShowIds)
callbacks?.also { cb ->
clickListener(View.OnClickListener { cb.onItemClicked(item) })
longClickListener(View.OnLongClickListener { cb.onItemLongClicked(item) })
@@ -102,6 +102,10 @@ class FollowedEpoxyController @Inject constructor(
}
}

fun clear() {
callbacks = null
}

interface Callbacks {
fun onItemClicked(item: FollowedShowEntryWithShow)
fun onItemLongClicked(item: FollowedShowEntryWithShow): Boolean
@@ -146,14 +146,15 @@ class FollowedFragment : TiviFragmentWithBinding<FragmentFollowedBinding>() {

if (state.followedShows != null) {
// PagingEpoxyController does not like being updated before it has a list
controller.viewState = state
controller.state = state
controller.submitList(state.followedShows)
}
}

override fun onDestroyView() {
super.onDestroyView()
currentActionMode?.finish()
controller.clear()
}

private fun startSelectionActionMode() {
@@ -18,31 +18,36 @@ package app.tivi.home.search

import app.tivi.data.entities.TiviShow
import app.tivi.home.HomeTextCreator
import app.tivi.common.epoxy.EpoxyModelProperty
import app.tivi.extensions.observable
import com.airbnb.epoxy.EpoxyController
import dagger.Lazy
import javax.inject.Inject

class SearchEpoxyController @Inject constructor(
private val textCreator: HomeTextCreator
private val textCreator: Lazy<HomeTextCreator>
) : EpoxyController() {
var callbacks by EpoxyModelProperty<Callbacks?> { null }
var viewState by EpoxyModelProperty { SearchViewState() }
var callbacks: Callbacks? by observable(null, ::requestModelBuild)
var state by observable(SearchViewState(), ::requestModelBuild)

interface Callbacks {
fun onSearchItemClicked(show: TiviShow)
}

override fun buildModels() {
val searchResult = viewState.searchResults
val searchResult = state.searchResults

searchResult?.results?.forEach { showDetailed ->
searchItemShow {
id(showDetailed.show.id)
tiviShow(showDetailed.show)
posterImage(showDetailed.poster)
textCreator(textCreator)
textCreator(textCreator.get())
clickListener { _ -> callbacks?.onSearchItemClicked(showDetailed.show) }
}
}
}

fun clear() {
callbacks = null
}
}
@@ -94,6 +94,11 @@ internal class SearchFragment : TiviFragmentWithBinding<FragmentSearchBinding>()

override fun invalidate(binding: FragmentSearchBinding) = withState(viewModel) { state ->
binding.state = state
controller.viewState = state
controller.state = state
}

override fun onDestroyView() {
super.onDestroyView()
controller.clear()
}
}
@@ -34,13 +34,14 @@ import app.tivi.data.entities.findHighestRatedPoster
import app.tivi.data.resultentities.RelatedShowEntryWithShow
import app.tivi.data.resultentities.SeasonWithEpisodesAndWatches
import app.tivi.data.views.FollowedShowsWatchStats
import app.tivi.extensions.observable
import app.tivi.inject.PerActivity
import app.tivi.showdetails.details.databinding.ViewHolderDetailsSeasonBinding
import app.tivi.ui.widget.PopupMenuButton
import com.airbnb.epoxy.Carousel
import com.airbnb.epoxy.EpoxyController
import com.airbnb.epoxy.EpoxyModelGroup
import com.airbnb.epoxy.IdUtils
import com.airbnb.epoxy.TypedEpoxyController
import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Success
import javax.inject.Inject
@@ -49,9 +50,9 @@ import kotlin.math.roundToInt
class ShowDetailsEpoxyController @Inject constructor(
@PerActivity private val context: Context,
private val textCreator: ShowDetailsTextCreator
) : TypedEpoxyController<ShowDetailsViewState>() {

var callbacks: Callbacks? = null
) : EpoxyController() {
var state by observable(ShowDetailsViewState(), ::requestModelBuild)
var callbacks: Callbacks? by observable(null, ::requestModelBuild)

interface Callbacks {
fun onRelatedShowClicked(show: TiviShow, itemView: View)
@@ -65,10 +66,10 @@ class ShowDetailsEpoxyController @Inject constructor(
fun onMarkPreviousSeasonsIgnored(season: Season)
}

override fun buildModels(viewState: ShowDetailsViewState) {
buildShowModels(viewState.show)
override fun buildModels() {
buildShowModels(state.show)

val episodeWithSeason = viewState.nextEpisodeToWatch()
val episodeWithSeason = state.nextEpisodeToWatch()
if (episodeWithSeason?.episode != null) {
detailsHeader {
id("next_episode_header")
@@ -87,9 +88,9 @@ class ShowDetailsEpoxyController @Inject constructor(
}
}

buildRelatedShowsModels(viewState.relatedShows)
buildRelatedShowsModels(state.relatedShows)

buildSeasonsModels(viewState.viewStats, viewState.seasons, viewState.expandedSeasonIds)
buildSeasonsModels(state.viewStats, state.seasons, state.expandedSeasonIds)
}

private fun buildShowModels(show: TiviShow) {
@@ -325,4 +326,8 @@ class ShowDetailsEpoxyController @Inject constructor(
return true
}
}

fun clear() {
callbacks = null
}
}
@@ -227,7 +227,12 @@ class ShowDetailsFragment : TiviFragmentWithBinding<FragmentShowDetailsBinding>(
}

binding.state = it
controller.setData(it)
controller.state = it
}
}

override fun onDestroyView() {
super.onDestroyView()
controller.clear()
}
}
@@ -27,7 +27,7 @@ import com.airbnb.mvrx.MvRxState
import com.airbnb.mvrx.Uninitialized

data class ShowDetailsViewState(
val showId: Long,
val showId: Long = 0,
val isFollowed: Boolean = false,
val show: TiviShow = TiviShow.EMPTY_SHOW,
val posterImage: ShowTmdbImage? = null,
@@ -39,11 +39,11 @@ class WatchedEpoxyController @Inject constructor(
private val textCreator: HomeTextCreator,
private val dateFormatter: TiviDateFormatter
) : PagedListEpoxyController<WatchedShowEntryWithShow>() {
var viewState by observable(WatchedViewState()) { requestForcedModelBuild() }
var callbacks: Callbacks? by observable(null) { requestForcedModelBuild() }
var state by observable(WatchedViewState(), ::requestModelBuild)
var callbacks: Callbacks? by observable(null, ::requestModelBuild)

override fun addModels(models: List<EpoxyModel<*>>) {
if (viewState.isEmpty) {
if (state.isEmpty) {
emptyState {
id("empty")
spanSizeOverride(TotalSpanOverride)
@@ -55,7 +55,7 @@ class WatchedEpoxyController @Inject constructor(

filter {
id("filters")
filter(viewState.filter)
filter(state.filter)
numberShows(models.size)
watcher(object : TextWatcher {
override fun afterTextChanged(s: Editable?) {
@@ -67,7 +67,7 @@ class WatchedEpoxyController @Inject constructor(
override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {}
})

popupMenuListener(SortPopupMenuListener(viewState.sort, viewState.availableSorts))
popupMenuListener(SortPopupMenuListener(state.sort, state.availableSorts))
popupMenuClickListener {
val option = popupMenuItemIdToSortOption(it.itemId)
?: throw IllegalArgumentException("Selected sort option is null")
@@ -91,7 +91,7 @@ class WatchedEpoxyController @Inject constructor(
tiviShow(item.show)
posterImage(item.images.findHighestRatedPoster())
posterTransitionName("show_${item.show.homepage}")
selected(item.show.id in viewState.selectedShowIds)
selected(item.show.id in state.selectedShowIds)
callbacks?.also { cb ->
clickListener(View.OnClickListener { cb.onItemClicked(item) })
longClickListener(View.OnLongClickListener { cb.onItemLongClicked(item) })
@@ -105,6 +105,10 @@ class WatchedEpoxyController @Inject constructor(
}
}

fun clear() {
callbacks = null
}

interface Callbacks {
fun onItemClicked(item: WatchedShowEntryWithShow)
fun onItemLongClicked(item: WatchedShowEntryWithShow): Boolean

0 comments on commit fb3522d

Please sign in to comment.
You can’t perform that action at this time.