Use a Better Name For Logcat Tabs

Includes a refactor that moves code from DeviceComboBoxDeviceTracker to a new class DeviceFactory

Fixes: 239437396
Test: Existing and Manually
Change-Id: I4a2b0da2e07f07281e87fe387b5cb20bd3cfbe7a

3 files changed, 103 insertions, 63 deletions

diff --git a/logcat/src/com/android/tools/idea/logcat/LogcatToolWindowFactory.kt b/logcat/src/com/android/tools/idea/logcat/LogcatToolWindowFactory.kt
index b0bd3365418..1e50b9079b5 100644
--- a/logcat/src/com/android/tools/idea/logcat/LogcatToolWindowFactory.kt
+++ b/logcat/src/com/android/tools/idea/logcat/LogcatToolWindowFactory.kt
@@ -18,8 +18,12 @@ package com.android.tools.idea.logcat
 import com.android.tools.adtui.TreeWalker
 import com.android.tools.adtui.toolwindow.splittingtabs.SplittingTabsToolWindowFactory
 import com.android.tools.idea.adb.processnamemonitor.ProcessNameMonitor
+import com.android.tools.idea.adblib.AdbLibService
+import com.android.tools.idea.concurrency.AndroidCoroutineScope
+import com.android.tools.idea.concurrency.AndroidDispatchers.uiThread
 import com.android.tools.idea.isAndroidEnvironment
 import com.android.tools.idea.logcat.LogcatExperimentalSettings.Companion.getInstance
+import com.android.tools.idea.logcat.devices.DeviceFactory
 import com.android.tools.idea.logcat.filters.LogcatFilterColorSettingsPage
 import com.android.tools.idea.logcat.messages.LogcatColorSettingsPage
 import com.android.tools.idea.logcat.messages.LogcatColors
