From 00f569a9f2de0d0235b055688c77b1384681d8dc Mon Sep 17 00:00:00 2001 From: Fynn Godau Date: Thu, 22 Jun 2023 23:01:27 +0200 Subject: [PATCH 1/5] Run user callbacks after `onCreate` Fixes Dott once more --- .../org/microg/gms/maps/mapbox/GoogleMap.kt | 66 ++++++++++++------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt index a52017f90..45bf6985b 100644 --- a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt +++ b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt @@ -88,7 +88,8 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG private var loaded = false private val mapLock = Object() - private val initializedCallbackList = mutableListOf() + private val internalOnInitializedCallbackList = mutableListOf() + private val userOnInitializedCallbackList = mutableListOf() private var loadedCallback: IOnMapLoadedCallback? = null private var cameraChangeListener: IOnCameraChangeListener? = null private var cameraMoveListener: IOnCameraMoveListener? = null @@ -205,7 +206,7 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG if (initialized) { runnable(map!!) } else { - initializedCallbackList.add(OnMapReadyCallback { + internalOnInitializedCallbackList.add(OnMapReadyCallback { runnable(it) }) } @@ -454,7 +455,7 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG override fun getUiSettings(): IUiSettingsDelegate = map?.uiSettings?.let { UiSettingsImpl(it) } ?: UiSettingsCache().also { // Apply cached UI settings after map is initialized - initializedCallbackList.add(it.getMapReadyCallback()) + internalOnInitializedCallbackList.add(it.getMapReadyCallback()) } override fun getProjection(): IProjectionDelegate = map?.projection?.let { @@ -562,6 +563,7 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG mapView.onCreate(savedInstanceState?.toMapbox()) mapView.getMapAsync(this::initMap) created = true + runOnMainLooper(forceQueue = true) { tryRunUserInitializedCallbacks("onCreate") } } } @@ -651,13 +653,16 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG synchronized(mapLock) { initialized = true waitingCameraUpdates.forEach { map.moveCamera(it) } - val initializedCallbackList = ArrayList(initializedCallbackList) - Log.d(TAG, "Invoking ${initializedCallbackList.size} callbacks delayed, as map is initialized") + val initializedCallbackList = ArrayList(internalOnInitializedCallbackList) + Log.d(TAG, "Invoking ${initializedCallbackList.size} internal callbacks now that the true map is initialized") for (callback in initializedCallbackList) { callback.onMapReady(map) } } + // No effect if no initialized callbacks are present. + tryRunUserInitializedCallbacks(tag = "initMap") + map.getStyle { mapView?.let { view -> if (loaded) return@let @@ -841,34 +846,45 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG } fun getMapAsync(callback: IOnMapReadyCallback) { - synchronized(mapLock) { + userOnInitializedCallbackList.add(callback) + tryRunUserInitializedCallbacks("getMapAsync") + } - val runCallback = { + fun tryRunUserInitializedCallbacks(tag: String = "") { + + if (userOnInitializedCallbackList.isEmpty()) return + + val runCallbacks = { + userOnInitializedCallbackList.forEach { try { - callback.onMapReady(this) + it.onMapReady(this) } catch (e: Exception) { Log.w(TAG, e) } + }.also { + userOnInitializedCallbackList.clear() } + } - if (initialized) { - Log.d(TAG, "Invoking callback instantly, as map is initialized") - runCallback() - } else if (mapView?.isShown == false) { - /* If map is hidden, an app (e.g. Dott) may expect it to initialize anyway and - * will not show the map until it is initialized. However, we should not call - * the callback before onCreate is started (we know this is the case if mapView is - * null), otherwise that results in other problems (e.g. Gas Now app not - * initializing). - */ - runOnMainLooper(forceQueue = true) { - Log.d(TAG, "Invoking callback now: map cannot be initialized because it is not shown (yet)") - runCallback() - } - } else { - Log.d(TAG, "Delay callback invocation, as map is not yet initialized") - initializedCallbackList.add { runCallback() } + val map = map + if (initialized && map != null) { + // Call all callbacks immediately, as map is ready + Log.d("TAG:$tag", "Invoking callback instantly, as map is initialized") + runCallbacks() + } else if (mapView?.isShown == false) { + /* If map is hidden, an app (e.g. Dott) may expect it to initialize anyway and + * will not show the map until it is initialized. However, we should not call + * the callback before onCreate is started (we know this is the case if mapView is + * null), otherwise that results in other problems (e.g. Gas Now app not + * initializing). + */ + runOnMainLooper(forceQueue = true) { + Log.d("TAG:$tag", "Invoking callback now: map cannot be initialized because it is not shown (yet)") + runCallbacks() } + } else { + Log.d("TAG:$tag", "Initialized callbacks could not be run at this point, as the map view has not been created yet.") + // Will be retried after initialization. } } -- GitLab From 462c57236775af94ebb020d47cd540bea29033a2 Mon Sep 17 00:00:00 2001 From: Fynn Godau Date: Fri, 23 Jun 2023 10:07:03 +0200 Subject: [PATCH 2/5] Synchronize user callback list operations --- .../org/microg/gms/maps/mapbox/GoogleMap.kt | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt index 45bf6985b..2ff572f95 100644 --- a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt +++ b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt @@ -846,7 +846,9 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG } fun getMapAsync(callback: IOnMapReadyCallback) { - userOnInitializedCallbackList.add(callback) + synchronized(mapLock) { + userOnInitializedCallbackList.add(callback) + } tryRunUserInitializedCallbacks("getMapAsync") } @@ -855,14 +857,16 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG if (userOnInitializedCallbackList.isEmpty()) return val runCallbacks = { - userOnInitializedCallbackList.forEach { - try { - it.onMapReady(this) - } catch (e: Exception) { - Log.w(TAG, e) + synchronized(mapLock) { + userOnInitializedCallbackList.forEach { + try { + it.onMapReady(this) + } catch (e: Exception) { + Log.w(TAG, e) + } + }.also { + userOnInitializedCallbackList.clear() } - }.also { - userOnInitializedCallbackList.clear() } } -- GitLab From 3eb6f52e40030520a8e45be3619aa3f34062d7e2 Mon Sep 17 00:00:00 2001 From: Fynn Godau Date: Fri, 23 Jun 2023 10:44:00 +0200 Subject: [PATCH 3/5] Synchronize list read access --- .../src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt index 2ff572f95..6202200d9 100644 --- a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt +++ b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt @@ -854,7 +854,9 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG fun tryRunUserInitializedCallbacks(tag: String = "") { - if (userOnInitializedCallbackList.isEmpty()) return + synchronized(mapLock) { + if (userOnInitializedCallbackList.isEmpty()) return + } val runCallbacks = { synchronized(mapLock) { -- GitLab From 4a3dd26c5ca4a90acf637cbe76f68c765ff038cb Mon Sep 17 00:00:00 2001 From: Fynn Godau Date: Tue, 27 Jun 2023 12:00:59 +0200 Subject: [PATCH 4/5] Apply review --- .../src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt index 6202200d9..7540d641f 100644 --- a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt +++ b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt @@ -875,7 +875,7 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG val map = map if (initialized && map != null) { // Call all callbacks immediately, as map is ready - Log.d("TAG:$tag", "Invoking callback instantly, as map is initialized") + Log.d("$TAG:$tag", "Invoking callback instantly, as map is initialized") runCallbacks() } else if (mapView?.isShown == false) { /* If map is hidden, an app (e.g. Dott) may expect it to initialize anyway and -- GitLab From 949b6ebb7a8d608c6fd06d01823c3ed2315fe421 Mon Sep 17 00:00:00 2001 From: Fynn Godau Date: Tue, 27 Jun 2023 12:56:02 +0200 Subject: [PATCH 5/5] Apply review --- .../src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt index 7540d641f..ab3e81112 100644 --- a/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt +++ b/play-services-maps-core-mapbox/src/main/kotlin/org/microg/gms/maps/mapbox/GoogleMap.kt @@ -885,11 +885,11 @@ class GoogleMapImpl(context: Context, var options: GoogleMapOptions) : AbstractG * initializing). */ runOnMainLooper(forceQueue = true) { - Log.d("TAG:$tag", "Invoking callback now: map cannot be initialized because it is not shown (yet)") + Log.d("$TAG:$tag", "Invoking callback now: map cannot be initialized because it is not shown (yet)") runCallbacks() } } else { - Log.d("TAG:$tag", "Initialized callbacks could not be run at this point, as the map view has not been created yet.") + Log.d("$TAG:$tag", "Initialized callbacks could not be run at this point, as the map view has not been created yet.") // Will be retried after initialization. } } -- GitLab