-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] Only post DisableTimer to the queue if it's not on the UI thread #4866
Conversation
API 19 tests are failing on the 41424 test; nothing to do with this change, and fixed by e9274c0 when that gets merged into master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this may break something, but would it be better to put this logic inside the begin invoke? Why is being requested here is to run something on the main thread... Shouldn't the method itself first check? Wouldn't that be an improvement everywhere?
Which bit do you mean when you say "this logic"? |
The logic for if on main then run else use invoke. Basically the if check. |
Oh, I get what you're saying. So changing the Android implementation of
That would definitely break a few things. Most notably around layout, where there's logic which depends on putting the full layout pass onto queue. There are a couple of other places in the Android renderers which rely on that behavior (letting current operations finish before continuing). It would also make Android behave very differently from iOS and UWP (and presumably the other platforms); they also post the action to the queue to be run later. |
Description of Change
PR #4177 attempts to fix an issue with disabling the Android timer on a non-UI thread by using BeginInvokeOnMainThread, which posts the action to the main looper to be run later on the UI thread.
But if the call to DisableTimer is happening because the Ticker itself has finished all of the pending actions, DisableTimer needs to be called immediately in order to end the animations so that awaited animation calls can return. Since the animations are running on the UI thread to begin with, these calls to DisableTimer are already on the UI thread; they don't need to be posted there for later invocation.
This change adds a check in DisableTimer to see if it's already on the UI thread; if so, it is executed immediately. Otherwise, it is posted to the UI thread.
Issues Resolved
API Changes
None
Platforms Affected
Behavioral/Visual Changes
None
Before/After Screenshots
Not applicable
Testing Procedure
Run UI Test 39821; without the fix, it will never finish (i.e., the "Success" label will never be displayed).
PR Checklist