Add timeouts to awaitDeferredWatchFace* and fix watchfaceOverlayStyle NPE
This patch adds a very conservative 10s timeout to
awaitDeferredWatchFaceImplThenRunOnUiThreadBlocking and
awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread.
In normal circumstances this shouldn't timeout including in OOBE
and DirectBoot scenarios where the CPU load is high. We've also
fixed a NPE if getWatchfaceOverlayStyle is called after close().
Test: Added a test for the NPE
Bug: 233574644
Change-Id: Idf71a26e966e164ab9827ddf108f6e6bd8fa0ab7
(cherry picked from commit d02de8fd4df50ab634bf0ab41d4381ac62dd20e2)
Merged-In: Idf71a26e966e164ab9827ddf108f6e6bd8fa0ab7
diff --git a/wear/watchface/watchface-client/src/androidTest/java/androidx/wear/watchface/client/test/WatchFaceControlClientTest.kt b/wear/watchface/watchface-client/src/androidTest/java/androidx/wear/watchface/client/test/WatchFaceControlClientTest.kt
index 53fe878..d41679a 100644
--- a/wear/watchface/watchface-client/src/androidTest/java/androidx/wear/watchface/client/test/WatchFaceControlClientTest.kt
+++ b/wear/watchface/watchface-client/src/androidTest/java/androidx/wear/watchface/client/test/WatchFaceControlClientTest.kt
@@ -1566,6 +1566,37 @@
}
@Test
+ fun watchfaceOverlayStyle_after_close() {
+ val wallpaperService = TestWatchfaceOverlayStyleWatchFaceService(
+ context,
+ surfaceHolder,
+ WatchFace.OverlayStyle(Color.valueOf(Color.RED), Color.valueOf(Color.BLACK))
+ )
+ val deferredInteractiveInstance = handlerCoroutineScope.async {
+ service.getOrCreateInteractiveWatchFaceClient(
+ "testId",
+ deviceConfig,
+ systemState,
+ null,
+ complications
+ )
+ }
+
+ // Create the engine which triggers creation of the interactive instance.
+ handler.post {
+ engine = wallpaperService.onCreateEngine() as WatchFaceService.EngineWrapper
+ }
+
+ // Wait for the instance to be created.
+ val interactiveInstance = awaitWithTimeout(deferredInteractiveInstance)
+
+ interactiveInstance.close()
+
+ assertThat(interactiveInstance.overlayStyle.backgroundColor).isNull()
+ assertThat(interactiveInstance.overlayStyle.foregroundColor).isNull()
+ }
+
+ @Test
fun computeUserStyleSchemaDigestHash() {
val headlessInstance1 = service.createHeadlessWatchFaceClient(
"id",
diff --git a/wear/watchface/watchface-client/src/main/java/androidx/wear/watchface/client/InteractiveWatchFaceClient.kt b/wear/watchface/watchface-client/src/main/java/androidx/wear/watchface/client/InteractiveWatchFaceClient.kt
index 9be90c8..1a3718d 100644
--- a/wear/watchface/watchface-client/src/main/java/androidx/wear/watchface/client/InteractiveWatchFaceClient.kt
+++ b/wear/watchface/watchface-client/src/main/java/androidx/wear/watchface/client/InteractiveWatchFaceClient.kt
@@ -332,15 +332,12 @@
override val overlayStyle: OverlayStyle
get() {
- return if (iInteractiveWatchFace.apiVersion >= 4) {
- val wireFormat = iInteractiveWatchFace.watchFaceOverlayStyle
- OverlayStyle(
- wireFormat.backgroundColor,
- wireFormat.foregroundColor
- )
- } else {
- OverlayStyle(null, null)
+ if (iInteractiveWatchFace.apiVersion >= 4) {
+ iInteractiveWatchFace.watchFaceOverlayStyle?.let {
+ return OverlayStyle(it.backgroundColor, it.foregroundColor)
+ }
}
+ return OverlayStyle(null, null)
}
override fun updateWatchFaceInstance(newInstanceId: String, userStyle: UserStyle) = TraceEvent(
diff --git a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/WatchFaceService.kt b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/WatchFaceService.kt
index cf14c70..c4ade27 100644
--- a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/WatchFaceService.kt
+++ b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/WatchFaceService.kt
@@ -98,6 +98,7 @@
import java.time.Instant
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeout
/** The wire format for [ComplicationData]. */
internal typealias WireComplicationData = android.support.wearable.complications.ComplicationData
@@ -317,6 +318,12 @@
/** The maximum permitted duration of [WatchFaceService.createWatchFace]. */
public const val MAX_CREATE_WATCHFACE_TIME_MILLIS: Int = 5000
+ /**
+ * The maximum delay for [awaitDeferredWatchFaceImplThenRunOnUiThreadBlocking] and
+ * [awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread] in milliseconds.
+ */
+ private const val AWAIT_DEFERRED_TIMEOUT = 10000L
+
/** The maximum reasonable wire size for a [UserStyleSchema] in bytes. */
internal const val MAX_REASONABLE_SCHEMA_WIRE_SIZE_BYTES = 50000
@@ -341,9 +348,11 @@
}
runBlocking {
try {
- val watchFaceImpl = engine.deferredWatchFaceImpl.await()
- withContext(engine.uiThreadCoroutineScope.coroutineContext) {
- task(watchFaceImpl)
+ withTimeout(AWAIT_DEFERRED_TIMEOUT) {
+ val watchFaceImpl = engine.deferredWatchFaceImpl.await()
+ withContext(engine.uiThreadCoroutineScope.coroutineContext) {
+ task(watchFaceImpl)
+ }
}
} catch (e: Exception) {
Log.e(HeadlessWatchFaceImpl.TAG, "Operation failed", e)
@@ -352,7 +361,7 @@
}
}
- internal fun <R> deferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
+ internal fun <R> awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
engine: WatchFaceService.EngineWrapper?,
traceName: String,
task: (watchFaceInitDetails: WatchFaceService.WatchFaceInitDetails) -> R
@@ -363,7 +372,9 @@
}
runBlocking {
try {
- task(engine.watchFaceInitDetails.await())
+ withTimeout(AWAIT_DEFERRED_TIMEOUT) {
+ task(engine.watchFaceInitDetails.await())
+ }
} catch (e: Exception) {
Log.e(HeadlessWatchFaceImpl.TAG, "Operation failed", e)
throw e
diff --git a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/HeadlessWatchFaceImpl.kt b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/HeadlessWatchFaceImpl.kt
index 4320488..40dfec1 100644
--- a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/HeadlessWatchFaceImpl.kt
+++ b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/HeadlessWatchFaceImpl.kt
@@ -98,7 +98,7 @@
) { watchFaceImpl -> watchFaceImpl.renderComplicationToBitmap(params) }
override fun getUserStyleSchema() =
- WatchFaceService.deferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
+ WatchFaceService.awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
engine,
"HeadlessWatchFaceImpl.getUserStyleSchema"
) { watchFaceInitDetails ->
@@ -106,7 +106,7 @@
}
override fun computeUserStyleSchemaDigestHash() =
- WatchFaceService.deferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
+ WatchFaceService.awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
engine,
"HeadlessWatchFaceImpl.computeUserStyleSchemaDigestHash"
) { watchFaceInitDetails ->
@@ -115,7 +115,7 @@
@OptIn(WatchFaceFlavorsExperimental::class)
override fun getUserStyleFlavors() =
- WatchFaceService.deferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
+ WatchFaceService.awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
engine,
"HeadlessWatchFaceImpl.getUserStyleFlavors"
) { watchFaceInitDetails ->
diff --git a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/InteractiveWatchFaceImpl.kt b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/InteractiveWatchFaceImpl.kt
index 17282bf..48adaeb 100644
--- a/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/InteractiveWatchFaceImpl.kt
+++ b/wear/watchface/watchface/src/main/java/androidx/wear/watchface/control/InteractiveWatchFaceImpl.kt
@@ -33,6 +33,8 @@
import androidx.wear.watchface.style.data.UserStyleWireFormat
import kotlinx.coroutines.launch
import java.time.Instant
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
/** An interactive watch face instance with SysUI and WCS facing interfaces.*/
internal class InteractiveWatchFaceImpl(
@@ -65,7 +67,7 @@
override fun unused18() {}
override fun getWatchFaceOverlayStyle(): WatchFaceOverlayStyleWireFormat? =
- WatchFaceService.deferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
+ WatchFaceService.awaitDeferredWatchFaceAndComplicationManagerThenRunOnBinderThread(
engine,
"InteractiveWatchFaceImpl.getWatchFaceOverlayStyle"
) { watchFaceInitDetails ->
@@ -113,15 +115,19 @@
}
override fun release(): Unit = TraceEvent("InteractiveWatchFaceImpl.release").use {
- uiThreadCoroutineScope.launch {
- engine?.let {
- try {
- it.deferredWatchFaceImpl.await()
- } catch (e: Exception) {
- // deferredWatchFaceImpl may have completed with an exception. This will
- // have already been reported so we can ignore it.
+ // Note this is a one way method called on a binder thread, so it shouldn't matter if we
+ // block.
+ runBlocking {
+ withContext(uiThreadCoroutineScope.coroutineContext) {
+ engine?.let {
+ try {
+ it.deferredWatchFaceImpl.await()
+ } catch (e: Exception) {
+ // deferredWatchFaceImpl may have completed with an exception. This will
+ // have already been reported so we can ignore it.
+ }
+ InteractiveInstanceManager.releaseInstance(instanceId)
}
- InteractiveInstanceManager.releaseInstance(instanceId)
}
}
}
@@ -177,8 +183,11 @@
}
fun onDestroy() {
- uiThreadCoroutineScope.launch {
- engine = null
+ // Note this is almost certainly called on the ui thread, from release() above.
+ runBlocking {
+ withContext(uiThreadCoroutineScope.coroutineContext) {
+ engine = null
+ }
}
}
}
