Skip to content

Commit

Permalink
Consolidate Picasso's main handler into dispatcher (#2405)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gamepro65
gamepro65 authored Jul 5, 2023
1 parent 48b8f58 commit 8205c9d
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 46 deletions.
31 changes: 22 additions & 9 deletions picasso/src/main/java/com/squareup/picasso3/Dispatcher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ import android.net.NetworkInfo
import android.os.Handler
import android.util.Log
import androidx.annotation.CallSuper
import androidx.annotation.MainThread
import androidx.core.content.ContextCompat
import com.squareup.picasso3.BitmapHunter.Companion.forRequest
import com.squareup.picasso3.MemoryPolicy.Companion.shouldWriteToMemoryCache
import com.squareup.picasso3.NetworkPolicy.NO_CACHE
import com.squareup.picasso3.NetworkRequestHandler.ContentLengthException
import com.squareup.picasso3.Picasso.Priority.HIGH
import com.squareup.picasso3.RequestHandler.Result.Bitmap
import com.squareup.picasso3.Utils.OWNER_DISPATCHER
import com.squareup.picasso3.Utils.VERB_CANCELED
Expand Down Expand Up @@ -87,7 +87,7 @@ internal abstract class Dispatcher internal constructor(
// Shutdown the thread pool only if it is the one created by Picasso.
(service as? PicassoExecutorService)?.shutdown()
// Unregister network broadcast receiver on the main thread.
Picasso.HANDLER.post { receiver.unregister() }
mainThreadHandler.post { receiver.unregister() }
}

