mapbox-maps-android icon indicating copy to clipboard operation
mapbox-maps-android copied to clipboard

Crash during removal of view annotation - Cannot get annotation options for id: '42', it does not exist.

Open mfazekas opened this issue 3 years ago • 1 comments

Environment

  • Android OS version: Simulator/Pixel 6 API 31
  • Devices affected: Simulator/Pixel 6 API 31
  • Maps SDK Version: 10.8.1, 10.9.0-beta.2

Observed behavior and steps to reproduce

Removing an annotation view if view is in transition can cause a crash.

See the following code and note the startViewTransition just before removeViewAnnotation(mAnnotationView!!)

package com.example.v10androidannotations

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.FrameLayout
import androidx.appcompat.app.AppCompatActivity
import androidx.core.view.updateLayoutParams
import com.example.v10androidannotations.R
import com.mapbox.geojson.Point
import com.mapbox.maps.CameraOptions
import com.mapbox.maps.MapView
import com.mapbox.maps.Style
import com.mapbox.maps.ViewAnnotationOptions
import com.mapbox.maps.viewannotation.viewAnnotationOptions

var mapView: MapView? = null

class MainActivity : AppCompatActivity() {

    var mMapView : MapView? = null;
    var mAnnotationView : View? = null;

    fun removeViewAnnotation(view: View) {
        mMapView?.startViewTransition(mAnnotationView)
        mMapView?.viewAnnotationManager?.removeViewAnnotation(mAnnotationView!!)
    }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        mapView = findViewById(R.id.mapView)
        mapView?.getMapboxMap()?.loadStyleUri(Style.MAPBOX_STREETS)
        mMapView = mapView

        val camera = CameraOptions.Builder()
            .center(
                Point.fromLngLat(
                    -74.0066,40.7135
                )
            )
            .zoom(15.5)
            .bearing(-17.6)
            .build();

        mapView!!.getMapboxMap()!!.setCamera(camera)

        val viewAnnotationManager = mapView!!.viewAnnotationManager

        val view = LayoutInflater.from(baseContext).inflate(R.layout.annotation_view, null);

        view.layoutParams = FrameLayout.LayoutParams(200, 62);
        val viewAnnotation = viewAnnotationManager.addViewAnnotation(
            view = view,
            options = viewAnnotationOptions {
                geometry(Point.fromLngLat(
                    -74.0066,40.7135
                ))
                width(100)
                height(100)
            }
        )

        mAnnotationView = view
    }
}

activity_mail.xml

<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    tools:context=".MainActivity">

    <Button
        android:id="@+id/button"
        android:layout_width="wrap_content"
        android:layout_height="22pt"
        android:layout_marginTop="20pt"
        app:layout_constraintBottom_toTopOf="@id/mapView"
        app:layout_constraintEnd_toEndOf="parent"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintTop_toTopOf="parent"
        android:text="Remove View Annotation"
        android:onClick="removeViewAnnotation"
        />

    <com.mapbox.maps.MapView
        android:id="@+id/mapView"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        app:layout_constraintBottom_toBottomOf="parent"
        app:layout_constraintEnd_toEndOf="parent"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintTop_toBottomOf="@id/button" />

</androidx.constraintlayout.widget.ConstraintLayout>

annotation_view.xml

<?xml version="1.0" encoding="utf-8"?>
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:id="@+id/frameLayout"
    android:layout_width="100dp"
    android:layout_height="100dp"
    android:background="@android:color/white"
    xmlns:app="http://schemas.android.com/apk/res-auto">

    <TextView
        android:id="@+id/annotation"
        android:text="hello world"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:gravity="center"
        android:textSize="20sp"
        android:padding="3dp"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintStart_toStartOf="parent" />

</FrameLayout>

crash:

D/AndroidRuntime: Shutting down VM
E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.example.v10androidannotations, PID: 24209
    com.mapbox.maps.MapboxViewAnnotationException: Cannot get annotation options for id: '42', it does not exist.
        at com.mapbox.maps.ViewAnnotationManagerImpl.prepareViewAnnotation$lambda-6(ViewAnnotationManagerImpl.kt:496)
        at com.mapbox.maps.ViewAnnotationManagerImpl.$r8$lambda$h-8qPi_VGvrrOLG6FgtIoOywDiM(Unknown Source:0)
        at com.mapbox.maps.ViewAnnotationManagerImpl$$ExternalSyntheticLambda2.onDraw(Unknown Source:6)
        at android.view.ViewTreeObserver.dispatchOnDraw(ViewTreeObserver.java:1132)
        at android.view.ViewRootImpl.draw(ViewRootImpl.java:4352)
        at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:4149)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3309)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2126)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8653)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037)
        at android.view.Choreographer.doCallbacks(Choreographer.java:845)
        at android.view.Choreographer.doFrame(Choreographer.java:780)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7839)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
I/Process: Sending signal. PID: 24209 SIG: 9
Disconnected from the target VM, address: 'localhost:60658', transport: 'socket'

Expected behavior

No crash just removal of view.

Notes / preliminary analysis

The code seems to rely that onViewDetachedFromWindow will be called from mapView.removeView(annotation.view) but if annotation.view is transitioning the detach will not happen. I think that on remove we could just directly call removeOnGlobalLayoutListener, also on remove we could remove the view from currentViewsDrawnMap as well.

Additional links and references

n/a

mfazekas avatar Oct 04 '22 18:10 mfazekas

@mfazekas indeed looks like a bug, thanks for detailed issue description. We'll try to pick it up in the nearest future.

kiryldz avatar Oct 11 '22 07:10 kiryldz

There is another case for this bug.

diff --git a/app/src/main/java/com/mapbox/maps/testapp/examples/markersandcallouts/ViewAnnotationBasicAddActivity.kt b/app/src/main/java/com/mapbox/maps/testapp/examples/markersandcallouts/ViewAnnotationBasicAddActivity.kt
index 29e50ca3..93e1967e 100644
--- a/app/src/main/java/com/mapbox/maps/testapp/examples/markersandcallouts/ViewAnnotationBasicAddActivity.kt
+++ b/app/src/main/java/com/mapbox/maps/testapp/examples/markersandcallouts/ViewAnnotationBasicAddActivity.kt
@@ -6,6 +6,7 @@ import android.view.Menu
 import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
+import android.view.animation.RotateAnimation
 import android.widget.Button
 import android.widget.Toast
 import androidx.appcompat.app.AppCompatActivity
@@ -98,6 +99,7 @@ class ViewAnnotationBasicAddActivity : AppCompatActivity(), OnMapClickListener {
     ItemCalloutViewBinding.bind(viewAnnotation).apply {
       textNativeView.text = "lat=%.2f\nlon=%.2f".format(point.latitude(), point.longitude())
       closeNativeView.setOnClickListener {
+        viewAnnotation.animation = RotateAnimation(0.1f,0.9f)
         viewAnnotationManager.removeViewAnnotation(viewAnnotation)
         viewAnnotationViews.remove(viewAnnotation)
       }

If there is an animation attached to the view while it's being removed then onViewDetachedFromWindow will not be called - see https://android.googlesource.com/platform/frameworks/base/+/ebb2d8d/core/java/android/view/ViewGroup.java#3354.

https://github.com/mapbox/mapbox-maps-android/blob/e42ab7bbd50d7724ce7ce48d2d805a109fc458d3/sdk/src/main/java/com/mapbox/maps/ViewAnnotationManagerImpl.kt#L421-L424

And the removed view annotation will be still watching the onDraw listener.

I'm getting this backtrace:

FATAL EXCEPTION: main
                                                                                                    Process: com.mapbox.maps.testapp, PID: 15667
                                                                                                    com.mapbox.maps.MapboxViewAnnotationException: Cannot get annotation options for id: '42', it does not exist.
                                                                                                    	at com.mapbox.maps.ViewAnnotationManagerImpl.prepareViewAnnotation$lambda-19(ViewAnnotationManagerImpl.kt:771)
                                                                                                    	at com.mapbox.maps.ViewAnnotationManagerImpl.$r8$lambda$oVDcIy94CHbv5l679NiXg9tc6DU(Unknown Source:0)
                                                                                                    	at com.mapbox.maps.ViewAnnotationManagerImpl$$ExternalSyntheticLambda1.onDraw(Unknown Source:6)
                                                                                                    	at android.view.ViewTreeObserver.dispatchOnDraw(ViewTreeObserver.java:1132)
                                                                                                    	at android.view.ViewRootImpl.draw(ViewRootImpl.java:4352)
                                                                                                    	at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:4149)
                                                                                                    	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3309)
                                                                                                    	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2126)
                                                                                                    	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8653)
                                                                                                    	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037)
                                                                                                    	at android.view.Choreographer.doCallbacks(Choreographer.java:845)
                                                                                                    	at android.view.Choreographer.doFrame(Choreographer.java:780)
                                                                                                    	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:938)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7839)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

mfazekas avatar Nov 13 '22 04:11 mfazekas

@kiryldz FYI I've submitted a PR addressing the issue #1829

mfazekas avatar Nov 13 '22 04:11 mfazekas