diff --git a/core/java/android/app/job/JobInfo.java b/core/java/android/app/job/JobInfo.java index 72eea849cbf47..7e8a7decb9e63 100644 --- a/core/java/android/app/job/JobInfo.java +++ b/core/java/android/app/job/JobInfo.java @@ -625,10 +625,6 @@ public class JobInfo implements Parcelable { return hasLateConstraint; } - private static boolean kindofEqualsBundle(BaseBundle a, BaseBundle b) { - return (a == b) || (a != null && a.kindofEquals(b)); - } - @Override public boolean equals(Object o) { if (!(o instanceof JobInfo)) { @@ -639,11 +635,11 @@ public class JobInfo implements Parcelable { return false; } // XXX won't be correct if one is parcelled and the other not. - if (!kindofEqualsBundle(extras, j.extras)) { + if (!BaseBundle.kindofEquals(extras, j.extras)) { return false; } // XXX won't be correct if one is parcelled and the other not. - if (!kindofEqualsBundle(transientExtras, j.transientExtras)) { + if (!BaseBundle.kindofEquals(transientExtras, j.transientExtras)) { return false; } // XXX for now we consider two different clip data objects to be different, diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index d859a3af73a2d..8b6840b09b0fc 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -6632,7 +6632,7 @@ public class Intent implements Parcelable, Cloneable { this.mClipData = new ClipData(o.mClipData); } } else { - if (o.mExtras != null && !o.mExtras.maybeIsEmpty()) { + if (o.mExtras != null && !o.mExtras.isDefinitelyEmpty()) { this.mExtras = Bundle.STRIPPED; } diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java index e6ad917fa0194..c2f67947a60b3 100644 --- a/core/java/android/os/BaseBundle.java +++ b/core/java/android/os/BaseBundle.java @@ -365,12 +365,16 @@ public class BaseBundle { } /** + * This method returns true when the parcel is 'definitely' empty. + * That is, it may return false for an empty parcel. But will never return true for a non-empty + * one. + * * @hide this should probably be the implementation of isEmpty(). To do that we * need to ensure we always use the special empty parcel form when the bundle is * empty. (This may already be the case, but to be safe we'll do this later when * we aren't trying to stabilize.) */ - public boolean maybeIsEmpty() { + public boolean isDefinitelyEmpty() { if (isParcelled()) { return isEmptyParcel(); } else { @@ -402,6 +406,9 @@ public class BaseBundle { if (other == null) { return false; } + if (isDefinitelyEmpty() && other.isDefinitelyEmpty()) { + return true; + } if (isParcelled() != other.isParcelled()) { // Big kind-of here! return false; diff --git a/core/tests/coretests/src/android/os/BundleTest.java b/core/tests/coretests/src/android/os/BundleTest.java index e4dc993478029..4cc70ba6448dc 100644 --- a/core/tests/coretests/src/android/os/BundleTest.java +++ b/core/tests/coretests/src/android/os/BundleTest.java @@ -30,12 +30,23 @@ import org.junit.runner.RunWith; * Unit tests for bundle that requires accessing hidden APS. Tests that can be written only with * public APIs should go in the CTS counterpart. * - * Run with: - * bit FrameworksCoreTests:android.os.BundleTest + * Run with: atest FrameworksCoreTests:android.os.BundleTest */ @SmallTest @RunWith(AndroidJUnit4.class) public class BundleTest { + + /** + * Take a bundle, write it to a parcel and return the parcel. + */ + private Parcel getParcelledBundle(Bundle bundle) { + final Parcel p = Parcel.obtain(); + // Don't use p.writeParcelabe(), which would write the creator, which we don't need. + bundle.writeToParcel(p, 0); + p.setDataPosition(0); + return p; + } + /** * Create a test bundle, parcel it and return the parcel. */ @@ -48,12 +59,7 @@ public class BundleTest { pipe[1].close(); source.putParcelable("fd", pipe[0]); } - final Parcel p = Parcel.obtain(); - // Don't use p.writeParcelabe(), which would write the creator, which we don't need. - source.writeToParcel(p, 0); - p.setDataPosition(0); - - return p; + return getParcelledBundle(source); } /** @@ -137,4 +143,78 @@ public class BundleTest { checkBundle(b, withFd); p.recycle(); } + + @Test + public void kindofEquals_bothUnparcelled_same() { + Bundle bundle1 = new Bundle(); + bundle1.putString("StringKey", "S"); + bundle1.putInt("IntKey", 2); + + Bundle bundle2 = new Bundle(); + bundle2.putString("StringKey", "S"); + bundle2.putInt("IntKey", 2); + + assertTrue(BaseBundle.kindofEquals(bundle1, bundle2)); + } + + @Test + public void kindofEquals_bothUnparcelled_different() { + Bundle bundle1 = new Bundle(); + bundle1.putString("StringKey", "S"); + bundle1.putInt("IntKey", 2); + + Bundle bundle2 = new Bundle(); + bundle2.putString("StringKey", "T"); + bundle2.putLong("LongKey", 30L); + + assertFalse(BaseBundle.kindofEquals(bundle1, bundle2)); + } + + @Test + public void kindofEquals_bothParcelled_same() { + Bundle bundle1 = new Bundle(); + bundle1.putString("StringKey", "S"); + bundle1.putInt("IntKey", 2); + bundle1.readFromParcel(getParcelledBundle(bundle1)); + + Bundle bundle2 = new Bundle(); + bundle2.putString("StringKey", "S"); + bundle2.putInt("IntKey", 2); + bundle2.readFromParcel(getParcelledBundle(bundle2)); + + assertTrue(bundle1.isParcelled()); + assertTrue(bundle2.isParcelled()); + assertTrue(BaseBundle.kindofEquals(bundle1, bundle2)); + } + + @Test + public void kindofEquals_bothParcelled_different() { + Bundle bundle1 = new Bundle(); + bundle1.putString("StringKey", "S"); + bundle1.putInt("IntKey", 2); + bundle1.readFromParcel(getParcelledBundle(bundle1)); + + Bundle bundle2 = new Bundle(); + bundle2.putString("StringKey", "T"); + bundle2.putLong("LongKey", 5); + bundle2.readFromParcel(getParcelledBundle(bundle2)); + + assertTrue(bundle1.isParcelled()); + assertTrue(bundle2.isParcelled()); + assertFalse(BaseBundle.kindofEquals(bundle1, bundle2)); + } + + @Test + public void kindofEquals_ParcelledUnparcelled_empty() { + Bundle bundle1 = new Bundle(); + bundle1.readFromParcel(getParcelledBundle(bundle1)); + + Bundle bundle2 = new Bundle(); + + assertTrue(bundle1.isParcelled()); + assertFalse(bundle2.isParcelled()); + // Even though one is parcelled and the other is not, both are empty, so it should + // return true + assertTrue(BaseBundle.kindofEquals(bundle1, bundle2)); + } } diff --git a/services/core/java/com/android/server/job/controllers/JobStatus.java b/services/core/java/com/android/server/job/controllers/JobStatus.java index e655b35d930fe..6fa99235ceaae 100644 --- a/services/core/java/com/android/server/job/controllers/JobStatus.java +++ b/services/core/java/com/android/server/job/controllers/JobStatus.java @@ -1502,11 +1502,11 @@ public final class JobStatus { pw.println(); } } - if (job.getExtras() != null && !job.getExtras().maybeIsEmpty()) { + if (job.getExtras() != null && !job.getExtras().isDefinitelyEmpty()) { pw.print(prefix); pw.print(" Extras: "); pw.println(job.getExtras().toShortString()); } - if (job.getTransientExtras() != null && !job.getTransientExtras().maybeIsEmpty()) { + if (job.getTransientExtras() != null && !job.getTransientExtras().isDefinitelyEmpty()) { pw.print(prefix); pw.print(" Transient extras: "); pw.println(job.getTransientExtras().toShortString()); } @@ -1702,10 +1702,10 @@ public final class JobStatus { job.getTriggerContentMaxDelay()); } } - if (job.getExtras() != null && !job.getExtras().maybeIsEmpty()) { + if (job.getExtras() != null && !job.getExtras().isDefinitelyEmpty()) { job.getExtras().writeToProto(proto, JobStatusDumpProto.JobInfo.EXTRAS); } - if (job.getTransientExtras() != null && !job.getTransientExtras().maybeIsEmpty()) { + if (job.getTransientExtras() != null && !job.getTransientExtras().isDefinitelyEmpty()) { job.getTransientExtras().writeToProto(proto, JobStatusDumpProto.JobInfo.TRANSIENT_EXTRAS); } if (job.getClipData() != null) {