abstract fun dispatchSubmit(action: Action)
Expand All @@ -108,6 +108,10 @@ internal abstract class Dispatcher internal constructor(

abstract fun dispatchAirplaneModeChange(airplaneMode: Boolean)

abstract fun dispatchCompleteMain(hunter: BitmapHunter)

abstract fun dispatchBatchResumeMain(batch: MutableList<Action>)

fun performSubmit(action: Action, dismissFailed: Boolean = true) {
if (action.tag in pausedTags) {
pausedActions[action.getTarget()] = action
Expand Down Expand Up @@ -270,7 +274,7 @@ internal abstract class Dispatcher internal constructor(
}

if (batch.isNotEmpty()) {
mainThreadHandler.sendMessage(mainThreadHandler.obtainMessage(REQUEST_BATCH_RESUME, batch))
dispatchBatchResumeMain(batch)
}
}

Expand Down Expand Up @@ -343,6 +347,19 @@ internal abstract class Dispatcher internal constructor(
}
}

@MainThread
fun performCompleteMain(hunter: BitmapHunter) {
hunter.picasso.complete(hunter)
}

@MainThread
fun performBatchResumeMain(batch: List<Action>) {
for (i in batch.indices) {
val action = batch[i]
action.picasso.resumeAction(action)
}
}

private fun flushFailedActions() {
if (failedActions.isNotEmpty()) {
val iterator = failedActions.values.iterator()
Expand Down Expand Up @@ -390,12 +407,8 @@ internal abstract class Dispatcher internal constructor(
}
}

val message = mainThreadHandler.obtainMessage(HUNTER_COMPLETE, hunter)
if (hunter.priority == HIGH) {
mainThreadHandler.sendMessageAtFrontOfQueue(message)
} else {
mainThreadHandler.sendMessage(message)
}
dispatchCompleteMain(hunter)

logDelivery(hunter)
}

Expand Down
39 changes: 37 additions & 2 deletions picasso/src/main/java/com/squareup/picasso3/HandlerDispatcher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import android.os.HandlerThread
import android.os.Looper
import android.os.Message
import android.os.Process.THREAD_PRIORITY_BACKGROUND
import com.squareup.picasso3.Picasso.Priority.HIGH
import com.squareup.picasso3.Utils.flushStackLocalLeaks
import java.util.concurrent.ExecutorService

Expand All @@ -34,13 +35,15 @@ internal class HandlerDispatcher internal constructor(

private val dispatcherThread: DispatcherThread
private val handler: Handler
private val mainHandler: Handler

init {
dispatcherThread = DispatcherThread()
dispatcherThread.start()
val dispatcherThreadLooper = dispatcherThread.looper
flushStackLocalLeaks(dispatcherThreadLooper)
handler = DispatcherHandler(dispatcherThreadLooper, this)
mainHandler = MainDispatcherHandler(mainThreadHandler.looper, this)
}

override fun shutdown() {
Expand Down Expand Up @@ -91,9 +94,22 @@ internal class HandlerDispatcher internal constructor(
)
}

override fun dispatchCompleteMain(hunter: BitmapHunter) {
val message = mainHandler.obtainMessage(HUNTER_COMPLETE, hunter)
if (hunter.priority == HIGH) {
mainHandler.sendMessageAtFrontOfQueue(message)
} else {
mainHandler.sendMessage(message)
}
}

override fun dispatchBatchResumeMain(batch: MutableList<Action>) {
mainHandler.sendMessage(mainHandler.obtainMessage(REQUEST_BATCH_RESUME, batch))
}

private class DispatcherHandler(
looper: Looper,
private val dispatcher: Dispatcher
private val dispatcher: HandlerDispatcher
) : Handler(looper) {
override fun handleMessage(msg: Message) {
when (msg.what) {
Expand Down Expand Up @@ -133,14 +149,33 @@ internal class HandlerDispatcher internal constructor(
dispatcher.performAirplaneModeChange(msg.arg1 == AIRPLANE_MODE_ON)
}
else -> {
Picasso.HANDLER.post {
dispatcher.mainHandler.post {
throw AssertionError("Unknown handler message received: ${msg.what}")
}
}
}
}
}

private class MainDispatcherHandler(
looper: Looper,
val dispatcher: Dispatcher
) : Handler(looper) {
override fun handleMessage(msg: Message) {
when (msg.what) {
HUNTER_COMPLETE -> {
val hunter = msg.obj as BitmapHunter
dispatcher.performCompleteMain(hunter)
}
REQUEST_BATCH_RESUME -> {
val batch = msg.obj as List<Action>
dispatcher.performBatchResumeMain(batch)
}
else -> throw AssertionError("Unknown handler message received: " + msg.what)
}
}
}

internal class DispatcherThread : HandlerThread(
Utils.THREAD_PREFIX + DISPATCHER_THREAD_NAME,
THREAD_PRIORITY_BACKGROUND
Expand Down
22 changes: 1 addition & 21 deletions picasso/src/main/java/com/squareup/picasso3/Picasso.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import android.graphics.Color
import android.net.Uri
import android.os.Handler
import android.os.Looper
import android.os.Message
import android.widget.ImageView
import android.widget.RemoteViews
import androidx.annotation.DrawableRes
Expand All @@ -32,8 +31,6 @@ import androidx.lifecycle.Lifecycle.Event.ON_START
import androidx.lifecycle.Lifecycle.Event.ON_STOP
import androidx.lifecycle.LifecycleObserver
import androidx.lifecycle.OnLifecycleEvent
import com.squareup.picasso3.Dispatcher.Companion.HUNTER_COMPLETE
import com.squareup.picasso3.Dispatcher.Companion.REQUEST_BATCH_RESUME
import com.squareup.picasso3.MemoryPolicy.Companion.shouldReadFromMemoryCache
import com.squareup.picasso3.Picasso.LoadedFrom.MEMORY
import com.squareup.picasso3.RemoteViewsAction.RemoteViewsTarget
Expand Down Expand Up @@ -794,24 +791,7 @@ class Picasso internal constructor(

internal companion object {
@get:JvmName("-handler")
internal val HANDLER: Handler = object : Handler(Looper.getMainLooper()) {
override fun handleMessage(msg: Message) {
when (msg.what) {
HUNTER_COMPLETE -> {
val hunter = msg.obj as BitmapHunter
hunter.picasso.complete(hunter)
}
REQUEST_BATCH_RESUME -> {
val batch = msg.obj as List<Action>
for (i in batch.indices) {
val action = batch[i]
action.picasso.resumeAction(action)
}
}
else -> throw AssertionError("Unknown handler message received: " + msg.what)
}
}
}
internal val HANDLER = Handler(Looper.getMainLooper())
}
}

Expand Down
52 changes: 38 additions & 14 deletions picasso/src/test/java/com/squareup/picasso3/DispatcherTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import com.squareup.picasso3.MemoryPolicy.NO_STORE
import com.squareup.picasso3.NetworkRequestHandler.ContentLengthException
import com.squareup.picasso3.Picasso.LoadedFrom.MEMORY
import com.squareup.picasso3.Picasso.LoadedFrom.NETWORK
import com.squareup.picasso3.Request.Builder
import com.squareup.picasso3.TestUtils.TestDelegatingService
import com.squareup.picasso3.TestUtils.URI_1
import com.squareup.picasso3.TestUtils.URI_2
Expand Down Expand Up @@ -60,7 +61,7 @@ import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations.initMocks
import org.robolectric.RobolectricTestRunner
import org.robolectric.Shadows.shadowOf
import java.lang.AssertionError
import org.robolectric.shadows.ShadowLooper
import java.lang.Exception
import java.util.concurrent.ExecutorService
import java.util.concurrent.Future
Expand Down Expand Up @@ -231,48 +232,63 @@ class DispatcherTest {

@Test fun performCompleteCleansUpAndPostsToMain() {
val data = Request.Builder(URI_1).build()
val action = noopAction(data)
var completed = false
val action = noopAction(data, onComplete = { completed = true })
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action)
hunter.run()

dispatcher.performComplete(hunter)
ShadowLooper.idleMainLooper()

assertThat(dispatcher.hunterMap).isEmpty()
// TODO verify post to main thread.
assertThat(completed).isTrue()
}

@Test fun performCompleteCleansUpAndDoesNotPostToMainIfCancelled() {
val data = Request.Builder(URI_1).build()
val action = noopAction(data)
var completed = false
val action = noopAction(data, onComplete = { completed = true })
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action)
hunter.run()
hunter.future = FutureTask<Any>(mock(Runnable::class.java), null)
hunter.future!!.cancel(false)

dispatcher.performComplete(hunter)
ShadowLooper.idleMainLooper()

assertThat(dispatcher.hunterMap).isEmpty()
// TODO verify no main thread interactions.
assertThat(completed).isFalse()
}

@Test fun performErrorCleansUpAndPostsToMain() {
val exception = RuntimeException()
val action = mockAction(picasso, URI_KEY_1, URI_1, mockBitmapTarget(), tag = "tag")
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action)
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action, exception)
dispatcher.hunterMap[hunter.key] = hunter
hunter.run()

dispatcher.performError(hunter)
ShadowLooper.idleMainLooper()

assertThat(dispatcher.hunterMap).isEmpty()
// TODO verify post to main thread.
assertThat(action.errorException).isSameInstanceAs(exception)
}

@Test fun performErrorCleansUpAndDoesNotPostToMainIfCancelled() {
val exception = RuntimeException()
val action = mockAction(picasso, URI_KEY_1, URI_1, mockBitmapTarget(), tag = "tag")
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action)
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action, exception)
hunter.future = FutureTask(mock(Runnable::class.java), mock(Any::class.java))
hunter.future!!.cancel(false)
dispatcher.hunterMap[hunter.key] = hunter
hunter.run()

