Status Update
Comments
il...@google.com <il...@google.com> #2
Branch: androidx-master-dev
commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020
Semantics no-op cleanups
Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:
- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".
- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)
- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)
Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."
Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc
M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt
sg...@google.com <sg...@google.com> #3
I have not tried to create a project, but the way -if
rules are evaluated is that if the condition public class ** extends androidx.fragment.app.Fragment
holds (that is at least one class extends androidx.fragment.app.Fragment
then the subsequent -keep
rule is activated, so if at least one class extends androidx.fragment.app.Fragment
this keep rule is activated:
-keep public class ** extends androidx.fragment.app.Fragment {
public <init>();
}
Causing all classes in the app extending androidx.fragment.app.Fragment
to be kept. The -if
rule captures <n>
are used to parameterize the subsequent keep rule with what was matched by the condition. Each wildcard in the condition will be given a capture number (starting from 1) and they can then be used in the subsequent rule. So if the keep rule is:
-keep public class <1> {
public <init>();
}
Then if e.g. class com.example.app.MyFragment
extends androidx.fragment.app.Fragment
, then the following rule will be activated:
-keep public class com.example.app.MyFragment {
public <init>();
}
Which I assume is the expected behaviour.
It might be for most apps all classes which extend androidx.fragment.app.Fragment
ends up being part of the program, so the end result ends up being the same, but I still think that the rule should be precise and only keep what the condition matches.
ap...@google.com <ap...@google.com> #4
Branch: androidx-master-dev
commit faa9401ccc7c645f56f12b9928c916a04dbb4c28
Author: Ian Lake <ilake@google.com>
Date: Tue Feb 25 09:25:48 2020
Ensure conditional keep only keeps what is necessary
Use the <1> placeholder to only apply the -keep
rule to the exact classes that the -if applies to
rather than having the keeping of one Fragment class
cause all Fragments to be kept.
Test: manual testing in project
Fixes: 149665169
Change-Id: I39ffb80e69adfa8fab3a3ed732822a4d937f403b
M fragment/fragment/proguard-rules.pro
il...@google.com <il...@google.com> #5
We've fixed the ProGuard rules and it'll be available in the upcoming Fragment 1.3.0-alpha01 (and if we do a Fragment 1.2.3, it'll also be a part of that).
an...@google.com <an...@google.com> #6
tb...@gmail.com <tb...@gmail.com> #7
Maybe this is intentional, but I updated my project to androidx fragments 1.2.3 (from 1.2.2), and now necessary fragments ("MyFragment" below) that used to be kept are now being stripped by R8. The fragment in question is referenced in a layout that is set using setContentView() on an activity.
<androidx.fragment.app.FragmentContainerView
android:id="@+id/fragment_id"
android:name="com.mycompany.MyFragment"
android:layout_width="match_parent"
android:layout_height="match_parent" />
AGP 3.6.1. Not sure how I'm supposed to keep it now. It has normal default constructor and has androidx.fragment.Fragment in the superclass hierarchy.
il...@google.com <il...@google.com> #8
Re #7 - that's unrelated and expected when using AGP 3.6.1 as per the
The only reason that it "worked" in Fragment 1.2.2 was because all Fragments were being kept if even one Fragment was being kept (which is what this bug fixed).
dy...@gmail.com <dy...@gmail.com> #9
What's the reason for having Proguard rules included with the Fragment library in the first place?
I'm asking because right now, this rule is actually preventing me from obfuscating, shrinking and optimizing my fragments for reasons I don't understand. One reason I could think of is reflection. Instead, is it possible to completely remove them and suggest developers to add these rules themselves?
il...@google.com <il...@google.com> #10
Re #9 - please star
Description
Component used: Fragment Version used: Master as of Feb 17 2020
Just noticed the Proguard rule which was added in ag/1216798 (and updated in ag/1227696), which looks like this:
I think the
**
in the conditional-keep
should have been<1>
to refer back to the class implementingandroidx.fragment.app.Fragment
(and then theextends
part does not matter then). Otherwise if one class extendsandroidx.fragment.app.Fragment
then the default constructor for all classes extendingandroidx.fragment.app.Fragment
will be kept.Like this: