Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

182-disposal-null-ref-crash #259

Merged
merged 6 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## upcoming version

### Breaking Change:
* `useDelayedDisposal` was removed since its now fixed in https://github.com/maplibre/flutter-maplibre-gl/pull/259
* `useHybridCompositionOverride` was removed since it was added in the following fix: https://github.com/maplibre/flutter-maplibre-gl/pull/203 and we reverted the fix and used another approach to fix the actual issue.
* The default for `myLocationRenderMode` was changed from `COMPASS` to `NORMAL` in https://github.com/maplibre/flutter-maplibre-gl/pull/244, since the previous default value of `COMPASS` implicitly enables displaying the location on iOS, which could crash apps that didn't want to display the device location. If you want to continue to use `MyLocationRenderMode.COMPASS`, please explicitly specify it in the constructor like this:
```dart
MaplibreMap(
Expand Down
7 changes: 1 addition & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,7 @@ buildTypes {
}
```

## Flutter 3.x.x issues
Since Flutter 3.x.x was introduced, it exposed some race conditions resulting in occasional crashes upon map disposal. The parameter `useDelayedDisposal` was introduced as a workaround for this issue until Flutter and/or Maplibre fix this issue properly. Use with caution.



### iOS app crashes on startup
### iOS app crashes when using location based features