dispatcher.performError(hunter)
ShadowLooper.idleMainLooper()

assertThat(dispatcher.hunterMap).isEmpty()
// TODO verify no main thread interactions.
assertThat(action.errorException).isNull()
assertThat(action.completedResult).isNull()
}

@Test fun performRetrySkipsIfHunterIsCancelled() {
Expand Down Expand Up @@ -516,9 +532,17 @@ class DispatcherTest {
assertThat(hunter.actions).containsExactly(action2)
}

@Test fun performResumeTagIsIdempotent() {
@Test fun performResumeTagResumesPausedActions() {
val action = noopAction(Builder(URI_1).tag("tag").build())
val hunter = mockHunter(picasso, RequestHandler.Result.Bitmap(bitmap1, MEMORY), action)
dispatcher.hunterMap[URI_KEY_1] = hunter
assertThat(dispatcher.pausedActions).isEmpty()
dispatcher.performPauseTag("tag")
assertThat(dispatcher.pausedActions).containsEntry(action.getTarget(), action)

dispatcher.performResumeTag("tag")
// TODO verify no main thread interactions.

assertThat(dispatcher.pausedActions).isEmpty()
}

@Test fun performNetworkStateChangeFlushesFailedHunters() {
Expand Down Expand Up @@ -589,11 +613,11 @@ class DispatcherTest {
return HandlerDispatcher(context, service, Handler(getMainLooper()), cache)
}

private fun noopAction(data: Request): Action {
private fun noopAction(data: Request, onComplete: () -> Unit = { }): Action {
return object : Action(picasso, data) {
override fun complete(result: RequestHandler.Result) = Unit
override fun complete(result: RequestHandler.Result) = onComplete()
override fun error(e: Exception) = Unit
override fun getTarget(): Any = throw AssertionError()
override fun getTarget(): Any = this
}
}
}

0 comments on commit 8205c9d

Please sign in to comment.