From e4e721634842e6a17fabedf78811f5e3b99980bd Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sat, 9 Nov 2024 12:45:56 +0100 Subject: [PATCH 1/2] Don't force recreating the map object and associated layer state If Main has been destroyed, both the Map and Logic objects may still be available and current, so we now reuse them instead of forcing recreation. This resolves race conditions that could be caused by selecting and loading files into layers via the system file picker that could lead to Main being removed. --- src/main/java/de/blau/android/Main.java | 27 +++++++++++-------- src/main/java/de/blau/android/Map.java | 2 +- .../java/de/blau/android/dialogs/Layers.java | 3 ++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/main/java/de/blau/android/Main.java b/src/main/java/de/blau/android/Main.java index 3bef635a6..8049f0c70 100644 --- a/src/main/java/de/blau/android/Main.java +++ b/src/main/java/de/blau/android/Main.java @@ -67,6 +67,7 @@ import android.view.View.OnLongClickListener; import android.view.View.OnTouchListener; import android.view.ViewGroup; +import android.view.ViewParent; import android.view.ViewStub; import android.widget.LinearLayout; import android.widget.RelativeLayout; @@ -492,13 +493,17 @@ protected void onCreate(final Bundle savedInstanceState) { LinearLayout ml = (LinearLayout) getLayoutInflater().inflate(layout, null); mapLayout = (RelativeLayout) ml.findViewById(R.id.mainMap); - if (map != null) { - Log.d(DEBUG_TAG, "map exists .. destroying"); - map.onDestroy(); + Logic logic = App.getLogic(); // logic instance might still be around + map = logic != null ? logic.getMap() : null; + if (map == null) { + map = new Map(this); + map.setId(R.id.map_view); + } else { + ViewGroup parent = (ViewGroup) map.getParent(); + if (parent != null) { + parent.removeView(map); + } } - map = new Map(this); - map.setId(R.id.map_view); - // Register some Listener mapTouchListener = new MapTouchListener(); map.setOnTouchListener(mapTouchListener); @@ -585,14 +590,14 @@ protected void onCreate(final Bundle savedInstanceState) { loadOnResume = false; - if (App.getLogic() == null) { + if (logic == null) { Log.i(DEBUG_TAG, "onCreate - creating new logic"); - App.newLogic(); + logic = App.newLogic(); } Log.i(DEBUG_TAG, "onCreate - setting new map"); - App.getLogic().setPrefs(prefs); - App.getLogic().setMap(map, true); + logic.setPrefs(prefs); + logic.setMap(map, true); Log.d(DEBUG_TAG, "StorageDelegator dirty is " + App.getDelegator().isDirty()); if (StorageDelegator.isStateAvailable(this) && !App.getDelegator().isDirty()) { @@ -806,7 +811,7 @@ public void onError(@Nullable AsyncResult result) { // loadStateFromFile does this above App.getLogic().loadEditingState(this, setViewBox); } - logic.loadLayerState(this, postLoadTasks); + // layer state should still be available if logic is still around, no need to load map.invalidate(); checkPermissions(() -> { postLoadData.onSuccess(); diff --git a/src/main/java/de/blau/android/Map.java b/src/main/java/de/blau/android/Map.java index 2b1e0bb96..a639f6786 100755 --- a/src/main/java/de/blau/android/Map.java +++ b/src/main/java/de/blau/android/Map.java @@ -427,7 +427,7 @@ public MapViewLayer getLayer(int index) { if (index >= 0 && index < mLayers.size()) { return mLayers.get(index); } - Log.e(DEBUG_TAG, "Layer for index " + index + " is null"); + Log.e(DEBUG_TAG, "Layer for index " + index + " is null " + mLayers.size() + " layers total"); return null; } } diff --git a/src/main/java/de/blau/android/dialogs/Layers.java b/src/main/java/de/blau/android/dialogs/Layers.java index dbe7ff7ce..637f6b794 100644 --- a/src/main/java/de/blau/android/dialogs/Layers.java +++ b/src/main/java/de/blau/android/dialogs/Layers.java @@ -420,7 +420,8 @@ protected Void doInBackground(Void param) { * @param map current Map * @param type the layer type */ - private void addStyleableLayerFromFile(final FragmentActivity activity, final Preferences prefs, final Map map, @NonNull final LayerType type) { + private void addStyleableLayerFromFile(@NonNull final FragmentActivity activity, @NonNull final Preferences prefs, @NonNull final Map map, + @NonNull final LayerType type) { Log.d(DEBUG_TAG, "addStyleableLayerFromFile"); SelectFile.read(activity, R.string.config_osmPreferredDir_key, new ReadFile() { private static final long serialVersionUID = 1L; From 86d40f6db2f6129d9fd93cfb44c68620102aa525 Mon Sep 17 00:00:00 2001 From: simonpoole Date: Sun, 10 Nov 2024 14:37:10 +0100 Subject: [PATCH 2/2] WIP --- src/test/java/de/blau/android/osm/ApiErrorTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/java/de/blau/android/osm/ApiErrorTest.java b/src/test/java/de/blau/android/osm/ApiErrorTest.java index 327580339..9d6b9eb2d 100644 --- a/src/test/java/de/blau/android/osm/ApiErrorTest.java +++ b/src/test/java/de/blau/android/osm/ApiErrorTest.java @@ -93,7 +93,7 @@ public void onSuccess() { @Override public void onError(AsyncResult result) { - System.out.println("FailOnErrorHandler onError"); + System.out.println("FailOnSuccessHandler onError " + expectedError + " " + result.getCode()); assertEquals(expectedError, result.getCode()); signal.countDown(); } @@ -106,14 +106,15 @@ public void onError(AsyncResult result) { public void setup() { mockServer = new MockWebServerPlus(); HttpUrl mockBaseUrl = mockServer.server().url("/api/0.6/"); + App.getDelegator().reset(true); main = Robolectric.buildActivity(Main.class).create().resume().get(); prefDB = new AdvancedPrefDatabase(main); prefDB.deleteAPI("Test"); prefDB.addAPI("Test", "Test", mockBaseUrl.toString(), null, null, "user", "pass", API.Auth.BASIC); prefDB.selectAPI("Test"); - System.out.println("mock api url " + mockBaseUrl.toString()); // NOSONAR - Logic logic = App.getLogic(); + System.out.println("mock api url " + mockBaseUrl.toString()); // NOSONAR prefs = new Preferences(main); + Logic logic = App.getLogic(); logic.setPrefs(prefs); logic.getMap().setPrefs(main, prefs); }