Ensure awaitFrame() only awaits a single frame when used from the main looper #3437
Conversation
qwwdfsad
force-pushed
the
better-await-frame
branch
from
38dd273 to
0768ee5
Compare
qwwdfsad
changed the title
| mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) | ||
| expect(5) | ||
| mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) | ||
| finish(6) |
There was a problem hiding this comment.
Do these lines test anything?
|
|
||
| private suspend fun awaitFrameSlowPath(): Long = suspendCancellableCoroutine { cont -> | ||
| if (Looper.myLooper() === Looper.getMainLooper()) { // Check if we are already in the main looper thread | ||
| updateChoreographerAndPostFrameCallback(cont) |
There was a problem hiding this comment.
I saw you mention Dispatchers.Main.immediate not being good enough for this and something about intending to fix it. Could you elaborate?
There was a problem hiding this comment.
Well, I was hoping that adding Dispatchers.main.immediate would solve the problem, but it won't due to how we do structure our code: CancellableContinuation already incapsulates the dispatcher (for this case -- Dispatchers.Main) and we cannot "just" resume it in place.
Neither can we do Dispatchers.Main.immediate.resumeUndispatched(cont) due to how it is implemented -- that's a grey area in "how should 'resumeUndispatched' behave if extension receiver and actual dispatcher do not match". And they do not match because Dispatchers.Main.immediate and Dispatchers.Main are different dispatchers, non-transparent to each other
| expect(5) | ||
| mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) | ||
| finish(6) | ||
| finish(5) |
There was a problem hiding this comment.
Then it's finish(4), as the action after expect(4) also does nothing.
See #3431 for the additional context