Status Update
Comments
da...@gmail.com <da...@gmail.com> #2
This requirement for addObserver
to be called on the main thread is part of the 2.3.0-alpha06
release notes
LifecycleRegistry now verifies that its methods are called on main thread. It was always a requirement for lifecycles of activities, fragments etc. An addition of observers from non-main threads resulted in hard to catch crashes in runtime.
It affects all APIs that use Lifecycle, including Navigation.
wu...@google.com <wu...@google.com> #3
Thanks! Somehow I've missed that.
sc...@google.com <sc...@google.com> #4
Branch: androidx-main
commit 76230d4cbcf193ad3c01e2fb317a1bb3c571b1de
Author: Ian Lake <ilake@google.com>
Date: Tue Apr 06 12:53:25 2021
Mark navigate/setGraph as @MainThread
With the upgrade to Lifecycle 2.3.1,
Lifecycle now enforces that all creation and
changes to the Lifecycle state (including adding
and removing observers) should be done on the
main thread.
By marking navigate() and setGraph() as
`@MainThread`, we can give developeres better
visibility that these operations should only
be done on the main thread.
Relnote: "Navigation now depends on
[Lifecycle `2.3.1`](/jetpack/androidx/releases/lifecycle#2.3.1)
and now marks `setGraph()` and `navigate()`, the
methods that update the `NavBackStackEntry` `Lifecycle`,
as `@MainThread`, aligning Navigation with the main thread
enforcement introduced in Lifecycle `2.3.0`."
Test: existing and updated tests pass
BUG: 171125856
Change-Id: Ifcbe407d9cb186b50f05cbbd8bc5e11e19115a82
M navigation/navigation-dynamic-features-fragment/src/androidTest/java/androidx/navigation/dynamicfeatures/fragment/DynamicNavHostFragmentTest.kt
M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/build.gradle
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
sh...@google.com <sh...@google.com> #5
For the upcoming Navigation 2.4.0-alpha01 release, we've made it explicit that setGraph()
and navigate()
should only be called on the main thread (by adding @MainThread
annotations to the methods), thus aligning with the main thread enforcement of Lifecycle 2.3.
da...@gmail.com <da...@gmail.com> #6
Branch: androidx-main
commit 00c608a86c44b066198bd8c1dc7e2aef6088d4e1
Author: Ian Lake <ilake@google.com>
Date: Mon Apr 12 14:04:54 2021
Mark popBackStack and navigateUp as @MainThread
In addition to the navigate() and setGraph()
methods updated in
the methods for popping the back stack
should also be marked as @MainThread.
Relnote: "Navigation now depends on
[Lifecycle `2.3.1`](/jetpack/androidx/releases/lifecycle#2.3.1)
and now marks all of the methods that update the
`NavBackStackEntry` `Lifecycle`, including `setGraph()`,
`navigate()`, `popBackStack()`, and `navigateUp()`,
as `@MainThread`, aligning Navigation with the main thread
enforcement introduced in Lifecycle `2.3.0`."
Test: existing and updated tests pass
BUG: 171125856
Change-Id: Ib8bedbb3fe384b28c7441cbd57a0751eba307a2b
M navigation/navigation-runtime/api/current.txt
M navigation/navigation-runtime/api/public_plus_experimental_current.txt
M navigation/navigation-runtime/api/restricted_current.txt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
da...@gmail.com <da...@gmail.com> #7
We have a project member in India who could buy one of these phones and since they're so cheap (around $80 or something) I could cover that but in general I don't really want to be buying phones we don't really need.
sc...@google.com <sc...@google.com> #8
Thanks for the informations. You are right, it's possible that the devices are not genuinely passing the CTS.
Regardless, we will fix this in CameraX. (Thanks for letting us know BTW)
We will also try to get one of these device to test if there are other issues.
Scott
tr...@google.com <tr...@google.com> #9
This crash looks similar to the one in
sc...@google.com <sc...@google.com> #10
1) Fix it generally
2) Have a quirk for this, only enable the fix when device has this quirk.
The problem with the quirk is that it is too easy to miss some other devices that have this issue too.
Having a general fallback that works reasonably can fix the issue for all problematic devices.
(In these two cases, they are both reasonable to return false if the key access failed)
But I do think it has benefits to have a quirk so that we know exactly what workaround we handled in CameraX.
Maybe , we can have a type of quirk or workaround that we apply generally for all devices ?
WDYT?
mi...@google.com <mi...@google.com> #11
I had similar considerations while adding the try-catch code (aosp/2092403). I want to have a quirk to document the workaround we made, but it would be a little weird that the quirk should always return true on the device quirk check. I was wondering if it is possible to have another type of quirk, e.g. a function with comment in quirk format.
ap...@google.com <ap...@google.com> #12
Branch: androidx-main
commit 874ef5abba7929d0110fd54392ae9d8ee91003f0
Author: mingdatsai <mingdatsai@google.com>
Date: Mon May 09 20:32:42 2022
Catch CameraCharacteristicsBaseImpl exceptions
Some devices may throw AssertionError, which is not the expected
behavior, when failed to get CameraCharacteristic. When that happens,
we catch the error and return null to workaround it.
Bug: 231701345
Test: ZoomControlTest, ZoomControlDeviceTest &
Camera2CameraControlImplDeviceTest
Change-Id: Ia248ae5580b9d4a0949f4448ccbafcedd1ba7b9b
M camera/camera-camera2/src/test/java/androidx/camera/camera2/internal/ZoomControlTest.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/ZoomControl.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/Camera2CameraControlImplDeviceTest.java
M camera/camera-camera2/src/androidTest/java/androidx/camera/camera2/internal/ZoomControlDeviceTest.java
mi...@google.com <mi...@google.com> #13
We have tested both Android 11 and Android 12 versions on JioPhone Next and couldn't reproduce the bug. I'm guessing it might have be fixed in an earlier device update. The code to handle the AsserionError has been merged, and will be included in a following release.
da...@gmail.com <da...@gmail.com> #14
That makes sense since we're not seeing enough of the errors for it to be impacting all of them. Play Store doesn't provide more than the major version though and we don't have any of our own crash reporting.
mi...@google.com <mi...@google.com> #15
Thanks for the information, it seems match with the guess.
ju...@google.com <ju...@google.com> #16
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.camera:camera-camera2:1.2.0
Description
We're receiving these crash reports for our app on the Play Store (https://play.google.com/store/apps/details?id=app.grapheneos.camera.play ). I'm unsure why it's happening, but perhaps you can get one of these devices and reproduce it. Ideally, CameraX could at least start catching this exception and turning it into an error surfaced to the app.
We're mostly seeing these crashes from the Jio JioPhone Next which is a very widely used, very cheap device in India. The crash traceback is nearly identical across the around 5 device models where we've seen it with only minor variations in Camera2 implementation line numbers. All of the impacted devices are Android 11.