Merge pull request #6676 from vector-im/fix/mna/fix-memory-leaks-lls

[Location sharing] - Fix the memory leaks (PSG-656)
2024-11-22 09:25:49 +03:00 · 2022-08-02 18:01:48 +02:00 · 2022-08-02 18:01:48 +02:00 · ac8597e745
commit ac8597e745
parent e2ed4b4ae1 c926b4cc69
12 changed files with 75 additions and 27 deletions
--- a/changelog.d/6674.misc
+++ b/changelog.d/6674.misc
@ -0,0 +1 @@
+[Location sharing] - Fix the memory leaks
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt
@ -52,8 +52,8 @@ import im.vector.app.features.home.room.detail.sticker.StickerPickerActionHandle
 import im.vector.app.features.home.room.detail.timeline.factory.TimelineFactory
 import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever
 import im.vector.app.features.home.room.typing.TypingHelper
-import im.vector.app.features.location.LocationSharingServiceConnection
 import im.vector.app.features.location.live.StopLiveLocationShareUseCase
+import im.vector.app.features.location.live.tracking.LocationSharingServiceConnection
 import im.vector.app.features.notifications.NotificationDrawerManager
 import im.vector.app.features.powerlevel.PowerLevelsFlowFactory
 import im.vector.app.features.raw.wellknown.getOutboundSessionKeySharingStrategyOrDefault
--- a/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt
+++ b/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt
@ -77,6 +77,12 @@ class MapTilerMapView @JvmOverloads constructor(
        showLocationButton = typedArray.getBoolean(R.styleable.MapTilerMapView_showLocateButton, false)
    }

+    override fun onDestroy() {
+        mapRefs?.symbolManager?.onDestroy()
+        mapRefs = null
+        super.onDestroy()
+    }
+
    /**
     * For location fragments.
     */
--- a/vector/src/main/java/im/vector/app/features/location/live/LiveLocationRunningBannerView.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/LiveLocationRunningBannerView.kt
@ -128,4 +128,9 @@ class LiveLocationRunningBannerView @JvmOverloads constructor(
        title.text = context.getString(R.string.location_share_live_view)
        subTitle.text = context.getString(R.string.location_share_live_until, viewState.formattedLocalTimeOfEndOfLive)
    }
+
+    override fun onDetachedFromWindow() {
+        countDownTimer?.cancel()
+        super.onDetachedFromWindow()
+    }
 }
--- a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationBottomSheetController.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationBottomSheetController.kt
@ -16,7 +16,6 @@

 package im.vector.app.features.location.live.map

-import android.content.Context
 import com.airbnb.epoxy.EpoxyController
 import im.vector.app.R
 import im.vector.app.core.date.DateFormatKind
