Add fix for lifecycle observe leak

Following a leak report, `RiveAnimationView` will now initialize the lifecycle observer with a controller parameter to allow for it release upon destruction. This still allows us to behave right when a RecyclerView allocates new `RiveAnimationView`s that never get attached to the window (and they can subsequently be released when the surrounding `Activity` is closed) but also to timely release resources when `RiveAnimationView` lives within a Fragment, so it doesn't have to wait until the whole Activity is closed. More specifically, it wasn't just the native objects that were leaking but the entire View due to the `inner RiveViewLifecycleObserver`. With the class now living outside `RiveAnimationView` we get rid of that leak.
In any case, to avoid contention between `LifecycleOwner`s, when `onAttachedToWindow()` is called, the `View` will now validate the current `lifecycleOwner` and swap it out if necessary.

It also add an example Activity that was reproducing the issue mentioned above.

Diffs=
fee8fbb01 Add fix for lifecycle observe leak (#5564)

Co-authored-by: Umberto Sonnino <[email protected]>

.rive_head

@@ -1 +1 @@
-094afae0b4f55e121a36981b9846ccbc9912c347
+fee8fbb01b8df639d11244f6ac00bd651a324fa0

app/src/main/AndroidManifest.xml

@@ -12,11 +12,6 @@
         android:roundIcon="@mipmap/ic_launcher_round"
         android:supportsRtl="true"
         android:theme="@style/AppTheme">
-        <activity
-            android:name=".ComposeActivity"
-            android:exported="false"
-            android:label="@string/title_activity_compose"
-            android:theme="@style/AppTheme" />
 
         <provider
             android:name="androidx.startup.InitializationProvider"
@@ -58,6 +53,13 @@
         <activity android:name=".MeshesActivity" />
         <activity android:name=".ViewStubActivity" />
         <activity android:name=".ViewPagerActivity" />
+        <activity
+            android:name=".ComposeActivity"
+            android:exported="false"
+            android:label="@string/title_activity_compose"
+            android:theme="@style/AppTheme" />
+        <activity android:name=".FrameActivity" />
+
         <!-- For testing -->
         <activity android:name=".SingleActivity" />
     </application>

app/src/main/java/app/rive/runtime/example/FrameActivity.kt

@@ -0,0 +1,68 @@
+package app.rive.runtime.example
+
+import android.os.Bundle
+import android.view.LayoutInflater
+import android.view.View
+import android.view.ViewGroup
+import android.widget.Button
+import androidx.appcompat.app.AppCompatActivity
+import androidx.fragment.app.Fragment
+import androidx.fragment.app.commit
+import app.rive.runtime.kotlin.RiveAnimationView
+
+class FrameActivity : AppCompatActivity() {
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContentView(R.layout.activity_frame)
+
+        // Don't do anything if restoring a previous state.
+        if (savedInstanceState != null) {
+            return
+        }
+
+        supportFragmentManager.commit {
+            setReorderingAllowed(true)
+            add(R.id.frame_fragment_container, TextFragment())
+        }
+
+        findViewById<Button>(R.id.swap_fragment).setOnClickListener {
+            val currentFragment =
+                supportFragmentManager.findFragmentById(R.id.frame_fragment_container)
+            supportFragmentManager.commit {
+                replace(
+                    R.id.frame_fragment_container,
+                    if (currentFragment is TextFragment) RiveSimpleFragment() else TextFragment()
+                )
+            }
+        }
+    }
+}
+
+class TextFragment : Fragment() {
+    override fun onCreateView(
+        inflater: LayoutInflater,
+        container: ViewGroup?,
+        savedInstanceState: Bundle?
+    ): View? {
+        return inflater.inflate(R.layout.fragment_text, container, false)
+    }
+}
+
+open class RiveSimpleFragment : Fragment() {
+    override fun onCreateView(
+        inflater: LayoutInflater, container: ViewGroup?,
+        savedInstanceState: Bundle?
+    ): View? {
+        val riveFragmentContainerView =
+            inflater.inflate(R.layout.fragment_rive, container, false)
+        val riveView =
+            riveFragmentContainerView.findViewById<RiveAnimationView>(R.id.rive_view_fragment)
+        riveView.layoutParams.apply {
+            width = 300
+            height = 300
+        }
+        riveView.setRiveResource(R.raw.basketball)
+
+        return riveFragmentContainerView
+    }
+}

app/src/main/java/app/rive/runtime/example/MainActivity.kt

