From 245a11c3596704908dc298b5f5c40c66568ad824 Mon Sep 17 00:00:00 2001 From: Kiryl Dzehtsiarenka Date: Tue, 28 Sep 2021 11:59:58 +0300 Subject: [PATCH 1/2] Revisit links to native objects --- sdk/src/main/java/com/mapbox/maps/MapController.kt | 4 ++-- sdk/src/main/java/com/mapbox/maps/MapProvider.kt | 5 +++-- sdk/src/main/java/com/mapbox/maps/MapboxMap.kt | 5 ++--- sdk/src/main/java/com/mapbox/maps/Snapshotter.kt | 12 ++++++++++-- sdk/src/main/java/com/mapbox/maps/Style.kt | 6 +----- sdk/src/main/java/com/mapbox/maps/StyleObserver.kt | 5 ++--- .../com/mapbox/maps/renderer/MapboxRenderThread.kt | 1 + 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/sdk/src/main/java/com/mapbox/maps/MapController.kt b/sdk/src/main/java/com/mapbox/maps/MapController.kt index d9924884c4..01fb81ab8c 100644 --- a/sdk/src/main/java/com/mapbox/maps/MapController.kt +++ b/sdk/src/main/java/com/mapbox/maps/MapController.kt @@ -61,9 +61,9 @@ internal class MapController : MapPluginProviderDelegate, MapControllable { mapInitOptions, renderer, ) - this.nativeObserver = NativeObserver(WeakReference(nativeMap)) + this.nativeObserver = NativeObserver(WeakReference(nativeMap as ObservableInterface)) this.mapboxMap = - MapProvider.getMapboxMap(nativeMap, nativeObserver, mapInitOptions.mapOptions.pixelRatio) + MapProvider.getMapboxMap(WeakReference(nativeMap), nativeObserver, mapInitOptions.mapOptions.pixelRatio) this.mapboxMap.renderHandler = renderer.renderThread.handlerThread.handler this.pluginRegistry = MapProvider.getMapPluginRegistry( mapboxMap, diff --git a/sdk/src/main/java/com/mapbox/maps/MapProvider.kt b/sdk/src/main/java/com/mapbox/maps/MapProvider.kt index eb4ec347ff..aea2dc23dc 100644 --- a/sdk/src/main/java/com/mapbox/maps/MapProvider.kt +++ b/sdk/src/main/java/com/mapbox/maps/MapProvider.kt @@ -3,6 +3,7 @@ package com.mapbox.maps import com.mapbox.maps.module.MapTelemetry import com.mapbox.maps.plugin.MapDelegateProviderImpl import com.mapbox.maps.plugin.MapPluginRegistry +import java.lang.ref.WeakReference internal object MapProvider { @@ -18,11 +19,11 @@ internal object MapProvider { ) fun getMapboxMap( - nativeMap: MapInterface, + nativeMapWeakRef: WeakReference, nativeObserver: NativeObserver, pixelRatio: Float ) = - MapboxMap(nativeMap, nativeObserver, pixelRatio) + MapboxMap(nativeMapWeakRef, nativeObserver, pixelRatio) fun getMapPluginRegistry( mapboxMap: MapboxMap, diff --git a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt index 0943d29a63..fceae0f830 100644 --- a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt +++ b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt @@ -35,7 +35,7 @@ import java.util.* * @property style the map style. */ class MapboxMap internal constructor( - nativeMap: MapInterface, + internal val nativeMapWeakRef: WeakReference, private val nativeObserver: NativeObserver, pixelRatio: Float ) : MapTransformDelegate, @@ -47,10 +47,9 @@ class MapboxMap internal constructor( MapCameraManagerDelegate, MapStyleStateDelegate { - private val nativeMapWeakRef = WeakReference(nativeMap) internal lateinit var style: Style internal var isStyleLoadInitiated = false - private val styleObserver = StyleObserver(this, nativeMapWeakRef, nativeObserver, pixelRatio) + private val styleObserver = StyleObserver(this, nativeObserver, pixelRatio) internal var renderHandler: Handler? = null internal var styleLoaded = false diff --git a/sdk/src/main/java/com/mapbox/maps/Snapshotter.kt b/sdk/src/main/java/com/mapbox/maps/Snapshotter.kt index 590e1534c7..9dc704a5f6 100644 --- a/sdk/src/main/java/com/mapbox/maps/Snapshotter.kt +++ b/sdk/src/main/java/com/mapbox/maps/Snapshotter.kt @@ -57,11 +57,19 @@ open class Snapshotter { coreSnapshotter.unsubscribe(observer) } MapEvents.STYLE_DATA_LOADED -> if (event.getStyleDataLoadedEventData().styleDataType == StyleDataType.STYLE) { - snapshotStyleCallback?.onDidFinishLoadingStyle(Style(coreSnapshotter, pixelRatio)) + snapshotStyleCallback?.onDidFinishLoadingStyle( + Style( + WeakReference(coreSnapshotter as StyleManagerInterface), + pixelRatio + ) + ) } MapEvents.STYLE_LOADED -> { snapshotStyleCallback?.onDidFullyLoadStyle( - Style(coreSnapshotter, pixelRatio) + Style( + WeakReference(coreSnapshotter as StyleManagerInterface), + pixelRatio + ) ) coreSnapshotter.unsubscribe(observer) } diff --git a/sdk/src/main/java/com/mapbox/maps/Style.kt b/sdk/src/main/java/com/mapbox/maps/Style.kt index 5c0a4139d2..f87c78369d 100644 --- a/sdk/src/main/java/com/mapbox/maps/Style.kt +++ b/sdk/src/main/java/com/mapbox/maps/Style.kt @@ -18,16 +18,12 @@ import java.nio.ByteBuffer * Note: Similar to a View object, a [Style] should only be read and modified * from the main thread. * - * - * @property fullyLoaded true is style is fully loaded, false if a new style is being loaded * @property pixelRatio the scale ratio of the style, default the device pixel ratio */ class Style internal constructor( - styleManager: StyleManagerInterface, + private val styleManagerRef: WeakReference, override val pixelRatio: Float ) : StyleInterface { - private val styleManagerRef = WeakReference(styleManager) - /** * Subscribes an Observer to a provided list of event types. * Observable will hold a strong reference to an Observer instance, therefore, diff --git a/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt b/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt index 7e47a1850a..8d772fda0c 100644 --- a/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt +++ b/sdk/src/main/java/com/mapbox/maps/StyleObserver.kt @@ -13,7 +13,6 @@ import java.util.concurrent.CopyOnWriteArrayList */ internal class StyleObserver( private val mapboxMap: MapboxMap, - private val nativeMap: WeakReference, private val nativeObserver: NativeObserver, private val pixelRatio: Float ) : OnStyleLoadedListener, OnMapLoadErrorListener { @@ -48,8 +47,8 @@ internal class StyleObserver( * Invoked when a style has loaded */ override fun onStyleLoaded() { - nativeMap.get()?.let { - mapboxMap.style = Style(it as StyleManagerInterface, pixelRatio) + mapboxMap.nativeMapWeakRef.get()?.let { + mapboxMap.style = Style(WeakReference(it as StyleManagerInterface), pixelRatio) val iterator = awaitingStyleLoadListeners.iterator() while (iterator.hasNext()) { iterator.next().onStyleLoaded(mapboxMap.style) diff --git a/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt b/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt index b47e5f638d..56e983c763 100644 --- a/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt +++ b/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt @@ -390,6 +390,7 @@ internal class MapboxRenderThread : Choreographer.FrameCallback { } } handlerThread.stop() + mapboxRenderer.map = null } companion object { From 1ee0653d01e880f4c9a8d438dac4a17f71595ca8 Mon Sep 17 00:00:00 2001 From: Kiryl Dzehtsiarenka Date: Thu, 30 Sep 2021 14:30:29 +0300 Subject: [PATCH 2/2] Fix tests --- .../java/com/mapbox/maps/MapControllerTest.kt | 3 +- .../java/com/mapbox/maps/MapboxMapTest.kt | 2 +- .../java/com/mapbox/maps/StyleObserverTest.kt | 35 ++++++++++--------- .../test/java/com/mapbox/maps/StyleTest.kt | 3 +- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt b/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt index 0cee135006..00ec5f8998 100644 --- a/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt @@ -19,6 +19,7 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import java.io.File +import java.lang.ref.WeakReference @RunWith(RobolectricTestRunner::class) @Config(shadows = [ShadowLogger::class]) @@ -76,7 +77,7 @@ class MapControllerTest { } answers { nativeMap } every { nativeMap.cameraState } returns cameraState - every { MapProvider.getMapboxMap(nativeMap, nativeObserver, 1.0f) } answers { mapboxMap } + every { MapProvider.getMapboxMap(WeakReference(nativeMap), nativeObserver, 1.0f) } answers { mapboxMap } every { MapProvider.getMapPluginRegistry(any(), any(), any()) } returns pluginRegistry every { mapboxMap.isStyleLoadInitiated } returns false every { renderer.renderThread.handlerThread.handler } returns mockk() diff --git a/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt b/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt index 5467dc16d0..cc8632cae1 100644 --- a/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt @@ -40,7 +40,7 @@ class MapboxMapTest { mockkStatic(Map::class) every { Map.clearData(any(), any()) } just runs every { nativeMap.resourceOptions } returns resourceOptions - mapboxMap = MapboxMap(nativeMap, nativeObserver, 1.0f) + mapboxMap = MapboxMap(WeakReference(nativeMap), nativeObserver, 1.0f) } @Test diff --git a/sdk/src/test/java/com/mapbox/maps/StyleObserverTest.kt b/sdk/src/test/java/com/mapbox/maps/StyleObserverTest.kt index 42d7ff3d68..3ff75fea35 100644 --- a/sdk/src/test/java/com/mapbox/maps/StyleObserverTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/StyleObserverTest.kt @@ -5,7 +5,6 @@ import com.mapbox.maps.plugin.delegates.listeners.OnMapLoadErrorListener import com.mapbox.maps.plugin.delegates.listeners.eventdata.MapLoadErrorType import io.mockk.mockk import io.mockk.verify -import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -16,21 +15,13 @@ import java.lang.ref.WeakReference @Config(shadows = [ShadowLogger::class]) class StyleObserverTest { - lateinit var map: WeakReference - - @Before - fun setUp() { - val mapInterface = mockk() - map = WeakReference(mapInterface) - } - /** * Verifies if the correct listeners are attached to NativeMapObserver when StyleObserver is created */ @Test fun onStyleObserverCreate() { val nativeObserver = mockk(relaxed = true) - StyleObserver(mockk(), mockk(), nativeObserver, 1.0f) + StyleObserver(mockk(), nativeObserver, 1.0f) verify { nativeObserver.addOnStyleLoadedListener(any()) } verify { nativeObserver.addOnMapLoadErrorListener(any()) } } @@ -41,7 +32,7 @@ class StyleObserverTest { @Test fun onStyleObserverDestroy() { val nativeObserver = mockk(relaxed = true) - StyleObserver(mockk(), mockk(), nativeObserver, 1.0f).onDestroy() + StyleObserver(mockk(), nativeObserver, 1.0f).onDestroy() verify { nativeObserver.removeOnStyleLoadedListener(any()) } verify { nativeObserver.removeOnMapLoadErrorListener(any()) } } @@ -51,7 +42,11 @@ class StyleObserverTest { */ @Test fun onStyleLoadSuccess() { - val styleObserver = StyleObserver(mockk(relaxed = true), map, mockk(relaxed = true), 1.0f) + val styleObserver = StyleObserver( + mapboxMap = MapboxMap(WeakReference(mockk(relaxed = true)), mockk(relaxed = true), 1.0f), + nativeObserver = mockk(relaxed = true), + pixelRatio = 1.0f + ) val styleLoaded = mockk(relaxed = true) styleObserver.onNewStyleLoad(styleLoaded, null) styleObserver.onStyleLoaded() @@ -63,7 +58,11 @@ class StyleObserverTest { */ @Test fun onStyleLoadSuccessMulti() { - val styleObserver = StyleObserver(mockk(relaxed = true), map, mockk(relaxed = true), 1.0f) + val styleObserver = StyleObserver( + mapboxMap = MapboxMap(WeakReference(mockk(relaxed = true)), mockk(relaxed = true), 1.0f), + nativeObserver = mockk(relaxed = true), + pixelRatio = 1.0f + ) val styleLoaded = mockk(relaxed = true) styleObserver.onNewStyleLoad(styleLoaded, null) val styleLoaded2 = mockk(relaxed = true) @@ -78,7 +77,11 @@ class StyleObserverTest { */ @Test fun onStyleLoadSuccessNotCalled() { - val styleObserver = StyleObserver(mockk(relaxed = true), map, mockk(relaxed = true), 1.0f) + val styleObserver = StyleObserver( + mapboxMap = MapboxMap(WeakReference(mockk(relaxed = true)), mockk(relaxed = true), 1.0f), + nativeObserver = mockk(relaxed = true), + pixelRatio = 1.0f + ) val styleLoadedFail = mockk(relaxed = true) styleObserver.onNewStyleLoad(styleLoadedFail, null) val styleLoadedSucces = mockk(relaxed = true) @@ -93,7 +96,7 @@ class StyleObserverTest { */ @Test fun onStyleLoadError() { - val styleObserver = StyleObserver(mockk(relaxed = true), map, mockk(relaxed = true), 1.0f) + val styleObserver = StyleObserver(mockk(relaxed = true), mockk(relaxed = true), 1.0f) val errorListener = mockk(relaxed = true) styleObserver.onNewStyleLoad(mockk(relaxed = true), errorListener) styleObserver.onMapLoadError(MapLoadErrorType.GLYPHS, "foobar") @@ -105,7 +108,7 @@ class StyleObserverTest { */ @Test fun onStyleLoadErrorNotCalled() { - val styleObserver = StyleObserver(mockk(relaxed = true), map, mockk(relaxed = true), 1.0f) + val styleObserver = StyleObserver(mockk(relaxed = true), mockk(relaxed = true), 1.0f) val errorListenerFail = mockk(relaxed = true) styleObserver.onNewStyleLoad(mockk(relaxed = true), errorListenerFail) val errorListenerSuccess = mockk(relaxed = true) diff --git a/sdk/src/test/java/com/mapbox/maps/StyleTest.kt b/sdk/src/test/java/com/mapbox/maps/StyleTest.kt index 912ecf2c3f..9c0fdfa6df 100644 --- a/sdk/src/test/java/com/mapbox/maps/StyleTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/StyleTest.kt @@ -9,6 +9,7 @@ import io.mockk.mockk import io.mockk.verify import org.junit.Before import org.junit.Test +import java.lang.ref.WeakReference class StyleTest { @@ -17,7 +18,7 @@ class StyleTest { @Before fun setUp() { - style = Style(nativeMap, 1.0f) + style = Style(WeakReference(nativeMap as StyleManagerInterface), 1.0f) } @Test