From 4a7d824c3b41eafc4ff91d3253ff8a9ebd60a454 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Thu, 3 Oct 2013 10:19:20 -0700 Subject: [PATCH] Fix issue #10921903: CTS: android.os.cts.ParcelTest#testReadBundle... ...fails from KRS84 across all platforms My fix for issue #10807048 was wrong, wrong, wrong. The problem was actually just a stupid mistake in ArrayMap.erase(). This makes it all right. Change-Id: I762f7a2d5100bceb86a091ab3d6368edc21b4266 --- core/java/android/os/Bundle.java | 49 ++++++++++++++++++------------------ core/java/android/os/Parcel.java | 1 + core/java/android/util/ArrayMap.java | 1 + 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java index f47ac4e49750..5a919fb8209e 100644 --- a/core/java/android/os/Bundle.java +++ b/core/java/android/os/Bundle.java @@ -30,7 +30,8 @@ import java.util.Set; * */ public final class Bundle implements Parcelable, Cloneable { - private static final String LOG_TAG = "Bundle"; + private static final String TAG = "Bundle"; + static final boolean DEBUG = false; public static final Bundle EMPTY; static final int BUNDLE_MAGIC = 0x4C444E42; // 'B' 'N' 'D' 'L' @@ -157,7 +158,7 @@ public final class Bundle implements Parcelable, Cloneable { unparcel(); int size = mMap.size(); if (size > 1) { - Log.w(LOG_TAG, "getPairValue() used on Bundle with multiple pairs."); + Log.w(TAG, "getPairValue() used on Bundle with multiple pairs."); } if (size == 0) { return null; @@ -210,10 +211,14 @@ public final class Bundle implements Parcelable, Cloneable { */ /* package */ synchronized void unparcel() { if (mParcelledData == null) { + if (DEBUG) Log.d(TAG, "unparcel " + Integer.toHexString(System.identityHashCode(this)) + + ": no parcelled data"); return; } int N = mParcelledData.readInt(); + if (DEBUG) Log.d(TAG, "unparcel " + Integer.toHexString(System.identityHashCode(this)) + + ": reading " + N + " maps"); if (N < 0) { return; } @@ -226,6 +231,8 @@ public final class Bundle implements Parcelable, Cloneable { mParcelledData.readArrayMapInternal(mMap, N, mClassLoader); mParcelledData.recycle(); mParcelledData = null; + if (DEBUG) Log.d(TAG, "unparcel " + Integer.toHexString(System.identityHashCode(this)) + + " final map: " + mMap); } /** @@ -794,6 +801,8 @@ public final class Bundle implements Parcelable, Cloneable { */ public boolean getBoolean(String key) { unparcel(); + if (DEBUG) Log.d(TAG, "Getting boolean in " + + Integer.toHexString(System.identityHashCode(this))); return getBoolean(key, false); } @@ -810,8 +819,8 @@ public final class Bundle implements Parcelable, Cloneable { sb.append(". The default value "); sb.append(defaultValue); sb.append(" was returned."); - Log.w(LOG_TAG, sb.toString()); - Log.w(LOG_TAG, "Attempt to cast generated internal exception:", e); + Log.w(TAG, sb.toString()); + Log.w(TAG, "Attempt to cast generated internal exception:", e); } private void typeWarning(String key, Object value, String className, @@ -1648,18 +1657,19 @@ public final class Bundle implements Parcelable, Cloneable { parcel.writeInt(BUNDLE_MAGIC); parcel.appendFrom(mParcelledData, 0, length); } else { + int lengthPos = parcel.dataPosition(); parcel.writeInt(-1); // dummy, will hold length parcel.writeInt(BUNDLE_MAGIC); - int oldPos = parcel.dataPosition(); + int startPos = parcel.dataPosition(); parcel.writeArrayMapInternal(mMap); - int newPos = parcel.dataPosition(); + int endPos = parcel.dataPosition(); // Backpatch length - parcel.setDataPosition(oldPos - 8); - int length = newPos - oldPos; + parcel.setDataPosition(lengthPos); + int length = endPos - startPos; parcel.writeInt(length); - parcel.setDataPosition(newPos); + parcel.setDataPosition(endPos); } } finally { parcel.restoreAllowFds(oldAllowFds); @@ -1694,24 +1704,13 @@ public final class Bundle implements Parcelable, Cloneable { Parcel p = Parcel.obtain(); p.setDataPosition(0); p.appendFrom(parcel, offset, length); + if (DEBUG) Log.d(TAG, "Retrieving " + Integer.toHexString(System.identityHashCode(this)) + + ": " + length + " bundle bytes starting at " + offset); p.setDataPosition(0); - if (mMap != null) { - // It is not allowed to have a Bundle with both a map and a parcel, so if we - // already have a map then we need to immediately unparcel into it. This also - // lets us know we need to go through the slow path of unparceling, since the - // map may already contains some data so the two need to be merged. - if (mFdsKnown) { - mHasFds |= p.hasFileDescriptors(); - } - int N = p.readInt(); - p.readArrayMapSafelyInternal(mMap, N, mClassLoader); - p.recycle(); - } else { - mParcelledData = p; - mHasFds = p.hasFileDescriptors(); - mFdsKnown = true; - } + mParcelledData = p; + mHasFds = p.hasFileDescriptors(); + mFdsKnown = true; } @Override diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 5f3a81c9c7a6..02b1998264a2 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -1610,6 +1610,7 @@ public final class Parcel { public final Bundle readBundle(ClassLoader loader) { int length = readInt(); if (length < 0) { + if (Bundle.DEBUG) Log.d(TAG, "null bundle: length=" + length); return null; } diff --git a/core/java/android/util/ArrayMap.java b/core/java/android/util/ArrayMap.java index fa534cc8f502..df1d4cd7047a 100644 --- a/core/java/android/util/ArrayMap.java +++ b/core/java/android/util/ArrayMap.java @@ -292,6 +292,7 @@ public final class ArrayMap implements Map { for (int i=0; i