@@ -29,6 +29,7 @@ class MainActivity : AppCompatActivity() {
         Pair(R.id.go_meshes, MeshesActivity::class.java),
         Pair(R.id.go_viewstub, ViewStubActivity::class.java),
         Pair(R.id.go_compose, ComposeActivity::class.java),
+        Pair(R.id.go_frame, FrameActivity::class.java),
     )
 
     override fun onCreate(savedInstanceState: Bundle?) {

app/src/main/res/layout/activity_frame.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>
+<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:id="@+id/frame_fragment_container"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:padding="16dp">
+
+    <Button
+        android:id="@+id/swap_fragment"
+        style="@style/Widget.Material3.Button"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_gravity="bottom|center"
+        android:text="Swap Fragment"
+        android:textColor="@color/textColorPrimary" />
+</FrameLayout>

app/src/main/res/layout/fragment_text.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="wrap_content"
+    android:layout_height="wrap_content"
+    android:background="@color/colorPrimary"
+    android:orientation="vertical">
+
+    <TextView
+        android:id="@+id/textview"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="Lorem Ipsum" />
+
+</LinearLayout>

app/src/main/res/layout/main.xml

@@ -190,6 +190,16 @@
                 android:layout_marginVertical="4dp"
                 android:text="Compose"
                 android:textColor="@color/textColorPrimary" />
+
+            <Button
+                android:id="@+id/go_frame"
+                style="@style/Widget.Material3.Button"
+                android:layout_width="fill_parent"
+                android:layout_height="wrap_content"
+                android:layout_marginVertical="4dp"
+                android:text="Frame"
+                android:textColor="@color/textColorPrimary" />
+
         </LinearLayout>
 
     </ScrollView>

kotlin/src/main/java/app/rive/runtime/kotlin/RiveAnimationView.kt

@@ -16,6 +16,7 @@ import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.LifecycleObserver
 import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.ViewTreeLifecycleOwner
 import app.rive.runtime.kotlin.ResourceType.Companion.makeMaybeResource
 import app.rive.runtime.kotlin.controllers.ControllerState
 import app.rive.runtime.kotlin.controllers.ControllerStateManagement
@@ -160,6 +161,11 @@ open class RiveAnimationView(context: Context, attrs: AttributeSet? = null) :
     val playingStateMachines: HashSet<StateMachineInstance>
         get() = controller.playingStateMachines
 
+    /**
+     * The current [LifecycleOwner]. At the time of creation it will be the [Activity].
+     */
+    private var lifecycleOwner: LifecycleOwner? = getContextAsType<LifecycleOwner>()
+
     /**
      * Tracks the renderer attributes that need to be applied when this [View] within its lifecycle.
      */
@@ -211,6 +217,13 @@ open class RiveAnimationView(context: Context, attrs: AttributeSet? = null) :
                     loop = rendererAttributes.loop,
                     autoplay = rendererAttributes.autoplay,
                 )
+                /**
+                 * Attach the observer to give us lifecycle hooks
+                 * N.B. we're attaching in the constructor because the View can be created without
+                 * getting ever attached (e.g. in RecyclerViews) - we need to register the Observer
+                 * right away. [lifecycleOwner] at this point is going to be the wrapping Activity.
+                 */
+                lifecycleOwner?.lifecycle?.addObserver(lifecycleObserver)
 
                 // Initialize resource if we have one.
                 resourceFromValue?.let {
@@ -633,11 +646,33 @@ open class RiveAnimationView(context: Context, attrs: AttributeSet? = null) :
     }
 
     override fun createObserver(): LifecycleObserver {
-        return RiveViewLifecycleObserver()
+        return RiveViewLifecycleObserver(controller)
+    }
+
+    /**
+     * Ensure that the current [lifecycleOwner] is still valid in this View's tree.
+     * Upon creation, the original [lifecycleOwner] is going to be the [Activity] containing this
+     * [RiveAnimationView]. If the [View] is added within a different [LifecycleOwner] this needs
+     * to be validated again.
+     * This could arise when adding the [View] to a [Fragment].
+     * If the [lifecycleOwner] has changed, this is swapped out.
+     * Calling addObserver again will trigger a onCreate/onStart/onResume again.
+     */
+    private fun validateLifecycleOwner() {
+        val currentLifecycleOwner = ViewTreeLifecycleOwner.get(this)
+        currentLifecycleOwner?.let {
+            if (it != lifecycleOwner) {
+                lifecycleOwner?.lifecycle?.removeObserver(lifecycleObserver)
+                lifecycleOwner = currentLifecycleOwner
+                lifecycleOwner?.lifecycle?.addObserver(lifecycleObserver)
+            }
+        }
+
     }
 
     override fun onAttachedToWindow() {
         super.onAttachedToWindow()
+        validateLifecycleOwner()
         // If a File hasn't been set yet, try to initialize it.
         if (controller.file == null) {
             loadFileFromResource {
@@ -789,31 +824,39 @@ open class RiveAnimationView(context: Context, attrs: AttributeSet? = null) :
         }
         return true
     }
+}
 
