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

fix: issue #265 #266

Merged
merged 2 commits into from
Oct 24, 2020
Merged

fix: issue #265 #266

merged 2 commits into from
Oct 24, 2020

Conversation

lcor-io
Copy link
Contributor

@lcor-io lcor-io commented Apr 30, 2020

Should fix the problem raised in issue #265

On Android, while disposing of the platform view, Mapbox tries to access Lifecycle events from the attached Activity. Problem is that this Activity could already be disposed of if the App where killed, with a back button for instance.
Checking if the activity is still registered should avoid Null Exceptions on those lifecycle callbacks.

@m0nac0
Copy link
Collaborator

m0nac0 commented May 3, 2020

Thank you for the contribution, I'd just like to wait for feedback from @tobrun whether we should actually return immediately if registrar.activity()==null or whether we can e.g. still safely call onDestroy() for the annotation managers.

@m0nac0 m0nac0 requested a review from tobrun May 3, 2020 09:47
@tobrun
Copy link
Collaborator

tobrun commented Jun 6, 2020

Problem is that this Activity could already be disposed of if the App where killed, with a back button for instance.

I haven't been able to reproduce the issue. The use-case is having a single page app that hosts a map and you use back button to kill the app?

@zhangxinagjunHJH
Copy link

@tobrun I created a flutter project, just show map and run app. it will crash when we try to use back button to exist app(try more than 5 times)

@m0nac0
Copy link
Collaborator

m0nac0 commented Aug 15, 2020

The docs for the new android plugin APIs (see #183 and https://flutter.dev/docs/development/packages-and-plugins/plugin-api-migration) explicitly mention registrar.activity() returning null and it being solved with the new APIs:

The new API has the advantage of providing a cleaner set of accessors for lifecycle dependent components compared to the old APIs. For instance PluginRegistry.Registrar.activity() could return null if Flutter isn’t attached to any activities.

So possibly this issue will also be solved if we migrate to the new plugin API.

@MATTYGILO
Copy link

Does this fix work?

@MATTYGILO
Copy link

com.mapbox.mapboxsdk.maps.Style.validateState (Style.java:777)
com.mapbox.mapboxsdk.maps.Style.getLayer (Style.java:240)
com.mapbox.mapboxsdk.location.SymbolLocationLayerRenderer.setLayerVisibility (SymbolLocationLayerRenderer.java:247)
com.mapbox.mapboxsdk.location.SymbolLocationLayerRenderer.hide (SymbolLocationLayerRenderer.java:109)
com.mapbox.mapboxsdk.location.LocationLayerController.hide (LocationLayerController.java:138)
com.mapbox.mapboxsdk.location.LocationComponent.disableLocationComponent (LocationComponent.java:1392)
com.mapbox.mapboxsdk.location.LocationComponent.setLocationComponentEnabled (LocationComponent.java:518)
com.mapbox.mapboxgl.MapboxMapController.dispose (MapboxMapController.java:806)
io.flutter.plugin.platform.VirtualDisplayController.dispose (VirtualDisplayController.java:171)
io.flutter.plugin.platform.PlatformViewsController.flushAllViews (PlatformViewsController.java:688)
io.flutter.plugin.platform.PlatformViewsController.onDetachedFromJNI (PlatformViewsController.java:570)
io.flutter.embedding.engine.FlutterEngine.destroy (FlutterEngine.java:361)
io.flutter.embedding.android.FlutterActivityAndFragmentDelegate.onDetach (FlutterActivityAndFragmentDelegate.java:522)
io.flutter.embedding.android.FlutterActivity.onDestroy (FlutterActivity.java:578)
android.app.Activity.performDestroy (Activity.java:8219)
android.app.Instrumentation.callActivityOnDestroy (Instrumentation.java:1342)
android.app.ActivityThread.performDestroyActivity (ActivityThread.java:5318)
android.app.ActivityThread.handleDestroyActivity (ActivityThread.java:5367)
android.app.servertransaction.DestroyActivityItem.execute (DestroyActivityItem.java:44)
android.app.servertransaction.TransactionExecutor.executeLifecycleState (TransactionExecutor.java:176)
android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:97)
android.app.ActivityThread$H.handleMessage (ActivityThread.java:2220)
android.os.Handler.dispatchMessage (Handler.java:107)
android.os.Looper.loop (Looper.java:237)
android.app.ActivityThread.main (ActivityThread.java:8016)
java.lang.reflect.Method.invoke (Method.java)
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:493)
com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1076)

@MATTYGILO
Copy link

This needs to be implemented!!!!

Copy link
Collaborator

@m0nac0 m0nac0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also haven't been able to reproduce this, but since it does seem fix an existing issue and shouldn't have any impact on anyone not affected by the issue, I'm going to approve this.

Thank you once again @leocornillon.

@christinachimi
Copy link

We're seeing similar errors causing crashes - curious if there is a timeline for releasing this?

@tobrun tobrun merged commit 2dd2054 into flutter-mapbox-gl:master Oct 24, 2020
IcyTv added a commit to IcyTv/flutter-mapbox-gl that referenced this pull request Nov 15, 2020
* Expose CompassViewPosition flutter-mapbox-gl#344 (flutter-mapbox-gl#346)

Co-authored-by: emre.yalcin <emre.yalcin@netcad.com.tr>

