Merge "Refactor Bundle for thread-safety of Bundle.hasFileDescriptors()"
diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java
index 2a344f6..ad3de25 100644
--- a/core/java/android/os/BaseBundle.java
+++ b/core/java/android/os/BaseBundle.java
@@ -103,7 +103,7 @@
* are unparcelled, mParcelledData willbe set to null.
*/
@UnsupportedAppUsage
- Parcel mParcelledData = null;
+ volatile Parcel mParcelledData = null;
/**
* Whether {@link #mParcelledData} was generated by native code or not.
@@ -182,13 +182,56 @@
* @param b a Bundle to be copied.
*/
BaseBundle(BaseBundle b) {
- copyInternal(b, false);
+ this(b, /* deep */ false);
}
/**
- * Special constructor that does not initialize the bundle.
+ * Constructs a {@link BaseBundle} containing a copy of {@code from}.
+ *
+ * @param from The bundle to be copied.
+ * @param deep Whether is a deep or shallow copy.
+ *
+ * @hide
*/
- BaseBundle(boolean doInit) {
+ BaseBundle(BaseBundle from, boolean deep) {
+ synchronized (from) {
+ mClassLoader = from.mClassLoader;
+
+ if (from.mMap != null) {
+ if (!deep) {
+ mMap = new ArrayMap<>(from.mMap);
+ } else {
+ final ArrayMap<String, Object> fromMap = from.mMap;
+ final int n = fromMap.size();
+ mMap = new ArrayMap<>(n);
+ for (int i = 0; i < n; i++) {
+ mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
+ }
+ }
+ } else {
+ mMap = null;
+ }
+
+ final Parcel parcelledData;
+ if (from.mParcelledData != null) {
+ if (from.isEmptyParcel()) {
+ parcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
+ mParcelledByNative = false;
+ } else {
+ parcelledData = Parcel.obtain();
+ parcelledData.appendFrom(from.mParcelledData, 0,
+ from.mParcelledData.dataSize());
+ parcelledData.setDataPosition(0);
+ mParcelledByNative = from.mParcelledByNative;
+ }
+ } else {
+ parcelledData = null;
+ mParcelledByNative = false;
+ }
+
+ // Keep as last statement to ensure visibility of other fields
+ mParcelledData = parcelledData;
+ }
}
/**
@@ -323,8 +366,8 @@
} else {
mMap.erase();
}
- mParcelledData = null;
mParcelledByNative = false;
+ mParcelledData = null;
return;
}
@@ -358,8 +401,8 @@
if (recycleParcel) {
recycleParcel(parcelledData);
}
- mParcelledData = null;
mParcelledByNative = false;
+ mParcelledData = null;
}
if (DEBUG) {
Log.d(TAG, "unparcel " + Integer.toHexString(System.identityHashCode(this))
@@ -501,44 +544,7 @@
mMap.clear();
}
- void copyInternal(BaseBundle from, boolean deep) {
- synchronized (from) {
- if (from.mParcelledData != null) {
- if (from.isEmptyParcel()) {
- mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
- mParcelledByNative = false;
- } else {
- mParcelledData = Parcel.obtain();
- mParcelledData.appendFrom(from.mParcelledData, 0,
- from.mParcelledData.dataSize());
- mParcelledData.setDataPosition(0);
- mParcelledByNative = from.mParcelledByNative;
- }
- } else {
- mParcelledData = null;
- mParcelledByNative = false;
- }
-
- if (from.mMap != null) {
- if (!deep) {
- mMap = new ArrayMap<>(from.mMap);
- } else {
- final ArrayMap<String, Object> fromMap = from.mMap;
- final int N = fromMap.size();
- mMap = new ArrayMap<>(N);
- for (int i = 0; i < N; i++) {
- mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
- }
- }
- } else {
- mMap = null;
- }
-
- mClassLoader = from.mClassLoader;
- }
- }
-
- Object deepCopyValue(Object value) {
+ private Object deepCopyValue(Object value) {
if (value == null) {
return null;
}
@@ -570,7 +576,7 @@
return value;
}
- ArrayList deepcopyArrayList(ArrayList from) {
+ private ArrayList deepcopyArrayList(ArrayList from) {
final int N = from.size();
ArrayList out = new ArrayList(N);
for (int i=0; i<N; i++) {
@@ -1717,9 +1723,9 @@
if (length < 0) {
throw new RuntimeException("Bad length in parcel: " + length);
} else if (length == 0) {
+ mParcelledByNative = false;
// Empty Bundle or end of data.
mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
- mParcelledByNative = false;
return;
} else if (length % 4 != 0) {
throw new IllegalStateException("Bundle length is not aligned by 4: " + length);
@@ -1757,8 +1763,8 @@
+ ": " + length + " bundle bytes starting at " + offset);
p.setDataPosition(0);
- mParcelledData = p;
mParcelledByNative = isNativeBundle;
+ mParcelledData = p;
}
/** {@hide} */
diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java
index feca81e..b2bbfd6 100644
--- a/core/java/android/os/Bundle.java
+++ b/core/java/android/os/Bundle.java
@@ -102,6 +102,18 @@
}
/**
+ * Constructs a {@link Bundle} containing a copy of {@code from}.
+ *
+ * @param from The bundle to be copied.
+ * @param deep Whether is a deep or shallow copy.
+ *
+ * @hide
+ */
+ Bundle(Bundle from, boolean deep) {
+ super(from, deep);
+ }
+
+ /**
* If {@link #mParcelledData} is not null, copy the HAS FDS bit from it because it's fast.
* Otherwise (if {@link #mParcelledData} is already null), leave {@link #FLAG_HAS_FDS_KNOWN}
* unset, because scanning a map is slower. We'll do it lazily in
@@ -167,13 +179,6 @@
}
/**
- * Constructs a Bundle without initializing it.
- */
- Bundle(boolean doInit) {
- super(doInit);
- }
-
- /**
* Make a Bundle for a single key/value pair.
*
* @hide
@@ -260,9 +265,7 @@
* are referenced as-is and not copied in any way.
*/
public Bundle deepCopy() {
- Bundle b = new Bundle(false);
- b.copyInternal(this, true);
- return b;
+ return new Bundle(this, /* deep */ true);
}
/**
diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
index 7b55e710..f4edcb1 100644
--- a/core/java/android/os/PersistableBundle.java
+++ b/core/java/android/os/PersistableBundle.java
@@ -156,10 +156,15 @@
}
/**
- * Constructs a PersistableBundle without initializing it.
+ * Constructs a {@link PersistableBundle} containing a copy of {@code from}.
+ *
+ * @param from The bundle to be copied.
+ * @param deep Whether is a deep or shallow copy.
+ *
+ * @hide
*/
- PersistableBundle(boolean doInit) {
- super(doInit);
+ PersistableBundle(PersistableBundle from, boolean deep) {
+ super(from, deep);
}
/**
@@ -190,9 +195,7 @@
* are referenced as-is and not copied in any way.
*/
public PersistableBundle deepCopy() {
- PersistableBundle b = new PersistableBundle(false);
- b.copyInternal(this, true);
- return b;
+ return new PersistableBundle(this, /* deep */ true);
}
/**