AppCompat doesn't pass call to my Resources object on low end devices (OS 4.x) [176129022]

Fixed

Bug

Status Update

No update yet.

Description

as...@gmail.com

created issue #1

Dec 22, 2020 07:29PM

Component used: appcompat-resources (androidx.appcompat) Version used: 1.1.0 Devices/Android versions reproduced on: OS 4.x (Jelly bean, Kitkat)

Issue Description I am trying to fetch strings via my own Resources object for non-English locales. In the attached project, my Hindi string is stored in assets folder. When I run the app on OS 5 and above, the string is correctly displayed (fetched from assets), whereas, when I run the app on OS 4.3 or (OS < 5.0) it displays English string instead of the correct locale. After debugging I found that when I have AppCompatDelegate.setCompatVectorFromResourcesEnabled(true), on low end devices, it wraps Resources object (my object) into VectorEnabledTintResources object. In this class only getDrawable is overridden but the rest of the calls are passed over to the parent Resources object instead of baseResources causing the issue.

Suggestion Extend VectorEnabledTintResources from ResourcesWrapper or delegate the rest of the calls to the passed Resources object

Attachments

HelloWorld.zip ; Sample project showing the issue
Working.mov : Screen record of OS 4.3 emulator with AppCompatDelegate.setCompatVectorFromResourcesEnabled(false);
Not Working.mov : Screen record of OS 4.3 emulator with AppCompatDelegate.setCompatVectorFromResourcesEnabled(true);

Not Working.mov

10 MB

Download

Working.mov

10 MB

Download

HelloWorld.zip

9 MB

Download

Comments

al...@google.com <al...@google.com> #2Jan 5, 2021 07:42PM

Hi Ed, Thank you so much for these suggestions. I've been reviewing them and merging them in. Hopefully it should be live. I've included a thank you note too in the article.

ch...@google.com <ch...@google.com> #3Jan 6, 2021 08:27AM

Great! Thanks a lot, I'll look for the live updates soon!

cs...@gmail.com <cs...@gmail.com> #4Jan 6, 2021 09:47PM

get this off my phone pal

al...@google.com <al...@google.com> Jan 7, 2021 08:12PM

Accepted by al...@google.com.

al...@google.com <al...@google.com> #5Jan 11, 2021 07:55PM

API change is held up while AppCompat goes through beta cycle. Should be short, though.

ap...@google.com <ap...@google.com> #6Mar 16, 2021 12:44AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 3ea8ff0e5b7e467fc6fd77b92e07c16692a2f5c9
Author: Alan Viverette <alanv@google.com>
Date: Thu Jan 07 11:21:56 2021

Wrap resources with VectorEnabledTintResources

Fixes: 176129022
Test: VectorEnabledTintResourcesTest
Change-Id: I4fe803df539a1e10a8d2428e02417180bc2dca79

M appcompat/appcompat-resources/api/restricted_current.txt
A appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/TestResources.java
M appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/TintResourcesTest.java
A appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/VectorEnabledTintResourcesTest.java
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/VectorEnabledTintResources.java

https://android-review.googlesource.com/1542390

al...@google.com <al...@google.com> #7Mar 19, 2021 09:33PM

Accepted by al...@google.com.

Fix seems to have regressed LayoutInflater.from(context) inflation behavior, reverting while I investigate what's going on.

al...@google.com <al...@google.com> #8Mar 19, 2021 09:35PM

Note this landed in appcompat release candidate, so whatever the fix is will need to be cherry-picked.

al...@google.com <al...@google.com> #9Mar 19, 2021 10:28PM

03-19 09:45:06.958 24627 24627 E AndroidRuntime: Caused by: android.content.res.Resources$NotFoundException: File res/jnq.xml from drawable resource ID #0x7f0804ea
03-19 09:45:06.958 24627 24627 E AndroidRuntime:        at android.content.res.Resources.loadDrawable(Resources.java:3453)
03-19 09:45:06.958 24627 24627 E AndroidRuntime:        at android.content.res.Resources.getDrawable(Resources.java:1893)
03-19 09:45:06.958 24627 24627 E AndroidRuntime:        at android.support.v7.widget.ResourcesWrapper.getDrawable(ResourcesWrapper.java:131)
03-19 09:45:06.958 24627 24627 E AndroidRuntime:        at android.support.v7.widget.VectorEnabledTintResources.superGetDrawable(VectorEnabledTintResources.java)
03-19 09:45:06.958 24627 24627 E AndroidRuntime:        at android.support.v7.widget.ResourceManagerInternal.onDrawableLoadedFromResources(ResourceManagerInternal.java:356)