* Readme: location features on Android (flutter-mapbox-gl#342)

* Update CONTRIBUTING.md (flutter-mapbox-gl#366)

* Release 0.8.0 (flutter-mapbox-gl#390)

* [release] update CHANGELOG.md for release of v0.8.0

* [release] update version numbers to v0.8.0

* Update CONTRIBUTING.md (flutter-mapbox-gl#401)

Fix stale pull, issues & changelog urls in the contributing guide.

* Fix data parameter for addLine and addCircle (flutter-mapbox-gl#388)

* Split padding values in CameraUpdate.newLatLngBounds() (flutter-mapbox-gl#382)

* Split padding values in CameraUpdate.newLatLngBounds()

* Remove old unused code

Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* Re-enable attribution on android (flutter-mapbox-gl#383)

* Upgrade Android annotation plugin to v0.9 (flutter-mapbox-gl#381)

* web: ignore myLocationTrackingMode if myLocationEnabled is false (flutter-mapbox-gl#363)

* Add methods to access projection (flutter-mapbox-gl#380)

* remove bitmap; add projection access

* Replace ScreenLocation with Point; expand iOS implementation

* fix iOS with guard let

* iOS: cast to NSObject

* fix typo

* round result of toScreenLocation()

* Revert "round result of toScreenLocation()"

This reverts commit 838726a.

* Docs: document rounding behaviour

* Add Fill API support (flutter-mapbox-gl#49)

* [flutter] [android] - add fill support

* Resolved merge conflict.

* A first working version for ios (after some extensive rebasing).

* Minor cleanup

* Minor cleanup.

* Fix broken build Android.

* A working version for Android.

* Minor cleanup.

* Added fill pattern example. Works on Android not on iOS. Seems to break consecutive fills though.

* For the first queried feature (when filter is set) create a fill.

* Fix lint issue (unused method).

* Updated code formatting.

* Added interior polygon to iOS.

* [docs] update readme support table

* fixup

Co-authored-by: Timothy Sealy <timothy.sealy@gmail.com>

* Listen to OnUserLocationUpdated to provide user location to app (flutter-mapbox-gl#237)

* Listen to OnUserLocationUpdated to provide user location to app

While the `myLocationEnabled` property is set to `true`, this method is
called whenever a new location update is received by the map view.

iOS only, needs Android. I did check that the location properties
carried here are also provided in Android's [Location][1] object.

[1]: https://developer.android.com/reference/android/location/Location

* add android, web; fix conflicts

Co-authored-by: m0nac0 <58807793+m0nac0@users.noreply.github.com>
Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* fix: correct bug on android where checking on activity lifecycles that were disposed (flutter-mapbox-gl#266)

Co-authored-by: leo cornillon <leo.cornillon.dev@gmail.com>
Co-authored-by: m0nac0 <58807793+m0nac0@users.noreply.github.com>

* Add support for custom font stack in symbol options (flutter-mapbox-gl#359)

* fix memory leak caused by strong self reference (flutter-mapbox-gl#370)

* Basic ImageSource Support (flutter-mapbox-gl#409)

* Introduce LatLngQuad

Introduce the LatLngQuad object which will be
useful to pass in all the required parameters
to the addSource() method we will define later.

* Introduce addSource() Method

Add the addSource(..) method to the mapbox_gl_platform_interface.dart

* Add addSource MethodChannel

* Place ImageSource Android

- Introduce the PlaceSource page as a playground
- Add 'addImageSource', 'removeImageSource', 'addLayer' & 'removeLayer' apis
- Implement Android platform interface

* iOS ImageSource Implementation

Implement addImageSource, removeImageSource, addLayer &
removeLayer on iOS.

* Fix iOS CoordinateQuad Mapping

Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* Get meters per pixel at latitude (flutter-mapbox-gl#416)

* fix git refferences

* fix git refferences

* implementation of getMetersPerPixelAtLatitude

* getMetersPerPixelAtLatitude

* fix refference paths

* Android implementation and Example updated.

* added comments to getMetersPerPixelAtLatitude method

* IOS implementation

* Removed modified lines from pubspec.yaml files

* web implementation

Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* fix onStyleLoadedCallback (flutter-mapbox-gl#418)

* fix onStyleLoadedCallback

* fix onStyleLoadedCallback called before onMapCreated

Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>

* Release 0.9.0

* update version numbers for v0.9.0 release

* fix ios onSymbolTapped

Co-authored-by: emreuguryalcintr <50848628+emreuguryalcintr@users.noreply.github.com>
Co-authored-by: emre.yalcin <emre.yalcin@netcad.com.tr>
Co-authored-by: m0nac0 <58807793+m0nac0@users.noreply.github.com>
Co-authored-by: Tobrun <tobrun.van.nuland@gmail.com>
Co-authored-by: cuberob <robdeknegt@gmail.com>
Co-authored-by: qbouchat <48958612+qbouchat@users.noreply.github.com>
Co-authored-by: Timothy Sealy <timothy.sealy@gmail.com>
Co-authored-by: Nathan <nathan@transit.app>
Co-authored-by: Leo Cornillon <corle.cugly@gmail.com>
Co-authored-by: leo cornillon <leo.cornillon.dev@gmail.com>
Co-authored-by: Philip Lindberg <philip.c.lindberg@gmail.com>
Co-authored-by: Thomas Kröniger <30893720+thirteenthstep@users.noreply.github.com>
Co-authored-by: cuberob <git@cuberob.com>
Co-authored-by: GULERTOLGA <gulertolga@gmail.com>
Co-authored-by: Andrea Valenzano <andr3a689@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants