Skip to content

Commit

Permalink
Merge pull request #113 from YkSix/fix_possible_ConcurrentModificatio…
Browse files Browse the repository at this point in the history
…nException

Fix a potential crash caused by ConcurrentModificationException
  • Loading branch information
r-ralph committed May 22, 2024
2 parents daecaae + 8661fa2 commit 1fea8da
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
12 changes: 9 additions & 3 deletions apng-drawable/src/main/kotlin/com/linecorp/apng/ApngDrawable.kt
Original file line number Diff line number Diff line change
Expand Up @@ -324,15 +324,19 @@ class ApngDrawable @VisibleForTesting internal constructor(
isFirstLoop() &&
animationPrevDrawTimeMillisSnapShot == null
) {
animationCallbacks.forEach {
// Make a copy of the list for safe iteration
val callbacksCopy = animationCallbacks.toList()
callbacksCopy.forEach {
it.onAnimationStart(this)
}
} else if (
isLastFrame() &&
hasNextLoop() &&
frameChanged
) {
repeatAnimationCallbacks.forEach {
// Make a copy of the list for safe iteration
val callbacksCopy = repeatAnimationCallbacks.toList()
callbacksCopy.forEach {
// TODO: Remove `onRepeat` invocation at the next version.
@Suppress("DEPRECATION")
it.onRepeat(this, currentLoopIndexInternal + 2)
Expand All @@ -342,7 +346,9 @@ class ApngDrawable @VisibleForTesting internal constructor(
}
if (exceedsRepeatCountLimitation()) {
isStarted = false
animationCallbacks.forEach {
// Make a copy of the list for safe iteration
val callbacksCopy = animationCallbacks.toList()
callbacksCopy.forEach {
it.onAnimationEnd(this)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package com.linecorp.apng

import android.graphics.Canvas
import android.graphics.Rect
import android.graphics.drawable.Drawable
import androidx.vectordrawable.graphics.drawable.Animatable2Compat
import com.linecorp.apng.decoder.Apng
import com.linecorp.apng.utils.assertFailureMessage
import com.nhaarman.mockitokotlin2.any
Expand Down Expand Up @@ -207,6 +209,29 @@ class ApngDrawableTest {
)
}

@Test
fun testDraw_withRegisterAnimationCallbackMultipleTimes() {
currentTimeProvider.currentTimeMillis = 0L

repeat (3) {
target.registerAnimationCallback(object : Animatable2Compat.AnimationCallback() {
override fun onAnimationEnd(drawable: Drawable?) {
target.unregisterAnimationCallback(this)
}
})
}

target.start()

currentTimeProvider.currentTimeMillis = 10L

target.draw(canvas)

currentTimeProvider.currentTimeMillis = 1_000L

target.draw(canvas)
}

@Test
fun testSeekToFrame() {
target.seekToFrame(0, 0)
Expand Down

0 comments on commit 1fea8da

Please sign in to comment.