This happens when the weak reference to the Context goes away, which should never happen. The resources are obtained either from a TintContextWrapper or AppCompatActivity, both of which retain a reference to the resources object.

The only way this would happen is if the resources were obtained and cached somewhere, then the wrapper was discarded and the cached instance used.

al...@google.com <al...@google.com> #10Mar 19, 2021 10:40PM

Strike that, this stack trace indicates that the context was present and loadDrawableFromDelegates returned null. There are quite a lot of reasons this could fail, including:

inflation of this resource ID ever failed
the resource is not an XML file
the delegate's createFromXmlInner call returned null
no delegate exists for the tag

al...@google.com <al...@google.com> #11Mar 24, 2021 07:23PM

The root cause was re-entrance to Resources.getDrawable(). Since it's wrapped, the first call runs through the AppCompat override but subsequent inner calls go to the wrapped class. As such, a <vector> nested inside anything else would fail to load.

ap...@google.com <ap...@google.com> #12Apr 29, 2021 06:18PM

Project: platform/frameworks/support
Branch: androidx-main

commit 22ba40be38aeea0e38d206a94d9ae2e2fb637f25
Author: Alan Viverette <alanv@google.com>
Date: Wed Mar 24 19:26:53 2021

Re-apply "Wrap resources with VectorEnabledTintResources" with fixes

Also fixes TintResources, since it will hit the same issue with
re-entrance. Uses ResourcesCompat to avoid UnsafeNewApi in wrapper.

Adds tests for correctness of resource wrapping classes, which
surprisingly did not exist yet.

This reverts commit 7450da45110f2ddb6de946cac4a3d58ab8a4be58.

RelNote: """Added support for nested tinted resources and vector drawables
in AppCompat resource loading backports. Note, however, that this means
apps cannot override getDrawable() on a custom Resources object when
backports are enabled."""
Bug: 176129022
Test: VectorEnabledTintResourcesTest, TintResourcesTest
Reason for revert: Fixed issue with nested drawables

Change-Id: Ia6b034d5143311a111c31d59afbb37dbeeea2131

M appcompat/appcompat-resources/api/restricted_current.txt
M appcompat/appcompat-resources/src/androidTest/AndroidManifest.xml
A appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/TestResources.java
M appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/TintResourcesTest.java
A appcompat/appcompat-resources/src/androidTest/java/androidx/appcompat/widget/VectorEnabledTintResourcesTest.java
A appcompat/appcompat-resources/src/androidTest/res/drawable/tint_nested.xml
A appcompat/appcompat-resources/src/androidTest/res/drawable/vector_nested.xml
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/ResourceManagerInternal.java
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/ResourcesWrapper.java
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/TintResources.java
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/VectorEnabledTintResources.java
M appcompat/appcompat/src/androidTest/AndroidManifest.xml
D appcompat/appcompat/src/androidTest/java/androidx/appcompat/widget/TintResourcesTest.java

https://android-review.googlesource.com/1652968

al...@google.com <al...@google.com> #13Apr 29, 2021 06:36PM

Marked as fixed.

So hopefully that doesn't break the world again, but now we can support nested tinting + vectors and custom Resources objects (but not getDrawable() overrides).

Issue 176129022

Description

Issue summary

Comments

al...@google.com <al...@google.com> #2Jan 5, 2021 07:42PM

ch...@google.com <ch...@google.com> #3Jan 6, 2021 08:27AM

cs...@gmail.com <cs...@gmail.com> #4Jan 6, 2021 09:47PM

al...@google.com <al...@google.com> Jan 7, 2021 08:12PM

al...@google.com <al...@google.com> #5Jan 11, 2021 07:55PM

ap...@google.com <ap...@google.com> #6Mar 16, 2021 12:44AM

al...@google.com <al...@google.com> #7Mar 19, 2021 09:33PM

al...@google.com <al...@google.com> #8Mar 19, 2021 09:35PM

al...@google.com <al...@google.com> #9Mar 19, 2021 10:28PM

al...@google.com <al...@google.com> #10Mar 19, 2021 10:40PM

al...@google.com <al...@google.com> #11Mar 24, 2021 07:23PM

ap...@google.com <ap...@google.com> #12Apr 29, 2021 06:18PM

al...@google.com <al...@google.com> #13Apr 29, 2021 06:36PM

Add comment

Issue metadata