From 3587a2a08ec26d11410b67dc91441f2f82a1cb5b Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Fri, 1 Sep 2017 22:28:04 +0300 Subject: [PATCH] Parcelable: Use ClassLoader from the container class to load Parcelable (KT-20027) When the parameter type is just "Parcelable", we would not pick the wrong (system) class loader anymore. --- .../parcel/ParcelableCodegenExtension.kt | 7 +- .../parcel/ParcelableDeclarationChecker.kt | 12 +++- .../parcel/serializers/ParcelSerializer.kt | 20 +++--- .../parcel/serializers/ParcelSerializers.kt | 4 +- .../testData/parcel/codegen/parcelable.kt | 9 +++ .../testData/parcel/codegen/parcelable.txt | 68 +++++++++++++++++++ .../ParcelBytecodeListingTestGenerated.java | 6 ++ 7 files changed, 111 insertions(+), 15 deletions(-) create mode 100644 plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.kt create mode 100644 plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.txt diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableCodegenExtension.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableCodegenExtension.kt index 11f5ccd6104..ea57f969d69 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableCodegenExtension.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableCodegenExtension.kt @@ -114,6 +114,8 @@ open class ParcelableCodegenExtension : ExpressionCodegenExtension { "(${containerAsmType.descriptor}${PARCEL_TYPE.descriptor}I)V", false) } else { + val context = ParcelSerializer.ParcelSerializerContext(codegen.typeMapper, containerAsmType) + for ((fieldName, type) in properties) { val asmType = codegen.typeMapper.mapType(type) @@ -121,7 +123,7 @@ open class ParcelableCodegenExtension : ExpressionCodegenExtension { v.load(0, containerAsmType) v.getfield(containerAsmType.internalName, fieldName, asmType.descriptor) - val serializer = ParcelSerializer.get(type, asmType, codegen.typeMapper) + val serializer = ParcelSerializer.get(type, asmType, context) serializer.writeValue(v) } } @@ -191,12 +193,13 @@ open class ParcelableCodegenExtension : ExpressionCodegenExtension { v.dup() val asmConstructorParameters = StringBuilder() + val context = ParcelSerializer.ParcelSerializerContext(codegen.typeMapper, containerAsmType) for ((_, type) in properties) { val asmType = codegen.typeMapper.mapType(type) asmConstructorParameters.append(asmType.descriptor) - val serializer = ParcelSerializer.get(type, asmType, codegen.typeMapper) + val serializer = ParcelSerializer.get(type, asmType, context) v.load(1, parcelAsmType) serializer.readValue(v) } diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt index c9da521d0b9..25c9f1a21bf 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/ParcelableDeclarationChecker.kt @@ -183,11 +183,16 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker { /* isJvm8TargetWithDefaults */ false) for (parameter in primaryConstructor?.valueParameters.orEmpty()) { - checkParcelableClassProperty(parameter, diagnosticHolder, typeMapper) + checkParcelableClassProperty(parameter, descriptor, diagnosticHolder, typeMapper) } } - private fun checkParcelableClassProperty(parameter: KtParameter, diagnosticHolder: DiagnosticSink, typeMapper: KotlinTypeMapper) { + private fun checkParcelableClassProperty( + parameter: KtParameter, + containerClass: ClassDescriptor, + diagnosticHolder: DiagnosticSink, + typeMapper: KotlinTypeMapper + ) { if (!parameter.hasValOrVar()) { val reportElement = parameter.nameIdentifier ?: parameter diagnosticHolder.reportFromPlugin( @@ -201,7 +206,8 @@ class ParcelableDeclarationChecker : SimpleDeclarationChecker { val asmType = typeMapper.mapType(type) try { - ParcelSerializer.get(type, asmType, typeMapper, strict = true) + val context = ParcelSerializer.ParcelSerializerContext(typeMapper, typeMapper.mapType(containerClass.defaultType)) + ParcelSerializer.get(type, asmType, context, strict = true) } catch (e: IllegalArgumentException) { // get() throws IllegalArgumentException on unknown types diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializer.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializer.kt index 759252aa3e4..bf0132eea4e 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializer.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializer.kt @@ -44,6 +44,8 @@ interface ParcelSerializer { fun writeValue(v: InstructionAdapter) fun readValue(v: InstructionAdapter) + class ParcelSerializerContext(val typeMapper: KotlinTypeMapper, val containerClassType: Type) + companion object { private fun KotlinTypeMapper.mapTypeSafe(type: KotlinType): Type { return if (type.isError) Type.getObjectType("java/lang/Object") else mapType(type) @@ -52,17 +54,19 @@ interface ParcelSerializer { fun get( type: KotlinType, asmType: Type, - typeMapper: KotlinTypeMapper, + context: ParcelSerializerContext, forceBoxed: Boolean = false, strict: Boolean = false ): ParcelSerializer { + val typeMapper = context.typeMapper + val className = asmType.className fun strict() = strict && !type.annotations.hasAnnotation(RAWVALUE_ANNOTATION_FQNAME) return when { asmType.sort == Type.ARRAY -> { val elementType = type.builtIns.getArrayElementType(type) - val elementSerializer = get(elementType, typeMapper.mapTypeSafe(elementType), typeMapper, strict = strict()) + val elementSerializer = get(elementType, typeMapper.mapTypeSafe(elementType), context, strict = strict()) wrapToNullAwareIfNeeded(type, ArrayParcelSerializer(asmType, elementSerializer)) } @@ -90,7 +94,7 @@ interface ParcelSerializer { -> { val elementType = type.arguments.single().type val elementSerializer = get( - elementType, typeMapper.mapTypeSafe(elementType), typeMapper, forceBoxed = true, strict = strict()) + elementType, typeMapper.mapTypeSafe(elementType), context, forceBoxed = true, strict = strict()) wrapToNullAwareIfNeeded(type, ListSetParcelSerializer(asmType, elementSerializer)) } @@ -104,9 +108,9 @@ interface ParcelSerializer { -> { val (keyType, valueType) = type.arguments.apply { assert(this.size == 2) } val keySerializer = get( - keyType.type, typeMapper.mapTypeSafe(keyType.type), typeMapper, forceBoxed = true, strict = strict()) + keyType.type, typeMapper.mapTypeSafe(keyType.type), context, forceBoxed = true, strict = strict()) val valueSerializer = get( - valueType.type, typeMapper.mapTypeSafe(valueType.type), typeMapper, forceBoxed = true, strict = strict()) + valueType.type, typeMapper.mapTypeSafe(valueType.type), context, forceBoxed = true, strict = strict()) wrapToNullAwareIfNeeded(type, MapParcelSerializer(asmType, keySerializer, valueSerializer)) } @@ -153,7 +157,7 @@ interface ParcelSerializer { asmType.isSparseArray() -> { val elementType = type.arguments.single().type val elementSerializer = get( - elementType, typeMapper.mapTypeSafe(elementType), typeMapper, forceBoxed = true, strict = strict()) + elementType, typeMapper.mapTypeSafe(elementType), context, forceBoxed = true, strict = strict()) wrapToNullAwareIfNeeded(type, SparseArrayParcelSerializer(asmType, elementSerializer)) } @@ -186,10 +190,10 @@ interface ParcelSerializer { } creatorAsmType?.let { EfficientParcelableParcelSerializer(asmType, creatorAsmType) } - ?: GenericParcelableParcelSerializer(asmType) + ?: GenericParcelableParcelSerializer(asmType, context.containerClassType) } else { - GenericParcelableParcelSerializer(asmType) + GenericParcelableParcelSerializer(asmType, context.containerClassType) } } diff --git a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializers.kt b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializers.kt index a764c44a613..92d2f698a45 100644 --- a/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializers.kt +++ b/plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/parcel/serializers/ParcelSerializers.kt @@ -462,7 +462,7 @@ internal class EfficientParcelableParcelSerializer(override val asmType: Type, p } } -internal class GenericParcelableParcelSerializer(override val asmType: Type) : ParcelSerializer { +internal class GenericParcelableParcelSerializer(override val asmType: Type, val containerClassType: Type) : ParcelSerializer { override fun writeValue(v: InstructionAdapter) { // -> parcel, parcelable v.aconst(0) // -> parcel, parcelable, flags @@ -471,7 +471,7 @@ internal class GenericParcelableParcelSerializer(override val asmType: Type) : P override fun readValue(v: InstructionAdapter) { // -> parcel - v.aconst(asmType) // -> parcel, type + v.aconst(containerClassType) // -> parcel, type v.invokevirtual("java/lang/Class", "getClassLoader", "()Ljava/lang/ClassLoader;", false) // -> parcel, classloader v.invokevirtual(PARCEL_TYPE.internalName, "readParcelable", "(Ljava/lang/ClassLoader;)Landroid/os/Parcelable;", false) v.castIfNeeded(asmType) diff --git a/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.kt b/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.kt new file mode 100644 index 00000000000..37f7829539a --- /dev/null +++ b/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.kt @@ -0,0 +1,9 @@ +// CURIOUS_ABOUT writeToParcel, createFromParcel, + +package test + +import kotlinx.android.parcel.* +import android.os.Parcelable + +@Parcelize +class Foo(val parcelable: Parcelable): Parcelable \ No newline at end of file diff --git a/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.txt b/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.txt new file mode 100644 index 00000000000..faf1764e22f --- /dev/null +++ b/plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.txt @@ -0,0 +1,68 @@ +public final class test/Foo$Companion : java/lang/Object { + private void () + + public void (kotlin.jvm.internal.DefaultConstructorMarker p0) +} + +public static class test/Foo$Creator : java/lang/Object, android/os/Parcelable$Creator { + public void () + + public final java.lang.Object createFromParcel(android.os.Parcel p0) { + LABEL (L0) + ALOAD (1) + LDC (in) + INVOKESTATIC (kotlin/jvm/internal/Intrinsics, checkParameterIsNotNull, (Ljava/lang/Object;Ljava/lang/String;)V) + NEW + DUP + ALOAD (1) + LDC (Ltest/Foo;) + INVOKEVIRTUAL (java/lang/Class, getClassLoader, ()Ljava/lang/ClassLoader;) + INVOKEVIRTUAL (android/os/Parcel, readParcelable, (Ljava/lang/ClassLoader;)Landroid/os/Parcelable;) + INVOKESPECIAL (test/Foo, , (Landroid/os/Parcelable;)V) + ARETURN + LABEL (L1) + } + + public final test.Foo[] newArray(int p0) +} + +public final class test/Foo : java/lang/Object, android/os/Parcelable { + public final static test.Foo$Creator CREATOR + + public final static test.Foo$Companion Companion + + private final android.os.Parcelable parcelable + + static void () { + NEW + DUP + ACONST_NULL + INVOKESPECIAL (test/Foo$Companion, , (Lkotlin/jvm/internal/DefaultConstructorMarker;)V) + PUTSTATIC (Companion, Ltest/Foo$Companion;) + NEW + DUP + INVOKESPECIAL (test/Foo$Creator, , ()V) + PUTSTATIC (CREATOR, Ltest/Foo$Creator;) + RETURN + } + + public void (android.os.Parcelable p0) + + public final int describeContents() + + public final android.os.Parcelable getParcelable() + + public final void writeToParcel(android.os.Parcel p0, int p1) { + LABEL (L0) + ALOAD (1) + LDC (parcel) + INVOKESTATIC (kotlin/jvm/internal/Intrinsics, checkParameterIsNotNull, (Ljava/lang/Object;Ljava/lang/String;)V) + ALOAD (1) + ALOAD (0) + GETFIELD (parcelable, Landroid/os/Parcelable;) + LDC (0) + INVOKEVIRTUAL (android/os/Parcel, writeParcelable, (Landroid/os/Parcelable;I)V) + RETURN + LABEL (L1) + } +} diff --git a/plugins/plugins-tests/tests/org/jetbrains/kotlin/android/parcel/ParcelBytecodeListingTestGenerated.java b/plugins/plugins-tests/tests/org/jetbrains/kotlin/android/parcel/ParcelBytecodeListingTestGenerated.java index 31df54a04fc..d0388a27337 100644 --- a/plugins/plugins-tests/tests/org/jetbrains/kotlin/android/parcel/ParcelBytecodeListingTestGenerated.java +++ b/plugins/plugins-tests/tests/org/jetbrains/kotlin/android/parcel/ParcelBytecodeListingTestGenerated.java @@ -84,6 +84,12 @@ public class ParcelBytecodeListingTestGenerated extends AbstractParcelBytecodeLi doTest(fileName); } + @TestMetadata("parcelable.kt") + public void testParcelable() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/parcelable.kt"); + doTest(fileName); + } + @TestMetadata("serializable.kt") public void testSerializable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("plugins/android-extensions/android-extensions-compiler/testData/parcel/codegen/serializable.kt");