Clean up unsafe NewApi calls and prevent future unsafe calls [141326133]

Fixed

Internal Cleanup

Status Update

No update yet.

Description

al...@google.com

created issue #1

Sep 19, 2019 09:30PM

We have a number of places where we make a call to a new API wrapped with an SDK_INT check; however, this is only safe if the actual method reference is within a different class such as a static inner class.

Any class that makes such a call will fail ART optimization and degrade overall performance.

All unsafe calls should be migrated into API level-specific static inner classes, e.g. the way BiometricManager handles API 29 calls in Api29Impl here:

https://cs.corp.google.com/aosp-androidx/biometric/src/main/java/androidx/biometric/BiometricManager.java?l=76

We will also need a way to prevent developers from making these types of mistakes in the future, such as a Lint rule.

Comments

al...@google.com <al...@google.com> #2Sep 19, 2019 09:30PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

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

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

ob...@google.com <ob...@google.com> Sep 19, 2019 09:47PM

Accepted by ob...@google.com.

al...@google.com <al...@google.com> #3Sep 24, 2019 01:49PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

ob...@google.com <ob...@google.com> #4Oct 28, 2019 04:34PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

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

ap...@google.com <ap...@google.com> #5Aug 21, 2020 03:34PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d50d11e3a4e35caa033be78ed48bad10b5e660bf
Author: Oussama Ben Abdelbaki <obenabde@google.com>
Date: Tue Aug 18 11:47:44 2020

Add @DoNotInline annotation

The annotation annotates only methods and makes it such that the
method is never inlined during optimization but can still be
shrunk and obfuscated.

Relnote: Adding @DoNotInline annotation which instructs proguard to not inline the annotated method.
Test: None
Bug: 141326133
Change-Id: I3dfe8732c062e500b19d1cf2e3812389c7fc3cca

M annotation/annotation/api/current.txt
M annotation/annotation/api/public_plus_experimental_current.txt
M annotation/annotation/api/restricted_current.txt
A annotation/annotation/src/main/java/androidx/annotation/DoNotInline.java
M annotation/annotation/src/main/resources/META-INF/proguard/

androidx-annotations.pro

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

ap...@google.com <ap...@google.com> #6Sep 1, 2020 02:28PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit dd0931fe099fc9f344607bdcb04beae9949578f3
Author: Oussama Ben Abdelbaki <obenabde@google.com>
Date: Wed Aug 05 12:17:43 2020

Add new lint check to detect practices that could inhibit ART optimization

Bug contains context.

Bug: 141326133
Test: gw lintDebug
Change-Id: I1a4340e48ad11d8bc11e1f9c9be87ba7a6e9befa