+/**
+ * The [DefaultLifecycleObserver] tied to a [RiveAnimationView].
+ * Created within RiveAnimationView() to make sure things are properly cleaned up when the View
+ * is destroyed.
+ *
+ * N.B. since the [RiveAnimationView] can change [LifecycleOwner] during its lifetime, this is
+ * updated within [RiveAnimationView.onAttachedToWindow]. If there is a new [LifecycleOwner]
+ * [onCreate], [onStart], and [onResume] will be called again when it is registered.
+ */
+class RiveViewLifecycleObserver(private val controller: RiveFileController) :
+    DefaultLifecycleObserver {
+    override fun onCreate(owner: LifecycleOwner) {}
 
-    inner class RiveViewLifecycleObserver : DefaultLifecycleObserver {
-
-        override fun onCreate(owner: LifecycleOwner) {}
-
-        override fun onStart(owner: LifecycleOwner) {}
+    override fun onStart(owner: LifecycleOwner) {}
 
-        override fun onResume(owner: LifecycleOwner) {}
+    override fun onResume(owner: LifecycleOwner) {}
 
-        override fun onPause(owner: LifecycleOwner) {}
+    override fun onPause(owner: LifecycleOwner) {}
 
-        override fun onStop(owner: LifecycleOwner) {}
+    override fun onStop(owner: LifecycleOwner) {}
 
-        /**
-         * DefaultLifecycleObserver.onDestroy() is called when the LifecycleOwner's onDestroy() method
-         * is called.
-         * This typically happens when the Activity or Fragment is in the process of being permanently
-         * destroyed.
-         */
-        @CallSuper
-        override fun onDestroy(owner: LifecycleOwner) {
-            controller.release()
-            owner.lifecycle.removeObserver(this)
-        }
+    /**
+     * DefaultLifecycleObserver.onDestroy() is called when the LifecycleOwner's onDestroy() method
+     * is called.
+     * This typically happens when the Activity or Fragment is in the process of being permanently
+     * destroyed.
+     */
+    @CallSuper
+    override fun onDestroy(owner: LifecycleOwner) {
+        controller.release()
+        owner.lifecycle.removeObserver(this)
     }
 }
 

kotlin/src/main/java/app/rive/runtime/kotlin/RiveTextureView.kt

@@ -10,8 +10,8 @@ import android.view.Surface
 import android.view.TextureView
 import android.view.View
 import androidx.annotation.CallSuper
+import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.LifecycleObserver
-import androidx.lifecycle.LifecycleOwner
 import app.rive.runtime.kotlin.renderers.RendererSkia
 
 abstract class RiveTextureView(context: Context, attrs: AttributeSet? = null) :
@@ -28,7 +28,7 @@ abstract class RiveTextureView(context: Context, attrs: AttributeSet? = null) :
         getContextAsType<Activity>()!!
     }
 
-    private val lifecycleObserver: LifecycleObserver by lazy { createObserver() }
+    protected val lifecycleObserver: LifecycleObserver by lazy { createObserver() }
     protected var renderer: RendererSkia? = null
     private lateinit var viewSurface: Surface
     protected abstract fun createRenderer(): RendererSkia
@@ -44,12 +44,7 @@ abstract class RiveTextureView(context: Context, attrs: AttributeSet? = null) :
         (sInNS / refreshRateHz).toLong()
     }
 
-    init {
-        // Attach the observer to give us lifecycle hooks.
-        getContextAsType<LifecycleOwner>()?.lifecycle?.addObserver(lifecycleObserver)
-    }
-
-    private inline fun <reified T> getContextAsType(): T? {
+    protected inline fun <reified T> getContextAsType(): T? {
         var ctx = context
         while (ctx is ContextWrapper) {
             if (ctx is T) {