From 4f717411077575869ccb670083b7be6b19c3a528 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Wed, 27 Jul 2022 17:11:17 +0200 Subject: [PATCH 1/8] Fixing leak on OnSymbolClickListener --- .../location/live/map/LiveLocationMapViewFragment.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git 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 index c258de82ac..d89977d7af 100644 --- 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 @@ -73,6 +74,7 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment() 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 +110,11 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment @@ -117,10 +124,10 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment Date: Wed, 27 Jul 2022 17:25:51 +0200 Subject: [PATCH 2/8] Fixing leak on bottomSheetController callback --- .../location/live/map/LiveLocationBottomSheetController.kt | 1 - .../features/location/live/map/LiveLocationMapViewFragment.kt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git 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 index e8a6b22b7e..55ddd3aae0 100644 --- 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 @@ -33,7 +33,6 @@ class LiveLocationBottomSheetController @Inject constructor( private val vectorDateFormatter: VectorDateFormatter, private val stringProvider: StringProvider, private val clock: Clock, - private val context: Context, ) : EpoxyController() { interface Callback { diff --git 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 index d89977d7af..31d15750a7 100644 --- 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 @@ -112,6 +112,7 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment Date: Thu, 28 Jul 2022 09:50:07 +0200 Subject: [PATCH 3/8] Fixing missing cleanUp on RecyclerView --- .../features/location/LocationSharingActivity.kt | 2 +- .../map/LiveLocationBottomSheetController.kt | 1 - .../live/map/LiveLocationMapViewFragment.kt | 6 ++++-- .../location/live/map/LiveLocationUserItem.kt | 16 ++++++---------- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt index 169af4a5a2..2c97913a89 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt @@ -35,7 +35,7 @@ data class LocationSharingArgs( @AndroidEntryPoint class LocationSharingActivity : VectorBaseActivity() { - +// TODO fix leak of mapTilerView override fun getBinding() = ActivityLocationSharingBinding.inflate(layoutInflater) override fun initUiAndData() { diff --git 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 index 55ddd3aae0..0616ea84d9 100644 --- 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 diff --git 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 index 31d15750a7..1e1faa26f5 100644 --- 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 @@ -43,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 @@ -110,10 +111,11 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment(R.id.itemUserAvatarImageView) val itemUserDisplayNameTextView by bind(R.id.itemUserDisplayNameTextView) val itemRemainingTimeTextView by bind(R.id.itemRemainingTimeTextView) From f44d8b0b2040f972df76e36a31f89da064e7ea1f Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 28 Jul 2022 14:42:57 +0200 Subject: [PATCH 4/8] Fixing missing call to SymbolManager.onDestroy() --- .../vector/app/features/location/LocationSharingActivity.kt | 2 +- .../java/im/vector/app/features/location/MapTilerMapView.kt | 6 ++++++ .../location/live/map/LiveLocationMapViewFragment.kt | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt index 2c97913a89..169af4a5a2 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingActivity.kt @@ -35,7 +35,7 @@ data class LocationSharingArgs( @AndroidEntryPoint class LocationSharingActivity : VectorBaseActivity() { -// TODO fix leak of mapTilerView + override fun getBinding() = ActivityLocationSharingBinding.inflate(layoutInflater) override fun initUiAndData() { diff --git a/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt b/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt index 491386ba64..698f8874b3 100644 --- a/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt +++ b/vector/src/main/java/im/vector/app/features/location/MapTilerMapView.kt @@ -76,6 +76,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. */ diff --git 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 index 1e1faa26f5..283774dbc6 100644 --- 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 @@ -113,6 +113,7 @@ class LiveLocationMapViewFragment @Inject constructor() : VectorBaseFragment Date: Thu, 28 Jul 2022 14:50:13 +0200 Subject: [PATCH 5/8] Fixing missing call to timer.cancel() when view is detached --- .../features/location/live/LiveLocationRunningBannerView.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git 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 index 539be1a8c9..0872250abb 100644 --- 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() + } } From e311d0e46927e53516bbdcfd6c314b0bf653f2b4 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 28 Jul 2022 15:26:38 +0200 Subject: [PATCH 6/8] Fixing missing cleanUp of the LocationSharingAndroidService --- .../LocationSharingAndroidServiceBinder.kt | 34 +++++++++++++++++++ .../LocationSharingServiceConnection.kt | 2 +- .../tracking/LocationSharingAndroidService.kt | 12 +++---- 3 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt new file mode 100644 index 0000000000..6d791365f7 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/location/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 + +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 +} diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt index 9e905060d9..7e873a3716 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt @@ -67,7 +67,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 diff --git 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 index 6babc94385..819ec8634e 100644 --- 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() 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) } From 1ef809c63355e7a8f63e5e68eda90b50b5eb9940 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Thu, 28 Jul 2022 16:04:25 +0200 Subject: [PATCH 7/8] Adding changelog entry --- changelog.d/6674.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6674.misc diff --git a/changelog.d/6674.misc b/changelog.d/6674.misc new file mode 100644 index 0000000000..830d528e27 --- /dev/null +++ b/changelog.d/6674.misc @@ -0,0 +1 @@ +[Location sharing] - Fix the memory leaks From c926b4cc6993930552c0b540c6ee6a83c4ba8eae Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 2 Aug 2022 10:47:38 +0200 Subject: [PATCH 8/8] Fix moving some components to dedicated package --- .../vector/app/features/home/room/detail/TimelineViewModel.kt | 2 +- .../app/features/location/live/map/LiveLocationMapViewModel.kt | 2 +- .../{ => live/tracking}/LocationSharingAndroidServiceBinder.kt | 2 +- .../{ => live/tracking}/LocationSharingServiceConnection.kt | 3 +-- .../app/test/fakes/FakeLocationSharingServiceConnection.kt | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) rename vector/src/main/java/im/vector/app/features/location/{ => live/tracking}/LocationSharingAndroidServiceBinder.kt (94%) rename vector/src/main/java/im/vector/app/features/location/{ => live/tracking}/LocationSharingServiceConnection.kt (96%) diff --git 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 index 848bd3aed4..9e31c737bf 100644 --- 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 diff --git 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 index 9788a19918..280789e4c6 100644 --- 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 diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidServiceBinder.kt similarity index 94% rename from vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt rename to vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidServiceBinder.kt index 6d791365f7..5298e75f08 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidServiceBinder.kt +++ b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingAndroidServiceBinder.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.os.Binder diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt b/vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingServiceConnection.kt similarity index 96% rename from vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt rename to vector/src/main/java/im/vector/app/features/location/live/tracking/LocationSharingServiceConnection.kt index 7e873a3716..9bbf7980fa 100644 --- a/vector/src/main/java/im/vector/app/features/location/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 diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationSharingServiceConnection.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationSharingServiceConnection.kt index db27a894f9..a42ce78b15 100644 --- 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