Fix issue #10807048: Groupon: The application crash when user rotates...

...device to another mode (portrait or landscape) on Main page.

So, it turns out that Bundle claimed to have an invariant that either
mParcelledData or mMap would hold its data, never both.  The new
implementation on top of ArrayMap assumed this was the case.  However,
there is one situation where it is not true: an application can take
an existing Bundle that contains data, and call readFromParcel() on it.
The implementation of readFromParcel() would just pull out the
parceled data and stuff it in to mParcelledData for later unparceling,
even if that Bundle already had a non-empty mMap.

To fix this, we just look for this case in readFromParcel() and
immediately unparcel at that point into the existing map, using a
new unparcelling method that doesn't rely on the target map being
empty.

Change-Id: Ib816b6876a6cd2760b7a3372c7a79ca2f12dfeba

diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java
index 32b1b60..f47ac4e 100644 (file)
--- a/core/java/android/os/Bundle.java
+++ b/core/java/android/os/Bundle.java
@@ -33,6 +33,8 @@ public final class Bundle implements Parcelable, Cloneable {
     private static final String LOG_TAG = "Bundle";
     public static final Bundle EMPTY;
 
+    static final int BUNDLE_MAGIC = 0x4C444E42; // 'B' 'N' 'D' 'L'
+
     static {
         EMPTY = new Bundle();
         EMPTY.mMap = ArrayMap.EMPTY;
@@ -1643,11 +1645,11 @@ public final class Bundle implements Parcelable, Cloneable {
             if (mParcelledData != null) {
                 int length = mParcelledData.dataSize();
                 parcel.writeInt(length);
-                parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'
+                parcel.writeInt(BUNDLE_MAGIC);
                 parcel.appendFrom(mParcelledData, 0, length);
             } else {
                 parcel.writeInt(-1); // dummy, will hold length
-                parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'
+                parcel.writeInt(BUNDLE_MAGIC);
     
                 int oldPos = parcel.dataPosition();
                 parcel.writeArrayMapInternal(mMap);
@@ -1679,11 +1681,10 @@ public final class Bundle implements Parcelable, Cloneable {
 
     void readFromParcelInner(Parcel parcel, int length) {
         int magic = parcel.readInt();
-        if (magic != 0x4C444E42) {
+        if (magic != BUNDLE_MAGIC) {
             //noinspection ThrowableInstanceNeverThrown
-            String st = Log.getStackTraceString(new RuntimeException());
-            Log.e("Bundle", "readBundle: bad magic number");
-            Log.e("Bundle", "readBundle: trace = " + st);
+            throw new IllegalStateException("Bad magic number for Bundle: 0x"
+                    + Integer.toHexString(magic));
         }
 
         // Advance within this Parcel
@@ -1694,10 +1695,23 @@ public final class Bundle implements Parcelable, Cloneable {
         p.setDataPosition(0);
         p.appendFrom(parcel, offset, length);
         p.setDataPosition(0);
-        
-        mParcelledData = p;
-        mHasFds = p.hasFileDescriptors();
-        mFdsKnown = true;
+
+        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;
+        }
     }
 
     @Override

diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index fec2a3e..5f3a81c 100644 (file)
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -178,6 +178,7 @@ import java.util.Set;
  */
 public final class Parcel {
     private static final boolean DEBUG_RECYCLE = false;
+    private static final boolean DEBUG_ARRAY_MAP = false;
     private static final String TAG = "Parcel";
 
     @SuppressWarnings({"UnusedDeclaration"})
@@ -605,7 +606,14 @@ public final class Parcel {
         }
         final int N = val.size();
         writeInt(N);
+        if (DEBUG_ARRAY_MAP) {
+            RuntimeException here =  new RuntimeException("here");
+            here.fillInStackTrace();
+            Log.d(TAG, "Writing " + N + " ArrayMap entries", here);
+        }
         for (int i=0; i<N; i++) {
+            if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Write #" + i + ": key=0x"
+                    + (val.keyAt(i) != null ? val.keyAt(i).hashCode() : 0) + " " + val.keyAt(i));
             writeValue(val.keyAt(i));
             writeValue(val.valueAt(i));
         }
@@ -2289,14 +2297,38 @@ public final class Parcel {
 
     /* package */ void readArrayMapInternal(ArrayMap outVal, int N,
         ClassLoader loader) {
+        if (DEBUG_ARRAY_MAP) {
+            RuntimeException here =  new RuntimeException("here");
+            here.fillInStackTrace();
+            Log.d(TAG, "Reading " + N + " ArrayMap entries", here);
+        }
         while (N > 0) {
             Object key = readValue(loader);
+            if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Read #" + (N-1) + ": key=0x"
+                    + (key != null ? key.hashCode() : 0) + " " + key);
             Object value = readValue(loader);
             outVal.append(key, value);
             N--;
         }
     }
 
+    /* package */ void readArrayMapSafelyInternal(ArrayMap outVal, int N,
+        ClassLoader loader) {
+        if (DEBUG_ARRAY_MAP) {
+            RuntimeException here =  new RuntimeException("here");
+            here.fillInStackTrace();
+            Log.d(TAG, "Reading safely " + N + " ArrayMap entries", here);
+        }
+        while (N > 0) {
+            Object key = readValue(loader);
+            if (DEBUG_ARRAY_MAP) Log.d(TAG, "  Read safe #" + (N-1) + ": key=0x"
+                    + (key != null ? key.hashCode() : 0) + " " + key);
+            Object value = readValue(loader);
+            outVal.put(key, value);
+            N--;
+        }
+    }
+
     private void readListInternal(List outVal, int N,
         ClassLoader loader) {
         while (N > 0) {