Remove @DoNotInline usage, disable lint check [345472586]

Fixed

Feature Request

Status Update

No update yet.

Description

al...@google.com

created issue #1

Jun 6, 2024 04:37PM

R8 may double-outline platform NewApi calls when automatic outlining is enabled, so we should consider no-op'ing the @DoNotInline annotation when we're using an R8 version that does outlining.

Looping in from email,

The design for targeted rules is in go/targeted-shrinking-rules.

You can actually target R8 rules based on R8 version, so that's probably even better. I don't know where it's documented, but you can do: META-INF/com.android.tools/r8-upto-$VERSION/rules.pro

Comments

al...@google.com <al...@google.com> #2Jun 6, 2024 07:32PM

Attached reproducing video of an app using calculateWindowSizeClass and LocalConfiguration:

On the second time resizing to be 50%/50% from smaller, the displayed configuration is incorrect.

sg...@google.com <sg...@google.com> #3Jun 7, 2024 06:54AM

When resizing the activity in multi-window mode under those conditions, there are cases where:

the Activity is not recreated Activity.onConfigurationChanged is not called View.onConfigurationChanged is not called

Is this just for 33 / is there a framework bug filed? The amount of broken things in configuration land we already had to workaround in Compose is a bit untenable. It seems like we are missing a lot of test coverage in the platform to avoid these issues hitting us in the future

al...@google.com <al...@google.com> #4Jun 10, 2024 02:51PM

In any case this seems like more ammunition for us to push more strongly on 'pure-compose apps should always opt out of configuration changes'

ag...@google.com <ag...@google.com> #5Jun 10, 2024 03:08PM

Yes, API 33 introduced this behavior, (internal) platform bug is here: b/247013643

al...@google.com <al...@google.com> #6Jun 10, 2024 04:07PM

I was incorrect, this is also reproducible on API 32 as well. Not reproducible on API 31.

al...@google.com <al...@google.com> Jun 10, 2024 08:57PM

Accepted by al...@google.com.

al...@google.com <al...@google.com> #7Jun 11, 2024 04:30PM

Status: New

Adam/Louis, how do we want to handle this one? Is there someone on platform who should take this, or since API 32 is already out in the wild maybe this should go to devrel to push more strongly on 'pure-compose apps should always opt out of configuration changes'.

al...@google.com <al...@google.com> Jul 2, 2024 02:50PM

Assigned to al...@google.com.

al...@google.com <al...@google.com> Jul 2, 2024 05:35PM

Accepted by al...@google.com.

ap...@google.com <ap...@google.com> #8Jul 8, 2024 05:41PM

deleted

ap...@google.com <ap...@google.com> #9Jul 8, 2024 07:31PM

I'm having issues on pre-33 api levels on a pure compose app (has all configChanges added in the manifest file) pared with AppCompatActivity just for multi language support. When changing the language I'm using AppCompatDelegate.setApplicationLocales(compat) which calls onConfigurationChanged on the activity. LocalConfiguration changes to the new configuration on 33+ but remains the same on pre-33. Workaround that i've found is calling if (Build.VERSION.SDK_INT < 33) contentView.dispatchConfigurationChanged(newConfig) in onConfigurationChanged. I'm guessing it's a compose issue since activity wise everything works as expected.

ap...@google.com <ap...@google.com> #10Jul 9, 2024 03:56PM

#comment9 sounds like b/321896385, which is also related to LocalConfiguration but a separate issue.

al...@google.com <al...@google.com> #11Jul 9, 2024 04:37PM

Thanks for the heads up, I've tried to find a related issue but only managed to find this one after a brief search.

ap...@google.com <ap...@google.com> #12Jul 9, 2024 05:08PM

ap...@google.com <ap...@google.com> #13Jul 9, 2024 05:38PM

Thanks for the heads up, I've tried to find a related issue but only managed to find this one after a brief search.

ap...@google.com <ap...@google.com> #14Jul 10, 2024 04:05PM

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

commit 12ea61e42c7f29a0f539a069f06507a0f133910d
Author: Alan Viverette <alanv@google.com>
Date: Tue Jul 02 13:55:26 2024

Remove usage of @DoNotInline for manual NewApi outlines (A thru Ca)

Does not include AppSearch because they have downstream changes.

We no longer need manual NewApi outlines, so we can start by letting the
compiler inline the static methods.

Relnote: """Removed manual outlining of access to new platform APIs
since this happens automatically via API modeling when using R8 with
AGP 7.3 or later (e.g. R8 version 3.3) and for all builds when using
AGP 8.1 or later (e.g. D8 version 8.1). Clients who are not using
AGP are advised to update to D8 version 8.1 or later. See
[this article](

https://medium.com/androiddevelopers/mitigating-soft-verification-issues-in-r8-and-d8-7e9e06827dfd)
for more details."""
Bug: 345472586
Test: ./gradlew bOS
Change-Id: I9496cfaeb50a5c484fbee6521b74a0605fb013dc

M activity/activity/src/main/java/androidx/activity/BackEventCompat.kt
M activity/activity/src/main/java/androidx/activity/ComponentActivity.kt
M activity/activity/src/main/java/androidx/activity/OnBackPressedDispatcher.kt
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/resources/Compatibility.java
M appcompat/appcompat-resources/src/main/java/androidx/appcompat/widget/DrawableUtils.java
M appcompat/appcompat/src/main/java/androidx/appcompat/app/AppCompatDelegate.java
M appcompat/appcompat/src/main/java/androidx/appcompat/app/AppCompatDelegateImpl.java
M appcompat/appcompat/src/main/java/androidx/appcompat/app/AppLocalesMetadataHolderService.java
M appcompat/appcompat/src/main/java/androidx/appcompat/view/WindowCallbackWrapper.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatReceiveContentHelper.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatSpinner.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatTextClassifierHelper.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatTextHelper.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/AppCompatTextViewAutoSizeHelper.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/DropDownListView.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/ListPopupWindow.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/MenuPopupWindow.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/SearchView.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/TintTypedArray.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/Toolbar.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/TooltipCompat.java
M appcompat/appcompat/src/main/java/androidx/appcompat/widget/ViewUtils.java
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/perfetto/server/PerfettoHttpServer.kt
M benchmark/integration-tests/macrobenchmark-target/src/main/java/androidx/benchmark/integration/macrobenchmark/target/SingleColorActivity.kt
M biometric/integration-tests/testapp/src/main/java/androidx/biometric/integration/testapp/CryptoUtils.kt
M bluetooth/bluetooth/src/main/java/androidx/bluetooth/AdvertiseParams.kt
M bluetooth/bluetooth/src/main/java/androidx/bluetooth/ScanFilter.kt
M bluetooth/bluetooth/src/main/java/androidx/bluetooth/ScanResult.kt
M browser/browser/src/main/java/androidx/browser/customtabs/Api33Impl.java
M browser/browser/src/main/java/androidx/browser/customtabs/CustomTabsIntent.java
M camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/adapter/EncoderProfilesProviderAdapter.kt
M camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/compat/ApiCompat.kt
M camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/impl/CameraInteropStateCallbackRepository.kt
M camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/internal/DynamicRangeConversions.kt
M camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/internal/DynamicRangeResolver.kt
M camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/ApiCompat.kt
M camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/CaptureSessionWrapper.kt
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CaptureRequestBuilder.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2EncoderProfilesProvider.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/DynamicRangeResolver.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/SupportedSurfaceCombination.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/SynchronizedCaptureSessionBaseImpl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/compat/ApiCompat.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/compat/StreamConfigurationMapCompatBaseImpl.java
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/compat/params/DynamicRangeConversions.java
M camera/camera-core/src/main/java/androidx/camera/core/imagecapture/Bitmap2JpegBytes.java
M camera/camera-core/src/main/java/androidx/camera/core/impl/CameraValidator.java
M camera/camera-core/src/main/java/androidx/camera/core/impl/utils/ContextUtil.java
M camera/camera-effects/src/main/java/androidx/camera/effects/internal/Utils.java
M camera/camera-extensions/src/androidTest/java/androidx/camera/extensions/util/Api21Impl.kt
M camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/ExtensionsUtils.java
M camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/workaround/CaptureOutputSurfaceForCaptureProcessor.java
M camera/camera-testing/src/main/java/androidx/camera/testing/impl/CameraUtil.java
M camera/camera-testing/src/main/java/androidx/camera/testing/impl/TestImageUtil.java
M camera/camera-testing/src/main/java/androidx/camera/testing/impl/WakelockEmptyActivityRule.kt
M camera/camera-testing/src/main/java/androidx/camera/testing/impl/compat/LooperCompat.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api23Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api24Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api26Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api28Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api29Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/compat/Api31Impl.java
M camera/camera-video/src/main/java/androidx/camera/video/internal/encoder/EncoderImpl.java
M camera/camera-view/src/main/java/androidx/camera/view/SurfaceViewImplementation.java
M camera/camera-viewfinder/src/main/java/androidx/camera/viewfinder/SurfaceViewImplementation.java
M camera/integration-tests/avsynctestapp/src/main/java/androidx/camera/integration/avsync/MainActivity.kt
M camera/integration-tests/camerapipetestapp/src/main/java/androidx/camera/integration/camera2/pipe/SimpleCamera.kt
M camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXActivity.java
M camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/CameraXService.java
M camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/OpenGLActivity.java
M camera/integration-tests/coretestapp/src/main/java/androidx/camera/integration/core/SurfaceViewRenderSurface.java
M camera/integration-tests/extensionstestapp/src/main/java/androidx/camera/integration/extensions/Camera2ExtensionsActivity.kt
M car/app/app-automotive/src/main/java/androidx/car/app/activity/BaseCarAppActivity.java
M car/app/app-automotive/src/main/java/androidx/car/app/activity/renderer/surface/TemplateSurfaceView.java
M car/app/app-automotive/src/main/java/androidx/car/app/hardware/info/AutomotiveCarInfo.java
M car/app/app/src/main/java/androidx/car/app/CarAppMetadataHolderService.java
M car/app/app/src/main/java/androidx/car/app/CarContext.java
M car/app/app/src/main/java/androidx/car/app/SessionInfoIntentEncoder.java
M car/app/app/src/main/java/androidx/car/app/connection/CarConnectionTypeLiveData.java
M car/app/app/src/main/java/androidx/car/app/model/DateTimeWithZone.java
M car/app/app/src/main/java/androidx/car/app/model/DurationSpan.java
M car/app/app/src/main/java/androidx/car/app/navigation/model/TravelEstimate.java
M car/app/app/src/main/java/androidx/car/app/notification/CarNotificationManager.java
M car/app/app/src/main/java/androidx/car/app/validation/HostValidator.java

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