Please include the `NSLocationWhenInUseUsageDescription` as described [here](#location-features)

Expand Down
30 changes: 28 additions & 2 deletions android/src/main/java/com/mapbox/mapboxgl/MapboxMapController.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
import android.util.Log;
import android.view.Gravity;
import android.view.MotionEvent;
import android.view.TextureView;
import android.view.View;
import android.widget.FrameLayout;

import androidx.annotation.NonNull;
import androidx.lifecycle.DefaultLifecycleObserver;
import androidx.lifecycle.Lifecycle;
Expand Down Expand Up @@ -118,6 +121,11 @@ final class MapboxMapController
private final float density;
private final Context context;
private final String styleStringInitial;
/**
* This container is returned as the final platform view instead of returning `mapView`.
* See {@link MapboxMapController#destroyMapViewIfNecessary()} for details.
*/
private FrameLayout mapViewContainer;
private MapView mapView;
private MapboxMap mapboxMap;
private boolean trackCameraPosition = false;
Expand Down Expand Up @@ -181,6 +189,7 @@ public void onStyleLoaded(@NonNull Style style) {
this.context = context;
this.dragEnabled = dragEnabled;
this.styleStringInitial = styleStringInitial;
this.mapViewContainer = new FrameLayout(context);
this.mapView = new MapView(context, options);
this.interactiveFeatureLayerIds = new HashSet<>();
this.addedFeaturesByLayer = new HashMap<String, FeatureCollection>();
Expand All @@ -190,13 +199,14 @@ public void onStyleLoaded(@NonNull Style style) {
this.androidGesturesManager = new AndroidGesturesManager(this.mapView.getContext(), false);
}

mapViewContainer.addView(mapView);
methodChannel = new MethodChannel(messenger, "plugins.flutter.io/mapbox_maps_" + id);
methodChannel.setMethodCallHandler(this);
}

@Override
public View getView() {
return mapView;
return mapViewContainer;
}

void init() {
Expand Down Expand Up @@ -1544,17 +1554,33 @@ public void onCancel() {
}
}

/**
* Destroy the MapView and cleans up listeners.
* It's very important to call mapViewContainer.removeView(mapView) to make sure
* that {@link TextureView#onDetachedFromWindowInternal()} is called which releases the
* underlying surface.
* This is required due to an FlutterEngine change that was introduce when updating from
* Flutter 2.10.5 to Flutter 3.10.0.
* This FlutterEngine change is not calling `removeView` on a PlatformView which causes the issue.
* <p>
* For more information check out:
* <a href="https://github.com/flutter/flutter/issues/107297">Flutter issue</a>
* <a href="https://github.com/flutter/engine/commit/8dc7cd1b1a33b5da561ac859cdcc49705ad1e598">Flutter Engine commit that introduced the issue</a>
* <a href="https://github.com/maplibre/flutter-maplibre-gl/issues/182">The reported issue in the MapLibre repo</a>
*/
private void destroyMapViewIfNecessary() {
if (mapView == null) {
return;
}
mapViewContainer.removeView(mapView);
mapView.onStop();
mapView.onDestroy();

if (locationComponent != null) {
locationComponent.setLocationComponentEnabled(false);
}
stopListeningForLocationUpdates();

mapView.onDestroy();
mapView = null;
}

Expand Down
10 changes: 0 additions & 10 deletions lib/src/mapbox_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ class MaplibreMap extends StatefulWidget {
AnnotationType.line,
AnnotationType.circle,
],
this.useDelayedDisposal,
this.useHybridCompositionOverride,
}) : assert(
myLocationRenderMode != MyLocationRenderMode.NORMAL
? myLocationEnabled
Expand Down Expand Up @@ -221,12 +219,6 @@ class MaplibreMap extends StatefulWidget {
/// * All fade/transition animations have completed
final OnMapIdleCallback? onMapIdle;

/// Use delayed disposal of Android View Controller to avoid flutter 3.x.x crashes
final bool? useDelayedDisposal;

/// Override hybrid mode per map instance
final bool? useHybridCompositionOverride;

/// Set `MapboxMap.useHybridComposition` to `false` in order use Virtual-Display
/// (better for Android 9 and below but may result in errors on Android 12)
/// or leave it `true` (default) to use Hybrid composition (Slower on Android 9 and below).
Expand Down Expand Up @@ -258,8 +250,6 @@ class _MaplibreMapState extends State<MaplibreMap> {
'options': _MapboxMapOptions.fromWidget(widget).toMap(),
//'onAttributionClickOverride': widget.onAttributionClick != null,
'dragEnabled': widget.dragEnabled,
'useDelayedDisposal': widget.useDelayedDisposal,
'useHybridCompositionOverride': widget.useHybridCompositionOverride,
};
return _mapboxGlPlatform.buildView(
creationParams, onPlatformViewCreated, widget.gestureRecognizers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
part 'src/view_wrappers.dart';
part 'src/annotation.dart';
part 'src/callbacks.dart';
part 'src/camera.dart';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,7 @@ class MethodChannelMaplibreGl extends MapLibreGlPlatform {
OnPlatformViewCreatedCallback onPlatformViewCreated,
Set<Factory<OneSequenceGestureRecognizer>>? gestureRecognizers) {
if (defaultTargetPlatform == TargetPlatform.android) {
final useDelayedDisposalParam =
(creationParams['useDelayedDisposal'] ?? false) as bool;
final useHybridCompositionParam =
(creationParams['useHybridCompositionOverride'] ??
useHybridComposition) as bool;
if (useHybridCompositionParam) {
if (useHybridComposition) {
return PlatformViewLink(
viewType: 'plugins.flutter.io/mapbox_gl',
surfaceFactory: (
Expand All @@ -158,26 +153,15 @@ class MethodChannelMaplibreGl extends MapLibreGlPlatform {
);
},
onCreatePlatformView: (PlatformViewCreationParams params) {
late AndroidViewController controller;
if (useDelayedDisposalParam) {
controller = WrappedPlatformViewsService.initAndroidView(
id: params.id,
viewType: 'plugins.flutter.io/mapbox_gl',
layoutDirection: TextDirection.ltr,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
onFocus: () => params.onFocusChanged(true),
);
} else {
controller = PlatformViewsService.initAndroidView(
id: params.id,
viewType: 'plugins.flutter.io/mapbox_gl',
layoutDirection: TextDirection.ltr,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
onFocus: () => params.onFocusChanged(true),
);
}
final controller = PlatformViewsService.initAndroidView(
id: params.id,
viewType: 'plugins.flutter.io/mapbox_gl',
layoutDirection: TextDirection.ltr,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
onFocus: () => params.onFocusChanged(true),
);

controller.addOnPlatformViewCreatedListener(
params.onPlatformViewCreated,
);
Expand All @@ -190,15 +174,6 @@ class MethodChannelMaplibreGl extends MapLibreGlPlatform {
},
);
} else {
if (useDelayedDisposalParam) {
return AndroidViewWithWrappedController(
viewType: 'plugins.flutter.io/mapbox_gl',
onPlatformViewCreated: onPlatformViewCreated,
gestureRecognizers: gestureRecognizers,
creationParams: creationParams,
creationParamsCodec: const StandardMessageCodec(),
);
}
return AndroidView(
viewType: 'plugins.flutter.io/mapbox_gl',
onPlatformViewCreated: onPlatformViewCreated,
Expand Down
Loading