Make ResourceIcon#get thread safe

Putting a new element in a SparseArray copies the backing array. As
ResourceIcon#get is called both from the main and BG thread, this is not
safe and sometimes leads to a bad state for ResourceIcon#ICONS. In
particular, keys would be assigned to wrong Icon, leading to QS tiles
retrieving and displaying the wrong icon.

Test: atest ResourceIconTest (it fails without the synchronize)
Test: no observable delays caused by icon updates
Fixes: 132435191

Change-Id: I9b3f535d925aa23529b59c0eacb4e67186fdabd2

2 files changed, 42 insertions, 1 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java
index 1f857ff7b5ea..90455395db45 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java
@@ -551,7 +551,7 @@ public abstract class QSTileImpl<TState extends State> implements QSTile, Lifecy
             mResId = resId;
         }
 
-        public static Icon get(int resId) {
+        public static synchronized Icon get(int resId) {
             Icon icon = ICONS.get(resId);
             if (icon == null) {
                 icon = new ResourceIcon(resId);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/ResourceIconTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/ResourceIconTest.kt
new file mode 100644
index 000000000000..9c1cad62d1aa
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/ResourceIconTest.kt
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2019 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.qs.tileimpl
+
+import android.testing.AndroidTestingRunner
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidTestingRunner::class)
+@SmallTest
+class ResourceIconTest : SysuiTestCase() {
+
+    val resIds = (1..1000).toList()
+
+    @Test
+    fun testMultipleThreadedPut() {
+        resIds.parallelStream().forEach {
+            QSTileImpl.ResourceIcon.get(it)
+        }
+        resIds.forEach {
+            assertEquals(it, (QSTileImpl.ResourceIcon.get(it) as QSTileImpl.ResourceIcon).mResId)
+        }
+    }
+}