@@ -33,8 +37,9 @@ import com.intellij.openapi.wm.ToolWindow
 import com.intellij.openapi.wm.ex.ToolWindowEx
 import com.intellij.ui.content.Content
 import com.intellij.util.text.UniqueNameGenerator
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import org.jetbrains.annotations.VisibleForTesting
-import java.awt.EventQueue
 
 internal class LogcatToolWindowFactory : SplittingTabsToolWindowFactory(), DumbAware {
 
@@ -63,22 +68,24 @@ internal class LogcatToolWindowFactory : SplittingTabsToolWindowFactory(), DumbA
   }
 
   private fun showLogcat(toolWindow: ToolWindowEx, serialNumber: String) {
-    EventQueue.invokeLater {
-      toolWindow.activate {
-        val contentManager = toolWindow.contentManager
-        val count = contentManager.contentCount
-        for (i in 0 until count) {
-          val content = contentManager.getContent(i)
-          content?.findLogcatPresenters()?.forEach {
-            if (it.getConnectedDevice()?.serialNumber == serialNumber) {
-              contentManager.setSelectedContent(content, true)
-              return@activate
+    AndroidCoroutineScope(toolWindow.project).launch {
+      val device = DeviceFactory(AdbLibService.getSession(toolWindow.project)).createDevice(serialNumber)
+      withContext(uiThread) {
+        toolWindow.activate {
+          val contentManager = toolWindow.contentManager
+          val count = contentManager.contentCount
+          for (i in 0 until count) {
+            val content = contentManager.getContent(i)
+            content?.findLogcatPresenters()?.forEach {
+              if (it.getConnectedDevice()?.serialNumber == serialNumber) {
+                contentManager.setSelectedContent(content, true)
+                return@activate
+              }
             }
           }
+          createNewTab(toolWindow, device.name).findLogcatPresenters().firstOrNull()?.selectDevice(serialNumber)
         }
-        // TODO(aalbert): Getting a pretty name for a device is complicated since it requires fetching properties from device. Use serial
-        //  number as a tab name for now.
-        createNewTab(toolWindow, serialNumber).findLogcatPresenters().firstOrNull()?.selectDevice(serialNumber)
+
       }
     }
   }
diff --git a/logcat/src/com/android/tools/idea/logcat/devices/DeviceComboBoxDeviceTracker.kt b/logcat/src/com/android/tools/idea/logcat/devices/DeviceComboBoxDeviceTracker.kt
index 6510169f428..e2eda0b68a2 100644
--- a/logcat/src/com/android/tools/idea/logcat/devices/DeviceComboBoxDeviceTracker.kt
+++ b/logcat/src/com/android/tools/idea/logcat/devices/DeviceComboBoxDeviceTracker.kt
@@ -17,15 +17,7 @@ package com.android.tools.idea.logcat.devices
 
 import com.android.adblib.AdbSession
 import com.android.adblib.DeviceInfo
-import com.android.adblib.DevicePropertyNames.RO_BOOT_QEMU_AVD_NAME
-import com.android.adblib.DevicePropertyNames.RO_BUILD_VERSION_RELEASE
-import com.android.adblib.DevicePropertyNames.RO_BUILD_VERSION_SDK
-import com.android.adblib.DevicePropertyNames.RO_KERNEL_QEMU_AVD_NAME
-import com.android.adblib.DevicePropertyNames.RO_PRODUCT_MANUFACTURER
-import com.android.adblib.DevicePropertyNames.RO_PRODUCT_MODEL
-import com.android.adblib.DeviceSelector
 import com.android.adblib.DeviceState.ONLINE
-import com.android.adblib.shellAsText
 import com.android.tools.idea.adblib.AdbLibService
 import com.android.tools.idea.logcat.devices.DeviceEvent.Added
 import com.android.tools.idea.logcat.devices.DeviceEvent.StateChanged
@@ -42,11 +34,8 @@ import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOn
 import java.io.IOException
-import java.time.Duration
 import kotlin.coroutines.CoroutineContext
 
-private val ADB_TIMEOUT = Duration.ofMillis(1000)
-
 /**
  * An implementation of IDeviceComboBoxDeviceTracker that uses an [AdbSession]
  */
@@ -77,11 +66,12 @@ internal class DeviceComboBoxDeviceTracker(
   private suspend fun FlowCollector<DeviceEvent>.trackDevicesInternal() {
     val onlineDevicesBySerial = mutableMapOf<String, Device>()
     val allDevicesById = mutableMapOf<String, Device>()
+    val deviceFactory = DeviceFactory(adbSession)
 
     // Initialize state by reading all current devices
     coroutineScope {
       val devices = adbSession.hostServices.devices()
-      devices.filter { it.isOnline() }.map { async { it.toDevice() } }.awaitAll().forEach {
+      devices.filter { it.isOnline() }.map { async { deviceFactory.createDevice(it.serialNumber) } }.awaitAll().forEach {
         onlineDevicesBySerial[it.serialNumber] = it
         allDevicesById[it.deviceId] = it
         emit(Added(it))
@@ -104,7 +94,7 @@ internal class DeviceComboBoxDeviceTracker(
       devices.values.forEach {
         val serialNumber = it.serialNumber
         if (!onlineDevicesBySerial.containsKey(serialNumber)) {
-          val device = it.toDevice()
+          val device = deviceFactory.createDevice(it.serialNumber)
           if (allDevicesById.containsKey(device.deviceId)) {
             emit(StateChanged(device))
           }
@@ -126,42 +116,6 @@ internal class DeviceComboBoxDeviceTracker(
       }
     }
   }
-
-  private suspend fun DeviceInfo.toDevice(): Device {
-    if (serialNumber.startsWith("emulator-")) {
-      val properties = getProperties(RO_BUILD_VERSION_RELEASE, RO_BUILD_VERSION_SDK, RO_BOOT_QEMU_AVD_NAME, RO_KERNEL_QEMU_AVD_NAME)
-      return Device.createEmulator(
-        serialNumber,
-        isOnline = true,
-        properties.getValue(RO_BUILD_VERSION_RELEASE).toIntOrNull() ?: 0,
-        properties.getValue(RO_BUILD_VERSION_SDK).toIntOrNull() ?: 0,
-        getAvdName(properties))
-    }
-    else {
-      val properties = getProperties(RO_BUILD_VERSION_RELEASE, RO_BUILD_VERSION_SDK, RO_PRODUCT_MANUFACTURER, RO_PRODUCT_MODEL)
-      return Device.createPhysical(
-        serialNumber,
-        isOnline = true,
-        properties.getValue(RO_BUILD_VERSION_RELEASE).toIntOrNull() ?: 0,
-        properties.getValue(RO_BUILD_VERSION_SDK).toIntOrNull() ?: 0,
-        properties.getValue(RO_PRODUCT_MANUFACTURER),
-        properties.getValue(RO_PRODUCT_MODEL))
-    }
-  }
-
-  private fun DeviceInfo.getAvdName(properties: Map<String, String>): String =
-    properties.getValue(RO_BOOT_QEMU_AVD_NAME).ifBlank { properties.getValue(RO_KERNEL_QEMU_AVD_NAME) }.ifBlank {
-      LOGGER.warn("Emulator has no avd_name property")
-      serialNumber
-    }
-
-  @Suppress("SameParameterValue") // The inspection is wrong. It only considers the first arg in the vararg
-  private suspend fun DeviceInfo.getProperties(vararg properties: String): Map<String, String> {
-    val selector = DeviceSelector.fromSerialNumber(serialNumber)
-    val command = properties.joinToString(" ; ") { "getprop $it" }
-    val lines = adbSession.deviceServices.shellAsText(selector, command, commandTimeout = ADB_TIMEOUT).split("\n")
-    return properties.withIndex().associate { it.value to lines[it.index].trimEnd('\r') }
-  }
 }
 
 private fun DeviceInfo.isOnline(): Boolean = deviceState == ONLINE
diff --git a/logcat/src/com/android/tools/idea/logcat/devices/DeviceFactory.kt b/logcat/src/com/android/tools/idea/logcat/devices/DeviceFactory.kt
new file mode 100644
index 00000000000..10589919c85
--- /dev/null
+++ b/logcat/src/com/android/tools/idea/logcat/devices/DeviceFactory.kt
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2022 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.tools.idea.logcat.devices
+
+import com.android.adblib.AdbSession
+import com.android.adblib.DevicePropertyNames.RO_BOOT_QEMU_AVD_NAME
+import com.android.adblib.DevicePropertyNames.RO_BUILD_VERSION_RELEASE
+import com.android.adblib.DevicePropertyNames.RO_BUILD_VERSION_SDK
+import com.android.adblib.DevicePropertyNames.RO_KERNEL_QEMU_AVD_NAME
+import com.android.adblib.DevicePropertyNames.RO_PRODUCT_MANUFACTURER
+import com.android.adblib.DevicePropertyNames.RO_PRODUCT_MODEL
+import com.android.adblib.DeviceSelector
+import com.android.adblib.shellAsText
+import com.android.tools.idea.logcat.util.LOGGER
+import java.time.Duration
+
+private val ADB_TIMEOUT = Duration.ofMillis(1000)
+
+/** Reads from a running device and creates a [Device] */
+internal class DeviceFactory(private val adbSession: AdbSession) {
+  suspend fun createDevice(serialNumber: String): Device {
+    if (serialNumber.startsWith("emulator-")) {
+      val properties = getProperties(
+        serialNumber,
+        RO_BUILD_VERSION_RELEASE,
+        RO_BUILD_VERSION_SDK,
+        RO_BOOT_QEMU_AVD_NAME,
+        RO_KERNEL_QEMU_AVD_NAME)
+      return Device.createEmulator(
+        serialNumber,
+        isOnline = true,
+        properties.getValue(RO_BUILD_VERSION_RELEASE).toIntOrNull() ?: 0,
+        properties.getValue(RO_BUILD_VERSION_SDK).toIntOrNull() ?: 0,
+        getAvdName(serialNumber, properties))
+    }
+    else {
+      val properties = getProperties(
+        serialNumber,
+        RO_BUILD_VERSION_RELEASE,
+        RO_BUILD_VERSION_SDK,
+        RO_PRODUCT_MANUFACTURER,
+        RO_PRODUCT_MODEL)
+      return Device.createPhysical(
+        serialNumber,
+        isOnline = true,
+        properties.getValue(RO_BUILD_VERSION_RELEASE).toIntOrNull() ?: 0,
+        properties.getValue(RO_BUILD_VERSION_SDK).toIntOrNull() ?: 0,
+        properties.getValue(RO_PRODUCT_MANUFACTURER),
+        properties.getValue(RO_PRODUCT_MODEL))
+    }
+  }
+
+  @Suppress("SameParameterValue") // The inspection is wrong. It only considers the first arg in the vararg
+  private suspend fun getProperties(serialNumber: String, vararg properties: String): Map<String, String> {
+    val selector = DeviceSelector.fromSerialNumber(serialNumber)
+    val command = properties.joinToString(" ; ") { "getprop $it" }
+    val lines = adbSession.deviceServices.shellAsText(selector, command, commandTimeout = ADB_TIMEOUT).split("\n")
+    return properties.withIndex().associate { it.value to lines[it.index].trimEnd('\r') }
+  }
+
+  private fun getAvdName(serialNumber: String, properties: Map<String, String>): String =
+    properties.getValue(RO_BOOT_QEMU_AVD_NAME).ifBlank { properties.getValue(RO_KERNEL_QEMU_AVD_NAME) }.ifBlank {
+      LOGGER.warn("Emulator has no avd_name property")
+      serialNumber
+    }
+}
\ No newline at end of file