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

The issue is that Jetifier modifies jars and does not remove signatures. Below is the original class:

Classfile /tmp/non-jetified/org/bouncycastle/jce/provider/ExtCRLException.class
Last modified Mar 1, 2015; size 362 bytes
MD5 checksum 3968a716f9e60e1ad36c34157bd0239f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
minor version: 0
major version: 49
flags: ACC_SUPER
Constant pool:
#1 = Utf8 <init>
#2 = Utf8 cause
#3 = Utf8 getCause
#4 = Utf8 java/security/cert/CRLException
#5 = Utf8 org/bouncycastle/jce/provider/ExtCRLException
#6 = Class #4 // java/security/cert/CRLException
#7 = Class #5 // org/bouncycastle/jce/provider/ExtCRLException
#8 = Utf8 Ljava/lang/Throwable;
#9 = Utf8 (Ljava/lang/String;)V
#10 = Utf8 ()Ljava/lang/Throwable;
#11 = Utf8 (Ljava/lang/String;Ljava/lang/Throwable;)V
#12 = NameAndType #2:#8 // cause:Ljava/lang/Throwable;
#13 = NameAndType #1:#9 // "<init>":(Ljava/lang/String;)V
#14 = Fieldref #7.#12 // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
#15 = Methodref #6.#13 // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
#16 = Utf8 Code
{
java.lang.Throwable cause;
descriptor: Ljava/lang/Throwable;
flags:

org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
flags:
Code:
stack=2, locals=3, args_size=3
0: aload_0
1: aload_1
2: invokespecial #15 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
5: aload_0
6: aload_2
7: putfield #14 // Field cause:Ljava/lang/Throwable;
10: return

public java.lang.Throwable getCause();
descriptor: ()Ljava/lang/Throwable;
flags: ACC_PUBLIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: getfield #14 // Field cause:Ljava/lang/Throwable;
4: areturn
}

One processed with Jetifier:

Classfile /tmp/jetified/org/bouncycastle/jce/provider/ExtCRLException.class
Last modified Oct 14, 2019; size 362 bytes
MD5 checksum 76bc557706ee3480e53d88b21316913f
class org.bouncycastle.jce.provider.ExtCRLException extends java.security.cert.CRLException
minor version: 0
major version: 49
flags: ACC_SUPER
Constant pool:
#1 = Utf8 org/bouncycastle/jce/provider/ExtCRLException
#2 = Class #1 // org/bouncycastle/jce/provider/ExtCRLException
#3 = Utf8 java/security/cert/CRLException
#4 = Class #3 // java/security/cert/CRLException
#5 = Utf8 cause
#6 = Utf8 Ljava/lang/Throwable;
#7 = Utf8 <init>
#8 = Utf8 (Ljava/lang/String;Ljava/lang/Throwable;)V
#9 = Utf8 (Ljava/lang/String;)V
#10 = NameAndType #7:#9 // "<init>":(Ljava/lang/String;)V
#11 = Methodref #4.#10 // java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
#12 = NameAndType #5:#6 // cause:Ljava/lang/Throwable;
#13 = Fieldref #2.#12 // org/bouncycastle/jce/provider/ExtCRLException.cause:Ljava/lang/Throwable;
#14 = Utf8 getCause
#15 = Utf8 ()Ljava/lang/Throwable;
#16 = Utf8 Code
{
java.lang.Throwable cause;
descriptor: Ljava/lang/Throwable;
flags:

org.bouncycastle.jce.provider.ExtCRLException(java.lang.String, java.lang.Throwable);
descriptor: (Ljava/lang/String;Ljava/lang/Throwable;)V
flags:
Code:
stack=2, locals=3, args_size=3
0: aload_0
1: aload_1
2: invokespecial #11 // Method java/security/cert/CRLException."<init>":(Ljava/lang/String;)V
5: aload_0
6: aload_2
7: putfield #13 // Field cause:Ljava/lang/Throwable;
10: return

public java.lang.Throwable getCause();
descriptor: ()Ljava/lang/Throwable;
flags: ACC_PUBLIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: getfield #13 // Field cause:Ljava/lang/Throwable;
4: areturn
}

Changes in the constants pool cause changes in the code (indices change), which causes different MD5 checksums. This means that signatures will not match any more.

This started happening in 3.6.0-beta01 because we are using Jetifier with copyUnmodifiedLibsAlso=true flag (previously it was false).

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

@Chris You can work around this by updating gradle.properties with:

android.jetifier.blacklist=.*bcprov.*

This will avoid Jetifying the bouncycastle library.

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

Thanks Ivan!

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

Pending fix in:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1151460

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

Marked as fixed.

did this fix make it in for 3.6?

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

Accepted by al...@google.com.

Jetifier 1.0.0-beta08 was released and has a fix for this.

To avoid waiting for new Android Studio you can workaround this in the meantime by forcing usage of a new version of jetifier in gradle:
buildscript {
dependencies {
classpath 'com.android.tools.build:gradle:x.y.z'
classpath 'com.android.tools.build.jetifier:jetifier-processor:1.0.0-beta08' // <- Add this
}
}

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

Jetifier 1.0.0-beta08, which has the fix for this issue, will be integrated into AGP 3.6.0-rc01 and 4.0.0-alpha06:

https://android.googlesource.com/platform/tools/base/+/ef5e56702b626b6be642ecce7d0078b7b1a81bcb

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