al...@google.com <al...@google.com> #15Jul 10, 2024 05:48PM

Assigned to bu...@google.com.

We still have some @DoNotInline usages lingering because of conflicts with androidx-platform-dev. Once the upstreaming is completed and those are removed, I think we should call it done. Manually inlining the ApiXXImpl classes isn't urgent or really necessary.

bu...@google.com <bu...@google.com> #16Jul 10, 2024 05:53PM

Accepted by bu...@google.com.

Hi. I've received your bug, you didn't specify what to wait for so I will wait for

b/346290560

to be resolved and then assign the bug to alanv@google.com.

Bugjuggler:

b/346290560

-> assigned to alanv@google.com

ap...@google.com <ap...@google.com> #17Jul 18, 2024 03:34PM

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

commit c520a87c6e27a14472cfcf8b03c0d4ea8c1a37c7
Author: Ian Baker <ibaker@google.com>
Date: Thu Jul 18 08:32:31 2024

Remove `@DoNotInline` recommendations from go/androidx-api-guidelines

The associated lint check has been disabled, see

b/345472586

for more details.

PiperOrigin-RevId: 653629895
Change-Id: I186c052a059e5ad30386cd45beb47f24bdafe657

M docs/api_guidelines/compat.md
M docs/api_guidelines/platform_compat.md

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

bu...@google.com <bu...@google.com> Jul 23, 2024 02:34PM

Assigned to al...@google.com.

al...@google.com <al...@google.com> #18Jul 29, 2024 02:25PM

Marked as fixed.

We'll take removing manual outlining on a case-by-case basis. Removing it all at once would be a lot of work for not much gain.