@ -33,7 +32,6 @@ class LiveLocationBottomSheetController @Inject constructor(
        private val vectorDateFormatter: VectorDateFormatter,
        private val stringProvider: StringProvider,
        private val clock: Clock,
-        private val context: Context,
 ) : EpoxyController() {

    interface Callback {
--- a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationMapViewFragment.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationMapViewFragment.kt
@ -35,6 +35,7 @@ import com.mapbox.mapboxsdk.maps.MapboxMap
 import com.mapbox.mapboxsdk.maps.MapboxMapOptions
 import com.mapbox.mapboxsdk.maps.Style
 import com.mapbox.mapboxsdk.maps.SupportMapFragment
+import com.mapbox.mapboxsdk.plugins.annotation.OnSymbolClickListener
 import com.mapbox.mapboxsdk.plugins.annotation.Symbol
 import com.mapbox.mapboxsdk.plugins.annotation.SymbolManager
 import com.mapbox.mapboxsdk.plugins.annotation.SymbolOptions
@ -42,6 +43,7 @@ import com.mapbox.mapboxsdk.style.layers.Property
 import dagger.hilt.android.AndroidEntryPoint
 import im.vector.app.R
 import im.vector.app.core.extensions.addChildFragment
+import im.vector.app.core.extensions.cleanup
 import im.vector.app.core.extensions.configureWith
 import im.vector.app.core.platform.VectorBaseFragment
 import im.vector.app.core.utils.DimensionConverter
@ -73,6 +75,7 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment<Fra
    private var mapStyle: Style? = null
    private val pendingLiveLocations = mutableListOf<UserLiveLocationViewState>()
    private var isMapFirstUpdate = true
+    private var onSymbolClickListener: OnSymbolClickListener? = null

    override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentLiveLocationMapViewBinding {
        return FragmentLiveLocationMapViewBinding.inflate(layoutInflater, container, false)
@ -108,6 +111,14 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment<Fra
        setupMap()
    }

+    override fun onDestroyView() {
+        onSymbolClickListener?.let { symbolManager?.removeClickListener(it) }
+        symbolManager?.onDestroy()
+        bottomSheetController.callback = null
+        views.liveLocationBottomSheetRecyclerView.cleanup()
+        super.onDestroyView()
+    }
+
    private fun setupMap() {
        val mapFragment = getOrCreateSupportMapFragment()
        mapFragment.getMapAsync { mapboxMap ->
@ -117,10 +128,10 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment<Fra
                    this@LiveLocationMapViewFragment.mapboxMap = WeakReference(mapboxMap)
                    symbolManager = SymbolManager(mapFragment.view as MapView, mapboxMap, style).apply {
                        iconAllowOverlap = true
-                        addClickListener {
+                        onSymbolClickListener = OnSymbolClickListener {
                            onSymbolClicked(it)
                            true
-                        }
+                        }.also { addClickListener(it) }
                    }
                    pendingLiveLocations
                            .takeUnless { it.isEmpty() }
--- a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationMapViewModel.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationMapViewModel.kt
@ -23,8 +23,8 @@ import dagger.assisted.AssistedInject
 import im.vector.app.core.di.MavericksAssistedViewModelFactory
 import im.vector.app.core.di.hiltMavericksViewModelFactory
 import im.vector.app.core.platform.VectorViewModel
-import im.vector.app.features.location.LocationSharingServiceConnection
 import im.vector.app.features.location.live.StopLiveLocationShareUseCase
+import im.vector.app.features.location.live.tracking.LocationSharingServiceConnection
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
--- a/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/map/LiveLocationUserItem.kt
@ -79,15 +79,12 @@ abstract class LiveLocationUserItem : VectorEpoxyModel<LiveLocationUserItem.Hold
            }
        }

-        stopTimer(holder)
-        holder.timer = CountUpTimer(1000).apply {
-            tickListener = object : CountUpTimer.TickListener {
+        holder.timer.tickListener = object : CountUpTimer.TickListener {
            override fun onTick(milliseconds: Long) {
                holder.itemLastUpdatedAtTextView.text = getFormattedLastUpdatedAt(locationUpdateTimeMillis)
            }
        }
-            resume()
-        }
+        holder.timer.resume()

        holder.view.setOnClickListener { callback?.onUserSelected(matrixItem.id) }
    }
@ -98,8 +95,7 @@ abstract class LiveLocationUserItem : VectorEpoxyModel<LiveLocationUserItem.Hold
    }

    private fun stopTimer(holder: Holder) {
-        holder.timer?.stop()
-        holder.timer = null
+        holder.timer.stop()
    }

    private fun getFormattedLastUpdatedAt(locationUpdateTimeMillis: Long?): String {
@ -111,7 +107,7 @@ abstract class LiveLocationUserItem : VectorEpoxyModel<LiveLocationUserItem.Hold
    }

    class Holder : VectorEpoxyHolder() {
-        var timer: CountUpTimer? = null
+        val timer: CountUpTimer = CountUpTimer(1000)
        val itemUserAvatarImageView by bind<ImageView>(R.id.itemUserAvatarImageView)
        val itemUserDisplayNameTextView by bind<TextView>(R.id.itemUserDisplayNameTextView)
        val itemRemainingTimeTextView by bind<TextView>(R.id.itemRemainingTimeTextView)
--- a/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidService.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidService.kt
@ -17,7 +17,6 @@
 package im.vector.app.features.location.live.tracking

 import android.content.Intent
-import android.os.Binder
 import android.os.IBinder
 import android.os.Parcelable
 import androidx.core.app.NotificationManagerCompat
@ -62,7 +61,7 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca
    @Inject lateinit var getLiveLocationShareSummaryUseCase: GetLiveLocationShareSummaryUseCase
    @Inject lateinit var checkIfEventIsRedactedUseCase: CheckIfEventIsRedactedUseCase

-    private val binder = LocalBinder()
+    private var binder: LocationSharingAndroidServiceBinder? = null

    private val liveInfoSet = linkedSetOf<LiveInfo>()
    var callback: Callback? = null
@ -76,6 +75,7 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca
    override fun onCreate() {
        super.onCreate()
        Timber.i("onCreate")
+        binder = LocationSharingAndroidServiceBinder().also { it.setup(this) }
        initLocationTracking()
    }

@ -205,6 +205,8 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca
    override fun onDestroy() {
        super.onDestroy()
        Timber.i("onDestroy")
+        binder?.cleanUp()
+        binder = null
        jobs.forEach { it.cancel() }
        jobs.clear()
        locationTracker.removeCallback(this)
@ -251,14 +253,10 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca
        return liveInfoSet.map { it.roomArgs.roomId }.toSet()
    }

-    override fun onBind(intent: Intent?): IBinder {
+    override fun onBind(intent: Intent?): IBinder? {
        return binder
    }

-    inner class LocalBinder : Binder() {
-        fun getService(): LocationSharingAndroidService = this@LocationSharingAndroidService
-    }
-
    interface Callback {
        fun onServiceError(error: Throwable)
    }
--- a/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidServiceBinder.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidServiceBinder.kt
@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.location.live.tracking
+
+import android.os.Binder
+
+class LocationSharingAndroidServiceBinder : Binder() {
+
+    private var locationSharingAndroidService: LocationSharingAndroidService? = null
+
+    fun setup(service: LocationSharingAndroidService) {
+        locationSharingAndroidService = service
+    }
+
+    fun cleanUp() {
+        locationSharingAndroidService = null
+    }
+
+    fun getService() = locationSharingAndroidService
+}
--- a/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingServiceConnection.kt
+++ b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingServiceConnection.kt
@ -14,7 +14,7 @@
 * limitations under the License.
 */

-package im.vector.app.features.location
+package im.vector.app.features.location.live.tracking

 import android.content.ComponentName
 import android.content.Context
@ -22,7 +22,6 @@ import android.content.Intent
 import android.content.ServiceConnection
 import android.os.IBinder
 import im.vector.app.core.di.ActiveSessionHolder
-import im.vector.app.features.location.live.tracking.LocationSharingAndroidService
 import im.vector.app.features.session.coroutineScope
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.launchIn
@ -67,7 +66,7 @@ class LocationSharingServiceConnection @Inject constructor(
    }

    override fun onServiceConnected(className: ComponentName, binder: IBinder) {
-        locationSharingAndroidService = (binder as LocationSharingAndroidService.LocalBinder).getService().also { service ->
+        locationSharingAndroidService = (binder as LocationSharingAndroidServiceBinder).getService()?.also { service ->
            service.callback = this
            getActiveSessionCoroutineScope()?.let { scope ->
                service.roomIdsOfActiveLives
--- a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationSharingServiceConnection.kt
+++ b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationSharingServiceConnection.kt
@ -16,7 +16,7 @@

 package im.vector.app.test.fakes

-import im.vector.app.features.location.LocationSharingServiceConnection
+import im.vector.app.features.location.live.tracking.LocationSharingServiceConnection
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk