From 35e9b9d52ae7ef1a46efcbc5987844c8f0975f3b Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 17 Nov 2022 09:27:52 -0800 Subject: [PATCH] Fixes #4712: Ensure CoordinatorLayout Behaviors are not renamed/removed during binary optimizations (#4731) ## Explanation Fixes #4712 Proguard was renaming CoordinatorLayout behaviors and removing implementation constructors, where both can be referenced via reflection in CoordinatorLayout. We haven't run into this issue before since it seems the new bottom navigation component introduced in #4482 is the first time we actually started using CoordinatorLayout behaviors. Note that this is inherently difficult to test as it requires verifying runtime functionality in a Proguard-optimized build. Hence, it's only testable with end-to-end tests (and our own end-to-end tests currently are manual tests) so no new unit/integration tests are being added. Note also that this PR will be cherry-picked for the upcoming beta MR2 release. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only This doesn't seem particularly valuable to fill out here--the video would just show the options menu opening as it should (and does for developer builds). I've verified it works locally, and #4712 will require reverifying once the PR is merged, --- config/proguard/androidx-proguard-rules.pro | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/config/proguard/androidx-proguard-rules.pro b/config/proguard/androidx-proguard-rules.pro index 1eaab2b2c54..68584436145 100644 --- a/config/proguard/androidx-proguard-rules.pro +++ b/config/proguard/androidx-proguard-rules.pro @@ -36,6 +36,14 @@ -keep class * extends androidx.room.RoomDatabase -keep @androidx.room.Entity class * +# CoordinatorLayout uses reflection to load Behavior subclasses (see #4712 for context), so they +# can't be renamed. +-keep class * extends androidx.coordinatorlayout.widget.CoordinatorLayout$Behavior +-keepclassmembers class * extends androidx.coordinatorlayout.widget.CoordinatorLayout$Behavior { + # Constructors may be referenced via reflection, so make sure they don't get removed. + (...); +} + # A strange unknown issue that arises within a Room class (it seems an actual dependency is missing # within Room). -dontwarn androidx.room.paging.LimitOffsetDataSource