A activity/activity/lint-baseline.xml
M appcompat/appcompat-resources/lint-baseline.xml
M appcompat/appcompat/lint-baseline.xml
M arch/core-runtime/lint-baseline.xml
A autofill/autofill/lint-baseline.xml
M benchmark/common/lint-baseline.xml
M biometric/biometric/lint-baseline.xml
A browser/browser/lint-baseline.xml
M buildSrc/src/main/kotlin/androidx/build/LintConfiguration.kt
M buildSrc/src/main/kotlin/androidx/build/dependencies/Dependencies.kt
A camera/camera-camera2-pipe/lint-baseline.xml
M camera/camera-camera2/lint-baseline.xml
M camera/camera-core/lint-baseline.xml
M camera/camera-testing/lint-baseline.xml
A camera/camera-view/lint-baseline.xml
M camera/integration-tests/extensionstestlib/lint-baseline.xml
D compose/runtime/runtime/lint-baseline.xml
M compose/test-utils/lint-baseline.xml
A compose/ui/ui-graphics/lint-baseline.xml
A compose/ui/ui-text-android/lint-baseline.xml
A compose/ui/ui-text/lint-baseline.xml
A compose/ui/ui/lint-baseline.xml
M contentpager/contentpager/lint-baseline.xml
M core/core-animation/lint-baseline.xml
A core/core-ktx/lint-baseline.xml
M core/core/lint-baseline.xml
A datastore/datastore-core/lint-baseline.xml
A documentfile/documentfile/lint-baseline.xml
M drawerlayout/drawerlayout/lint-baseline.xml
M dynamic-animation/dynamic-animation/lint-baseline.xml
M emoji/core/lint-baseline.xml
M exifinterface/exifinterface/lint-baseline.xml
A fragment/fragment/lint-baseline.xml
M leanback/leanback/lint-baseline.xml
A lifecycle/lifecycle-livedata-ktx/lint-baseline.xml
A lifecycle/lifecycle-process/lint-baseline.xml
M lint-checks/build.gradle
M lint-checks/src/main/java/androidx/build/lint/AndroidXIssueRegistry.kt
A lint-checks/src/main/java/androidx/build/lint/UnsafeNewApiCallsDetector.kt
M media/media/lint-baseline.xml
M media2/common/lint-baseline.xml
M media2/player/lint-baseline.xml
M media2/session/lint-baseline.xml
M media2/session/version-compat-tests/common/lint-baseline.xml
M media2/widget/lint-baseline.xml
M mediarouter/mediarouter/lint-baseline.xml
M preference/preference/lint-baseline.xml
A print/print/lint-baseline.xml
M room/guava/lint-baseline.xml
A room/ktx/lint-baseline.xml
M room/runtime/lint-baseline.xml
M security/crypto/lint-baseline.xml
M slices/builders/lint-baseline.xml
M slices/core/lint-baseline.xml
M slices/view/lint-baseline.xml
M sqlite/sqlite-framework/lint-baseline.xml
M sqlite/sqlite/lint-baseline.xml
M testutils/testutils-runtime/lint-baseline.xml
M textclassifier/textclassifier/lint-baseline.xml
A tracing/tracing/lint-baseline.xml
M transition/transition/lint-baseline.xml
M tv-provider/tv-provider/lint-baseline.xml
A ui/ui-graphics/lint-baseline.xml
A ui/ui-test/lint-baseline.xml
A ui/ui-text-android/lint-baseline.xml
A ui/ui-tooling/lint-baseline.xml
M vectordrawable/vectordrawable-animated/lint-baseline.xml
M versionedparcelable/versionedparcelable/lint-baseline.xml
M viewpager2/viewpager2/lint-baseline.xml
A window/window/lint-baseline.xml
M work/workmanager-testing/lint-baseline.xml
M work/workmanager/lint-baseline.xml

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

ap...@google.com <ap...@google.com> #7Sep 1, 2020 08:31PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 69d3a1a7dc9a5c574728039180a1ca97a716c658
Author: Jeff Gaston <jeffrygaston@google.com>
Date: Tue Sep 01 12:23:14 2020

Spelling buf -> but

Bug: 141326133
Change-Id: Ie25a0db7320cd135aa6108a0a73f8dfbd7590779

M lint-checks/src/main/java/androidx/build/lint/UnsafeNewApiCallsDetector.kt

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

al...@google.com <al...@google.com> #8Oct 20, 2020 02:10PM

Relatedly, b/171298014 filed against API guidelines. We'll want to recommend @DoNotInline.

ob...@google.com <ob...@google.com> #9Oct 27, 2020 11:43AM

Alan, I think we said we could have some sort of event where library owners do this? Or we send an email urging owners to do this?

al...@google.com <al...@google.com> #10Oct 27, 2020 01:53PM

We've got a couple of other baseline clean-up tasks pending. At some point we'll run a Fix It by dumping all the baselines into a spreadsheet and automating the bug filing.

For now, we can send an email explaining (1) that we baselined violations across almost every library, (2) violations are known to lead to crashes on certain device versions, and (3) how to see where you violations are (ex. run the linter from Studio).

I can handle that if you've got other P1's to close out, just assign to me.

ob...@google.com <ob...@google.com> #11Nov 4, 2020 01:35PM

Assigned to al...@google.com.

Alan, since you probably know how to explain this better than I do, I might as well assign it to you now.

al...@google.com <al...@google.com> Apr 29, 2021 06:42PM

Accepted by al...@google.com.

al...@google.com <al...@google.com> #12May 20, 2021 03:07PM

Marked as fixed.

Bugs filed and discussion kicked off on how to best address them.

na...@google.com <na...@google.com> #13Sep 21, 2022 05:54PM

This bug was linked in a change in the following release(s):

androidx.drawerlayout:drawerlayout:1.2.0-alpha01