From def77b5cd5b242770fdacdf405326666d05b431e Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Thu, 10 Oct 2019 09:56:15 +0100 Subject: [PATCH] Make TimestampedValue Parcelable Make TimestampedValue Parcelable for simplicity. TimetampedValue objects are not generally parcelable, depending on the type of the value held. Previously, TimestampedValue did not implement Parcelable to avoid committing to a general contract. Developers parceling TimestampedValue objects were expected to call TimestampedValue.writeToParcel() / TimestampedValue.readFromParcel() explicitly when they knew it was safe to do so. This also meant that TimestampedValues couldn't be used directly via AIDL. This change makes TimestampedValue parcelable because it's more familiar / convenient. Attempts to marshall a TimestampedValue that contains a non-parcelable value will still throw a RuntimeException. Bug: 140712361 Test: atest android.util.TimestampedValueTest Change-Id: I8ca9c72f0433b380ce720cd813f650e743b3ddae --- .../android/app/timedetector/TimeSignal.java | 5 +- core/java/android/util/TimestampedValue.java | 89 ++++++++----------- .../android/util/TimestampedValueTest.java | 29 +++--- 3 files changed, 54 insertions(+), 69 deletions(-) diff --git a/core/java/android/app/timedetector/TimeSignal.java b/core/java/android/app/timedetector/TimeSignal.java index da21794cd6497..b49426000d885 100644 --- a/core/java/android/app/timedetector/TimeSignal.java +++ b/core/java/android/app/timedetector/TimeSignal.java @@ -56,8 +56,7 @@ public final class TimeSignal implements Parcelable { private static TimeSignal createFromParcel(Parcel in) { String sourceId = in.readString(); - TimestampedValue utcTime = - TimestampedValue.readFromParcel(in, null /* classLoader */, Long.class); + TimestampedValue utcTime = in.readParcelable(null /* classLoader */); return new TimeSignal(sourceId, utcTime); } @@ -69,7 +68,7 @@ public final class TimeSignal implements Parcelable { @Override public void writeToParcel(@NonNull Parcel dest, int flags) { dest.writeString(mSourceId); - TimestampedValue.writeToParcel(dest, mUtcTime); + dest.writeParcelable(mUtcTime, 0); } @NonNull diff --git a/core/java/android/util/TimestampedValue.java b/core/java/android/util/TimestampedValue.java index 1289e4db07436..45056730b08bf 100644 --- a/core/java/android/util/TimestampedValue.java +++ b/core/java/android/util/TimestampedValue.java @@ -19,6 +19,7 @@ package android.util; import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Parcel; +import android.os.Parcelable; import android.os.SystemClock; import java.util.Objects; @@ -30,14 +31,14 @@ import java.util.Objects; * If a suitable clock is used the reference time can be used to identify the age of a value or * ordering between values. * - *

To read and write a timestamped value from / to a Parcel see - * {@link #readFromParcel(Parcel, ClassLoader, Class)} and - * {@link #writeToParcel(Parcel, TimestampedValue)}. + *

This class implements {@link Parcelable} for convenience but instances will only actually be + * parcelable if the value type held is {@code null}, {@link Parcelable}, or one of the other types + * supported by {@link Parcel#writeValue(Object)} / {@link Parcel#readValue(ClassLoader)}. * * @param the type of the value with an associated timestamp * @hide */ -public final class TimestampedValue { +public final class TimestampedValue implements Parcelable { private final long mReferenceTimeMillis; private final T mValue; @@ -80,53 +81,6 @@ public final class TimestampedValue { + '}'; } - /** - * Read a {@link TimestampedValue} from a parcel that was stored using - * {@link #writeToParcel(Parcel, TimestampedValue)}. - * - *

The marshalling/unmarshalling of the value relies upon {@link Parcel#writeValue(Object)} - * and {@link Parcel#readValue(ClassLoader)} and so this method can only be used with types - * supported by those methods. - * - * @param in the Parcel to read from - * @param classLoader the ClassLoader to pass to {@link Parcel#readValue(ClassLoader)} - * @param valueClass the expected type of the value, typically the same as {@code } but can - * also be a subclass - * @throws RuntimeException if the value read is not compatible with {@code valueClass} or the - * object could not be read - */ - @SuppressWarnings("unchecked") - @NonNull - public static TimestampedValue readFromParcel( - @NonNull Parcel in, @Nullable ClassLoader classLoader, Class valueClass) { - long referenceTimeMillis = in.readLong(); - T value = (T) in.readValue(classLoader); - // Equivalent to static code: if (!(value.getClass() instanceof {valueClass})) { - if (value != null && !valueClass.isAssignableFrom(value.getClass())) { - throw new RuntimeException("Value was of type " + value.getClass() - + " is not assignable to " + valueClass); - } - return new TimestampedValue<>(referenceTimeMillis, value); - } - - /** - * Write a {@link TimestampedValue} to a parcel so that it can be read using - * {@link #readFromParcel(Parcel, ClassLoader, Class)}. - * - *

The marshalling/unmarshalling of the value relies upon {@link Parcel#writeValue(Object)} - * and {@link Parcel#readValue(ClassLoader)} and so this method can only be used with types - * supported by those methods. - * - * @param dest the Parcel - * @param timestampedValue the value - * @throws RuntimeException if the value could not be written to the Parcel - */ - public static void writeToParcel( - @NonNull Parcel dest, @NonNull TimestampedValue timestampedValue) { - dest.writeLong(timestampedValue.mReferenceTimeMillis); - dest.writeValue(timestampedValue.mValue); - } - /** * Returns the difference in milliseconds between two instance's reference times. */ @@ -134,4 +88,37 @@ public final class TimestampedValue { @NonNull TimestampedValue one, @NonNull TimestampedValue two) { return one.mReferenceTimeMillis - two.mReferenceTimeMillis; } + + public static final @NonNull Parcelable.Creator> CREATOR = + new Parcelable.ClassLoaderCreator>() { + + @Override + public TimestampedValue createFromParcel(@NonNull Parcel source) { + return createFromParcel(source, null); + } + + @Override + public TimestampedValue createFromParcel( + @NonNull Parcel source, @Nullable ClassLoader classLoader) { + long referenceTimeMillis = source.readLong(); + Object value = source.readValue(classLoader); + return new TimestampedValue<>(referenceTimeMillis, value); + } + + @Override + public TimestampedValue[] newArray(int size) { + return new TimestampedValue[size]; + } + }; + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(@NonNull Parcel dest, int flags) { + dest.writeLong(mReferenceTimeMillis); + dest.writeValue(mValue); + } } diff --git a/core/tests/coretests/src/android/util/TimestampedValueTest.java b/core/tests/coretests/src/android/util/TimestampedValueTest.java index 6e3ab796c5d37..6fc2400316c24 100644 --- a/core/tests/coretests/src/android/util/TimestampedValueTest.java +++ b/core/tests/coretests/src/android/util/TimestampedValueTest.java @@ -55,12 +55,12 @@ public class TimestampedValueTest { TimestampedValue stringValue = new TimestampedValue<>(1000, "Hello"); Parcel parcel = Parcel.obtain(); try { - TimestampedValue.writeToParcel(parcel, stringValue); + parcel.writeParcelable(stringValue, 0); parcel.setDataPosition(0); TimestampedValue stringValueCopy = - TimestampedValue.readFromParcel(parcel, null /* classLoader */, String.class); + parcel.readParcelable(null /* classLoader */); assertEquals(stringValue, stringValueCopy); } finally { parcel.recycle(); @@ -72,12 +72,12 @@ public class TimestampedValueTest { TimestampedValue stringValue = new TimestampedValue<>(1000, "Hello"); Parcel parcel = Parcel.obtain(); try { - TimestampedValue.writeToParcel(parcel, stringValue); + parcel.writeParcelable(stringValue, 0); parcel.setDataPosition(0); - TimestampedValue stringValueCopy = - TimestampedValue.readFromParcel(parcel, null /* classLoader */, Object.class); + TimestampedValue stringValueCopy = + parcel.readParcelable(null /* classLoader */); assertEquals(stringValue, stringValueCopy); } finally { parcel.recycle(); @@ -85,15 +85,15 @@ public class TimestampedValueTest { } @Test - public void testParceling_valueClassIncompatible() { - TimestampedValue stringValue = new TimestampedValue<>(1000, "Hello"); + public void testParceling_valueClassNotParcelable() { + // This class is not one supported by Parcel.writeValue(). + class NotParcelable {} + + TimestampedValue notParcelableValue = + new TimestampedValue<>(1000, new NotParcelable()); Parcel parcel = Parcel.obtain(); try { - TimestampedValue.writeToParcel(parcel, stringValue); - - parcel.setDataPosition(0); - - TimestampedValue.readFromParcel(parcel, null /* classLoader */, Double.class); + parcel.writeParcelable(notParcelableValue, 0); fail(); } catch (RuntimeException expected) { } finally { @@ -106,12 +106,11 @@ public class TimestampedValueTest { TimestampedValue nullValue = new TimestampedValue<>(1000, null); Parcel parcel = Parcel.obtain(); try { - TimestampedValue.writeToParcel(parcel, nullValue); + parcel.writeParcelable(nullValue, 0); parcel.setDataPosition(0); - TimestampedValue nullValueCopy = - TimestampedValue.readFromParcel(parcel, null /* classLoader */, String.class); + TimestampedValue nullValueCopy = parcel.readParcelable(null /* classLoader */); assertEquals(nullValue, nullValueCopy); } finally { parcel.recycle();