Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2251213003:
Don't let proguard rename the CREATOR static of any Parcelable (Closed)
|
Created:
4 years, 4 months ago by Tobias Sargeant Modified:
4 years, 4 months ago CC:
chromium-reviews, android-webview-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't let proguard rename the CREATOR static of any Parcelable
Currently Clank's proguard config does this, but WebView also
needs it. The most expedient solution appears to be to move the
declaration to the base proguard config.
BUG=638901
Committed: https://crrev.com/e339d7ccee8a30818ed8d7c511ca0df01b8342c3
Cr-Commit-Position: refs/heads/master@{#412948}
Patch Set 1 #Patch Set 2 : move to base #
Total comments: 1
Patch Set 3 : comment #8 #
Messages
Total messages: 19 (8 generated)
tobiasjs@chromium.org changed reviewers: + torne@chromium.org
Description was changed from ========== Don't let proguard strip the CREATOR static from FileDescriptorInfo. ========== to ========== Don't let proguard rename the CREATOR static of any Parcelable Currently Clank's proguard config does this, but WebView also needs it. The most expedient solution appears to be to move the declaration to the base proguard config. BUG=638901 ==========
tobiasjs@chromium.org changed reviewers: + bauerb@chromium.org
LGTM - I can't think why any apk wouldn't want this, so should be fine in base (it only keeps the name intact if the class is already kept, which should be safe)
smaier@chromium.org changed reviewers: + smaier@chromium.org
https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... File base/android/base_proguard_config.flags (right): https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... base/android/base_proguard_config.flags:41: -keepnames class * implements android.os.Parcelable { This rule should become: -keepclassmembers class * implements android.os.Parcelable { public static *** CREATOR; } (note -keepnames -> -keepclassmembers and final ** -> ***) I haven't had the chance to change it yet.
On 2016/08/18 14:42:29, smaier wrote: > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > File base/android/base_proguard_config.flags (right): > > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > base/android/base_proguard_config.flags:41: -keepnames class * implements > android.os.Parcelable { > This rule should become: > > -keepclassmembers class * implements android.os.Parcelable { > public static *** CREATOR; > } > (note -keepnames -> -keepclassmembers and final ** -> ***) > > I haven't had the chance to change it yet. Ok. I've taken your change. Do we need to also not obfuscate names of Parcelable subclasses? It seems like that's something that your change relaxes.
On 2016/08/18 14:56:27, Tobias Sargeant wrote: > On 2016/08/18 14:42:29, smaier wrote: > > > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > > File base/android/base_proguard_config.flags (right): > > > > > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > > base/android/base_proguard_config.flags:41: -keepnames class * implements > > android.os.Parcelable { > > This rule should become: > > > > -keepclassmembers class * implements android.os.Parcelable { > > public static *** CREATOR; > > } > > (note -keepnames -> -keepclassmembers and final ** -> ***) > > > > I haven't had the chance to change it yet. > > Ok. I've taken your change. Do we need to also not obfuscate names of Parcelable > subclasses? It seems like that's something that your change relaxes. We don't need to keep the names around of those classes. CREATOR is accessed by introspection, so it needs to be kept around, but the class itself doesn't need anything special.
On 2016/08/18 15:02:10, smaier wrote: > On 2016/08/18 14:56:27, Tobias Sargeant wrote: > > On 2016/08/18 14:42:29, smaier wrote: > > > > > > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > > > File base/android/base_proguard_config.flags (right): > > > > > > > > > https://codereview.chromium.org/2251213003/diff/20001/base/android/base_progu... > > > base/android/base_proguard_config.flags:41: -keepnames class * implements > > > android.os.Parcelable { > > > This rule should become: > > > > > > -keepclassmembers class * implements android.os.Parcelable { > > > public static *** CREATOR; > > > } > > > (note -keepnames -> -keepclassmembers and final ** -> ***) > > > > > > I haven't had the chance to change it yet. > > > > Ok. I've taken your change. Do we need to also not obfuscate names of > Parcelable > > subclasses? It seems like that's something that your change relaxes. > > We don't need to keep the names around of those classes. CREATOR is accessed by > introspection, so it needs to be kept around, but the class itself doesn't need > anything special. lgtm
Rubberstamp LGTM
The CQ bit was checked by tobiasjs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org Link to the patchset: https://codereview.chromium.org/2251213003/#ps40001 (title: "comment #8")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Don't let proguard rename the CREATOR static of any Parcelable Currently Clank's proguard config does this, but WebView also needs it. The most expedient solution appears to be to move the declaration to the base proguard config. BUG=638901 ========== to ========== Don't let proguard rename the CREATOR static of any Parcelable Currently Clank's proguard config does this, but WebView also needs it. The most expedient solution appears to be to move the declaration to the base proguard config. BUG=638901 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Don't let proguard rename the CREATOR static of any Parcelable Currently Clank's proguard config does this, but WebView also needs it. The most expedient solution appears to be to move the declaration to the base proguard config. BUG=638901 ========== to ========== Don't let proguard rename the CREATOR static of any Parcelable Currently Clank's proguard config does this, but WebView also needs it. The most expedient solution appears to be to move the declaration to the base proguard config. BUG=638901 Committed: https://crrev.com/e339d7ccee8a30818ed8d7c511ca0df01b8342c3 Cr-Commit-Position: refs/heads/master@{#412948} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/e339d7ccee8a30818ed8d7c511ca0df01b8342c3 Cr-Commit-Position: refs/heads/master@{#412948} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
