Merge pull request #2664 from DataDog/nogorodnikov/rum-9973/replace-addfirst-removefirst-usages

0xnm · web-flow · commit c696aa0c55c5 · 2025-05-21T09:54:27.000+02:00
RUM-9973: Remove `addFirst`, `removeFirst`, `removeLast` usages
diff --git a/detekt_custom.yml b/detekt_custom.yml
@@ -253,6 +253,8 @@ datadog:
       # endregion
       # region Java Collections
       - "java.util.Collections.newSetFromMap(kotlin.collections.MutableMap?):java.lang.IllegalArgumentException"
+      - "java.util.LinkedList.add(kotlin.Int, android.view.View):java.lang.IndexOutOfBoundsException"
+      - "java.util.LinkedList.add(kotlin.Int, com.datadog.android.webview.internal.rum.domain.WebViewNativeRumViewsCache.ViewEntry):java.lang.IndexOutOfBoundsException"
       - "java.util.LinkedList.offer(com.datadog.android.core.internal.data.upload.UploadWorker.UploadNextBatchTask):java.lang.ClassCastException,java.lang.NullPointerException,java.lang.IllegalArgumentException"
       - "java.util.LinkedList.removeFirst():java.util.NoSuchElementException"
       - "java.util.LinkedList.removeLast():java.util.NoSuchElementException"
@@ -652,15 +654,19 @@ datadog:
       - "java.util.LinkedList.addAll(kotlin.collections.Collection)"
       - "java.util.LinkedList.addAll(kotlin.Int, kotlin.collections.Collection)"
       - "java.util.LinkedList.addFirst(android.view.View?)"
+      - "java.util.LinkedList.addFirst(com.datadog.android.webview.internal.rum.domain.WebViewNativeRumViewsCache.ViewEntry?)"
       - "java.util.LinkedList.clear()"
       - "java.util.LinkedList.constructor()"
       - "java.util.LinkedList.constructor(kotlin.collections.MutableCollection?)"
       - "java.util.LinkedList.firstOrNull(kotlin.Function1)"
       - "java.util.LinkedList.forEach(kotlin.Function1)"
       - "java.util.LinkedList.isEmpty()"
       - "java.util.LinkedList.isNotEmpty()"
+      - "java.util.LinkedList.iterator()"
+      - "java.util.LinkedList.peekLast()"
       - "java.util.LinkedList.poll()"
       - "java.util.LinkedList.remove(com.datadog.android.privacy.TrackingConsentProviderCallback)"
+      - "java.util.LinkedList.remove(com.datadog.android.webview.internal.rum.domain.WebViewNativeRumViewsCache.ViewEntry)"
       - "java.util.LinkedHashMap.remove(kotlin.String)"
       - "java.util.Queue.addAll(kotlin.collections.Collection)"
       - "java.util.Queue.clear()"
@@ -820,10 +826,6 @@ datadog:
       - "java.util.HashMap.clear()"
       - "java.util.HashSet.addAll(kotlin.collections.Collection)"
       - "java.util.HashSet.find(kotlin.Function1)"
-      - "java.util.LinkedList.addFirst(com.datadog.android.webview.internal.rum.domain.WebViewNativeRumViewsCache.ViewEntry?)"
-      - "java.util.LinkedList.peekLast()"
-      - "java.util.LinkedList.remove(com.datadog.android.webview.internal.rum.domain.WebViewNativeRumViewsCache.ViewEntry)"
-      - "java.util.LinkedList.iterator()"
       - "java.util.Properties.constructor()"
       - "java.util.Properties.setProperty(kotlin.String?, kotlin.String?)"
       - "java.util.UUID.constructor(kotlin.Long, kotlin.Long)"
diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesListener.kt
@@ -135,13 +135,19 @@ internal class GesturesListener(
         isScroll: Boolean = false
     ): ViewTarget? {
         val queue = LinkedList<View>()
-        queue.addFirst(decorView)
+        // add(index, element) instead of addFirst here is on purpose, to prevent issues with old AGP being used
+        // when compiling with Android API 35.
+        // Index 0 is always safe
+        @Suppress("UnsafeThirdPartyFunctionCall")
+        queue.add(0, decorView)
         var target: ViewTarget? = null
         var composeViewDetected = false
         while (queue.isNotEmpty()) {
-            // removeFirst can't fail because we checked isNotEmpty
+            // removeAt(index) instead of removeFirst here is on purpose, to prevent issues
+            // with old AGP being used when compiling with Android API 35.
+            // removeAt can't fail because we checked isNotEmpty
             @Suppress("UnsafeThirdPartyFunctionCall")
-            val view = queue.removeFirst()
+            val view = queue.removeAt(0)
             composeViewDetected = composeViewDetected || isJetpackComposeView(view)
             val newTarget = if (isScroll) {
                 findTargetForScroll(view, x, y)
diff --git a/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/PendingTrace.java b/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/PendingTrace.java
@@ -362,6 +362,8 @@ public synchronized boolean clean() {
     @Override
     public void addFirst(final DDSpan span) {
         synchronized (this) {
+            // add(index, element) instead of addFirst here is on purpose, to prevent issues
+            // with old AGP being used when compiling with Android API 35.
             super.add(0, span);
         }
         completedSpanCount.incrementAndGet();
diff --git a/features/dd-sdk-android-webview/src/main/kotlin/com/datadog/android/webview/internal/rum/domain/WebViewNativeRumViewsCache.kt b/features/dd-sdk-android-webview/src/main/kotlin/com/datadog/android/webview/internal/rum/domain/WebViewNativeRumViewsCache.kt
@@ -63,12 +63,24 @@ internal class WebViewNativeRumViewsCache(
                     parentViewsHistoryQueue.first.timestamp <= entry.timestamp
                 )
         ) {
-            parentViewsHistoryQueue.addFirst(entry)
+            // add(index, element) instead of addFirst here is on purpose, to prevent issues
+            // with old AGP being used when compiling with Android API 35.
+            // Index 0 is always safe
+            @Suppress("UnsafeThirdPartyFunctionCall")
+            parentViewsHistoryQueue.add(0, entry)
         } else if (parentViewsHistoryQueue.first.viewId == entry.viewId) {
             // the function is synchronized and we are checking the size before
+            if (parentViewsHistoryQueue.isNotEmpty()) {
+                // removeAt(index) instead of removeFirst here is on purpose, to prevent issues
+                // with old AGP being used when compiling with Android API 35.
+                @Suppress("UnsafeThirdPartyFunctionCall")
+                parentViewsHistoryQueue.removeAt(0)
+            }
+            // add(index, element) instead of addFirst here is on purpose, to prevent issues
+            // with old AGP being used when compiling with Android API 35.
+            // Index 0 is always safe
             @Suppress("UnsafeThirdPartyFunctionCall")
-            parentViewsHistoryQueue.removeFirst()
-            parentViewsHistoryQueue.addFirst(entry)
+            parentViewsHistoryQueue.add(0, entry)
         }
     }
 
@@ -84,9 +96,11 @@ internal class WebViewNativeRumViewsCache(
             }
         }
         while (parentViewsHistoryQueue.size > DATA_CACHE_ENTRIES_LIMIT) {
+            // removeAt(index) instead of removeLast here is on purpose, to prevent issues
+            // with old AGP being used when compiling with Android API 35.
             // the function is synchronized and we are checking the size before
             @Suppress("UnsafeThirdPartyFunctionCall")
-            parentViewsHistoryQueue.removeLast()
+            parentViewsHistoryQueue.removeAt(parentViewsHistoryQueue.size - 1)
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -362,6 +362,8 @@ public synchronized boolean clean() {`
`362`	`362`	`@Override`
`363`	`363`	`public void addFirst(final DDSpan span) {`
`364`	`364`	`synchronized (this) {`
	`365`	`+ // add(index, element) instead of addFirst here is on purpose, to prevent issues`
	`366`	`+ // with old AGP being used when compiling with Android API 35.`
`365`	`367`	`super.add(0, span);`
`366`	`368`	`}`
`367`	`369`	`completedSpanCount.incrementAndGet();`
Original file line number	Diff line number	Diff line change
`@@ -63,12 +63,24 @@ internal class WebViewNativeRumViewsCache(`
`63`	`63`	`parentViewsHistoryQueue.first.timestamp <= entry.timestamp`
`64`	`64`	`)`
`65`	`65`	`) {`
`66`		`- parentViewsHistoryQueue.addFirst(entry)`
	`66`	`+ // add(index, element) instead of addFirst here is on purpose, to prevent issues`
	`67`	`+ // with old AGP being used when compiling with Android API 35.`
	`68`	`+ // Index 0 is always safe`
	`69`	`+ @Suppress("UnsafeThirdPartyFunctionCall")`
	`70`	`+ parentViewsHistoryQueue.add(0, entry)`
`67`	`71`	`} else if (parentViewsHistoryQueue.first.viewId == entry.viewId) {`
`68`	`72`	`// the function is synchronized and we are checking the size before`
	`73`	`+ if (parentViewsHistoryQueue.isNotEmpty()) {`
	`74`	`+ // removeAt(index) instead of removeFirst here is on purpose, to prevent issues`
	`75`	`+ // with old AGP being used when compiling with Android API 35.`
	`76`	`+ @Suppress("UnsafeThirdPartyFunctionCall")`
	`77`	`+ parentViewsHistoryQueue.removeAt(0)`
	`78`	`+ }`
	`79`	`+ // add(index, element) instead of addFirst here is on purpose, to prevent issues`
	`80`	`+ // with old AGP being used when compiling with Android API 35.`
	`81`	`+ // Index 0 is always safe`
`69`	`82`	`@Suppress("UnsafeThirdPartyFunctionCall")`
`70`		`- parentViewsHistoryQueue.removeFirst()`
`71`		`- parentViewsHistoryQueue.addFirst(entry)`
	`83`	`+ parentViewsHistoryQueue.add(0, entry)`
`72`	`84`	`}`
`73`	`85`	`}`
`74`	`86`
`@@ -84,9 +96,11 @@ internal class WebViewNativeRumViewsCache(`
`84`	`96`	`}`
`85`	`97`	`}`
`86`	`98`	`while (parentViewsHistoryQueue.size > DATA_CACHE_ENTRIES_LIMIT) {`
	`99`	`+ // removeAt(index) instead of removeLast here is on purpose, to prevent issues`
	`100`	`+ // with old AGP being used when compiling with Android API 35.`
`87`	`101`	`// the function is synchronized and we are checking the size before`
`88`	`102`	`@Suppress("UnsafeThirdPartyFunctionCall")`
`89`		`- parentViewsHistoryQueue.removeLast()`
	`103`	`+ parentViewsHistoryQueue.removeAt(parentViewsHistoryQueue.size - 1)`
`90`	`104`	`}`
`91`	`105`	`}`
`92`	`106`