[Sb refactor] Add a reusable method to TableLogBufferFactory

The majority of uses of LogBuffer and TableLogBuffer will be set up in
the dagger graph, as the controllers logging to it are typically known
at compile time.

In the case of mobile, we are lazily creating the log buffers, and want
to reference them by name. This CL adds a cache to
TableLogBufferFactory, which is opt-in to avoid wasting memory caching
every single log buffer. They are all already retained by DumpManager
which throws an exception if a reregister is caught.

Test: MobileConnectionsRepositoryTest
Fixes: 264257095
Change-Id: I0e93e4317eb8108288983ffa01ccbc092e1e242e

packages/SystemUI/src/com/android/systemui/log/table/TableLogBufferFactory.kt

@@ -29,6 +29,18 @@ constructor(
     private val dumpManager: DumpManager,
     private val systemClock: SystemClock,
 ) {
+    private val existingBuffers = mutableMapOf<String, TableLogBuffer>()
+
+    /**
+     * Creates a new [TableLogBuffer]. This method should only be called from static contexts, where
+     * it is guaranteed only to be created one time. See [getOrCreate] for a cache-aware method of
+     * obtaining a buffer.
+     *
+     * @param name a unique table name
+     * @param maxSize the buffer max size. See [adjustMaxSize]
+     *
+     * @return a new [TableLogBuffer] registered with [DumpManager]
+     */
     fun create(
         name: String,
         maxSize: Int,
@@ -37,4 +49,23 @@ constructor(
         dumpManager.registerNormalDumpable(name, tableBuffer)
         return tableBuffer
     }
+
+    /**
+     * Log buffers are retained indefinitely by [DumpManager], so that they can be represented in
+     * bugreports. Because of this, many of them are created statically in the Dagger graph.
+     *
+     * In the case where you have to create a logbuffer with a name only known at runtime, this
+     * method can be used to lazily create a table log buffer which is then cached for reuse.
+     *
+     * @return a [TableLogBuffer] suitable for reuse
+     */
+    fun getOrCreate(
+        name: String,
+        maxSize: Int,
+    ): TableLogBuffer =
+        existingBuffers.getOrElse(name) {
+            val buffer = create(name, maxSize)
+            existingBuffers[name] = buffer
+            buffer
+        }
 }

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepository.kt

@@ -149,7 +149,7 @@ constructor(
     }
 
     private fun createDemoMobileConnectionRepo(subId: Int): DemoMobileConnectionRepository {
-        val tableLogBuffer = logFactory.create("DemoMobileConnectionLog [$subId]", 100)
+        val tableLogBuffer = logFactory.getOrCreate("DemoMobileConnectionLog [$subId]", 100)
 
         return DemoMobileConnectionRepository(
             subId,

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt

@@ -308,7 +308,7 @@ class MobileConnectionRepositoryImpl(
             networkNameSeparator: String,
             globalMobileDataSettingChangedEvent: Flow<Unit>,
         ): MobileConnectionRepository {
-            val mobileLogger = logFactory.create(tableBufferLogName(subId), 100)
+            val mobileLogger = logFactory.getOrCreate(tableBufferLogName(subId), 100)
 
             return MobileConnectionRepositoryImpl(
                 context,

packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferFactoryTest.kt

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.log.table
+
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.dump.DumpManager
+import com.android.systemui.util.mockito.mock
+import com.android.systemui.util.time.FakeSystemClock
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+
+@SmallTest
+class TableLogBufferFactoryTest : SysuiTestCase() {
+    private val dumpManager: DumpManager = mock()
+    private val systemClock = FakeSystemClock()
+    private val underTest = TableLogBufferFactory(dumpManager, systemClock)
+
+    @Test
+    fun `create - always creates new instance`() {
+        val b1 = underTest.create(NAME_1, SIZE)
+        val b1_copy = underTest.create(NAME_1, SIZE)
+        val b2 = underTest.create(NAME_2, SIZE)
+        val b2_copy = underTest.create(NAME_2, SIZE)
+
+        assertThat(b1).isNotSameInstanceAs(b1_copy)
+        assertThat(b1).isNotSameInstanceAs(b2)
+        assertThat(b2).isNotSameInstanceAs(b2_copy)
+    }
+
+    @Test
+    fun `getOrCreate - reuses instance`() {
+        val b1 = underTest.getOrCreate(NAME_1, SIZE)
+        val b1_copy = underTest.getOrCreate(NAME_1, SIZE)
+        val b2 = underTest.getOrCreate(NAME_2, SIZE)
+        val b2_copy = underTest.getOrCreate(NAME_2, SIZE)
+
+        assertThat(b1).isSameInstanceAs(b1_copy)
+        assertThat(b2).isSameInstanceAs(b2_copy)
+        assertThat(b1).isNotSameInstanceAs(b2)
+        assertThat(b1_copy).isNotSameInstanceAs(b2_copy)
+    }
+
+    companion object {
+        const val NAME_1 = "name 1"
+        const val NAME_2 = "name 2"
+
+        const val SIZE = 8
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt

@@ -37,6 +37,7 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.log.table.TableLogBuffer
 import com.android.systemui.log.table.TableLogBufferFactory
 import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel
+import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel
 import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel
 import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
@@ -46,6 +47,7 @@ import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.mock
 import com.android.systemui.util.mockito.whenever
 import com.android.systemui.util.settings.FakeSettings
+import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -94,7 +96,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             }
         }
 
-        whenever(logBufferFactory.create(anyString(), anyInt())).thenAnswer { _ ->
+        whenever(logBufferFactory.getOrCreate(anyString(), anyInt())).thenAnswer { _ ->
             mock<TableLogBuffer>()
         }
 
@@ -292,13 +294,13 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             // Get repos to trigger creation
             underTest.getRepoForSubId(SUB_1_ID)
             verify(logBufferFactory)
-                .create(
+                .getOrCreate(
                     eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_1_ID)),
                     anyInt(),
                 )
             underTest.getRepoForSubId(SUB_2_ID)
             verify(logBufferFactory)
-                .create(
+                .getOrCreate(
                     eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_2_ID)),
                     anyInt(),
                 )
@@ -306,6 +308,46 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() {
             job.cancel()
         }
 
+    @Test
+    fun `connection repository factory - reuses log buffers for same connection`() =
+        runBlocking(IMMEDIATE) {
+            val realLoggerFactory = TableLogBufferFactory(mock(), FakeSystemClock())
+
+            connectionFactory =
+                MobileConnectionRepositoryImpl.Factory(
+                    fakeBroadcastDispatcher,
+                    context = context,
+                    telephonyManager = telephonyManager,
+                    bgDispatcher = IMMEDIATE,
+                    globalSettings = globalSettings,
+                    logger = logger,
+                    mobileMappingsProxy = mobileMappings,
+                    scope = scope,
+                    logFactory = realLoggerFactory,
+                )
+
+            // Create two connections for the same subId. Similar to if the connection appeared
+            // and disappeared from the connectionFactory's perspective
+            val connection1 =
+                connectionFactory.build(
+                    1,
+                    NetworkNameModel.Default("default_name"),
+                    "-",
+                    underTest.globalMobileDataSettingChangedEvent,
+                )
+
+            val connection1_repeat =
+                connectionFactory.build(
+                    1,
+                    NetworkNameModel.Default("default_name"),
+                    "-",
+                    underTest.globalMobileDataSettingChangedEvent,
+                )
+
+            assertThat(connection1.tableLogBuffer)
+                .isSameInstanceAs(connection1_repeat.tableLogBuffer)
+        }
+
     @Test
     fun mobileConnectivity_default() {
         assertThat(underTest.defaultMobileNetworkConnectivity.value)