Fixed
Status Update
Comments
il...@google.com <il...@google.com> #2
Yigit, do you have time to fix it?
reemission of the same liveData is racy
reemission of the same liveData is racy
sa...@gmail.com <sa...@gmail.com> #3
yea i'll take it.
il...@google.com <il...@google.com> #4
Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.
ki...@google.com <ki...@google.com> #5
just emitting same live data reproduces the issue.
@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}
@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}
ki...@google.com <ki...@google.com> #6
With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)
an...@google.com <an...@google.com> #7
yea sorry immediate does not fix it.
I actually have a WIP fix for it:
https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186
if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}
I actually have a WIP fix for it:
if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}
an...@google.com <an...@google.com>
ki...@google.com <ki...@google.com> #8
Project: platform/frameworks/support
Branch: androidx-master-dev
commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019
Fix coroutine livedata race condition
This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.
We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)
Bug: 140249349
Test: BuildLiveDataTest#raceTest_*
Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40
M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt
https://android-review.googlesource.com/1112186
https://goto.google.com/android-sha1/af12e75e6b4110f48e44ca121466943909de8f06
Branch: androidx-master-dev
commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019
Fix coroutine livedata race condition
This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.
We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)
Bug: 140249349
Test: BuildLiveDataTest#raceTest_*
Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40
M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt
sa...@gmail.com <sa...@gmail.com> #9
Just to clarify, how should I go about integrating TransitionManager.beginDelayedTransition(mToolbar); with AndroidX Navigation?
sa...@gmail.com <sa...@gmail.com> #10
Never mind, I got it to work using this code:
Toolbar appBar = findViewById(R.id.toolbar);
setSupportActionBar(appBar);
// Hook up the app bar with the navigation
navController = Navigation.findNavController(this, R.id.nav_host_fragment);
NavigationUI.setupActionBarWithNavController(this, navController);
navController.addOnDestinationChangedListener((controller, destination, arguments) -> TransitionManager.beginDelayedTransition(appBar));
Thanks for your help!
Toolbar appBar = findViewById(R.id.toolbar);
setSupportActionBar(appBar);
// Hook up the app bar with the navigation
navController = Navigation.findNavController(this, R.id.nav_host_fragment);
NavigationUI.setupActionBarWithNavController(this, navController);
navController.addOnDestinationChangedListener((controller, destination, arguments) -> TransitionManager.beginDelayedTransition(appBar));
Thanks for your help!
ki...@google.com <ki...@google.com> #11
Ian - is it worth adding this somewhere in the navigation module docs?
il...@google.com <il...@google.com> #12
NavigationUI is the one triggering the change in app icons, so we'll add the code on our side.
ap...@google.com <ap...@google.com> #13
Project: platform/frameworks/support
Branch: androidx-master-dev
commit e28e8678478c7c04b55886e7071750cbdea89121
Author: jbwoods <jbwoods@google.com>
Date: Thu Jun 27 14:32:13 2019
Animate Toolbar Changes in NavigationUI
When animateLayoutChanges is set on a Toolbar, there are errors in how
the text within the toolbar is displayed if it has an arrow. The text
leaves a big space on destinations with no arrow.
This change checks so see if the icon will be null and then uses a
TransitionManager to animate the toolbar text into the proper position.
We only animate when the icon is going to disappear. If we were animate
when the icon is appearing, the new icon and text appear stacked when
the animation begins.
Test: Tested visually in app
BUG: 131403621
Change-Id: I97ff213bdeb6da3282287d9b572f33c74cf9e9e7
M navigation/navigation-ui/build.gradle
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
https://android-review.googlesource.com/1007867
https://goto.google.com/android-sha1/e28e8678478c7c04b55886e7071750cbdea89121
Branch: androidx-master-dev
commit e28e8678478c7c04b55886e7071750cbdea89121
Author: jbwoods <jbwoods@google.com>
Date: Thu Jun 27 14:32:13 2019
Animate Toolbar Changes in NavigationUI
When animateLayoutChanges is set on a Toolbar, there are errors in how
the text within the toolbar is displayed if it has an arrow. The text
leaves a big space on destinations with no arrow.
This change checks so see if the icon will be null and then uses a
TransitionManager to animate the toolbar text into the proper position.
We only animate when the icon is going to disappear. If we were animate
when the icon is appearing, the new icon and text appear stacked when
the animation begins.
Test: Tested visually in app
BUG: 131403621
Change-Id: I97ff213bdeb6da3282287d9b572f33c74cf9e9e7
M navigation/navigation-ui/build.gradle
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/CollapsingToolbarOnDestinationChangedListener.java
M navigation/navigation-ui/src/main/java/androidx/navigation/ui/ToolbarOnDestinationChangedListener.java
jb...@google.com <jb...@google.com> #14
This has been fixed internally and will be available in the Navigation 2.1.0-beta01 release.
Description
Version used: 2.0.0
Devices/Android versions reproduced on: all
My navigation destinations have different title bars:
* The top-level bar has no menu/back icon
* Each destination has its own title text
When I navigate between destinations, the Navigation library updates the app bar icon and title text, but no animation occurs. As a result, the app bar change is startling, whereas the content change is smooth. For the user, this makes the app feel like it has a bug or something.
Can you add a feature to animate changes to the app bar? Some animations I'd like to see are:
* If the icon is being added or removed, slide the title text from its original position to its new position
* Otherwise, either cross-fade the title or maybe match the destination transition animation
Thanks