Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 25b52121 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Fix recursive loop in DisplayComponent lifecycle listeners

DisplayComponentInstanceProvider was calling the lifecycle "start" method during the instance creation, but the start method was trying to access the instance itself, causing a recursive loop.

This adds another callback to initialize the instance after it is created and cached.

Bug: 433203587
Flag: NONE - bugfix
Test: PerDisplayInstanceRepositoryImplTest, DisplayComponentInstanceProviderTest
Change-Id: I4efe87f34fc7f0fae6135b68b2565c7c518a6153
parent af469e17
Loading
Loading
Loading
Loading
+47 −10
Original line number Diff line number Diff line
@@ -79,6 +79,20 @@ interface PerDisplayInstanceProviderWithTeardown<T> : PerDisplayInstanceProvider
    fun destroyInstance(instance: T)
}

/**
 * Extends [PerDisplayInstanceProvider], adding support for setting up an instance after it's
 * created.
 *
 * This is useful to run custom setup after an instance of the repository is created and cached. Why
 * not doing it in the [createInstance] itself? if some deps of the setup code tries to get the
 * instance again through the repository, it would cause a recursive loop (as it will try to create
 * a new instance). Splitting this into another method helps avoiding the recursion.
 */
interface PerDisplayInstanceProviderWithSetup<T> : PerDisplayInstanceProvider<T> {
    /** Sets up a previously created instance of `T`. */
    fun setupInstance(instance: T)
}

/**
 * Provides access to per-display instances of type `T`.
 *
@@ -225,9 +239,19 @@ constructor(
            return null
        }

        // Let's not let this method return the new instance until the possible setup for it was
        // executed.
        // There is no need to synchronize the other accesses to the map as it's already a
        // concurrent one.
        return synchronized(this) {
            var newlyCreated = false
            // If it doesn't exist, create it and put it in the map.
        return perDisplayInstances.computeIfAbsent(displayId) { key ->
            Log.d(TAG, "<$debugName> creating instance for displayId=$key, as it wasn't available.")
            val instance =
                perDisplayInstances.computeIfAbsent(displayId) { key ->
                    Log.d(
                        TAG,
                        "<$debugName> creating instance for displayId=$key, as it wasn't available.",
                    )
                    val instance =
                        traceSection({ "creating instance of $debugName for displayId=$key" }) {
                            instanceProvider.createInstance(key)
@@ -238,6 +262,19 @@ constructor(
                            "<$debugName> returning null because createInstance($key) returned null.",
                        )
                    }
                    newlyCreated = true
                    instance
                }

            if (
                newlyCreated &&
                    instance != null &&
                    instanceProvider is PerDisplayInstanceProviderWithSetup
            ) {
                traceSection({ "setting up instance of $debugName for displayId=$displayId" }) {
                    instanceProvider.setupInstance(instance)
                }
            }
            instance
        }
    }