Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f82f5ede authored by Nan Wu's avatar Nan Wu
Browse files

Fix collect extra intent keys on server. 

There are 2 issues:
1. in order to improve performance, we only collect keys on server
   if the extra bundle contains intent. This was indicated with a
   bit flag stored in BaseBundle.mFlags. However, the field mFlags
   is not parceled/unparceled. So that won't work - the flag is turned on on the client side and checked on the
   server side. To fix it, use a boolean member and parcel it.
2. This bug is discovered when investigating b/377190225. We found
   out that in collectNestedIntenKeysRecur, the intent was marked
   as keys are collected before it actually collects it. A recent
   change (do not unparcel a bundle when collecting keys) means
   calling this method may not have collected keys if the bundle is
   parceled. The fix is to move the line below after the bundle is
   unparceled.

Bug: 377190225
Test: manual
Flag: EXEMPT bug fix
Change-Id: I645ded2f1e3422c9297962da47f93ca59f6f0b55
parent c7bd665f

core/java/android/content/Intent.java

+1 −1
Original line number Diff line number Diff line
@@ -12426,8 +12426,8 @@ public class Intent implements Parcelable, Cloneable { 
    }















    private void collectNestedIntentKeysRecur(Set<Intent> visited, boolean forceUnparcel) {













        addExtendedFlags(EXTENDED_FLAG_NESTED_INTENT_KEYS_COLLECTED);















        if (mExtras != null && (forceUnparcel || !mExtras.isParcelled()) && !mExtras.isEmpty()) {













            addExtendedFlags(EXTENDED_FLAG_NESTED_INTENT_KEYS_COLLECTED);















            for (String key : mExtras.keySet()) {















                Object value;















                try {

























core/java/android/os/BaseBundle.java



+16
−1






























Original line number
Diff line number
Diff line








@@ -142,6 +142,7 @@ public class BaseBundle {













    /** {@hide} */















    @VisibleForTesting















    public int mFlags;













    private boolean mHasIntent = false;































    /**















     * Constructs a new, empty Bundle that uses a specific ClassLoader for










@@ -258,9 +259,20 @@ public class BaseBundle {





























            // Keep as last statement to ensure visibility of other fields















            mParcelledData = parcelledData;













            mHasIntent = from.mHasIntent;















        }















    }





























    /** @hide */













    public boolean hasIntent() {













        return mHasIntent;













    }



























    /** @hide */













    public void setHasIntent(boolean hasIntent) {













        mHasIntent = hasIntent;













    }





























    /**















     * TODO: optimize this later (getting just the value part of a Bundle















     * with a single pair) once Bundle.forPair() above is implemented










@@ -1837,6 +1849,7 @@ public class BaseBundle {













                    parcel.writeInt(length);















                    parcel.writeInt(mParcelledByNative ? BUNDLE_MAGIC_NATIVE : BUNDLE_MAGIC);















                    parcel.appendFrom(mParcelledData, 0, length);













                    parcel.writeBoolean(mHasIntent);















                }















                return;















            }









@@ -1851,7 +1864,6 @@ public class BaseBundle {













        int lengthPos = parcel.dataPosition();















        parcel.writeInt(-1); // placeholder, will hold length















        parcel.writeInt(BUNDLE_MAGIC);





























        int startPos = parcel.dataPosition();















        parcel.writeArrayMapInternal(map);















        int endPos = parcel.dataPosition();









@@ -1861,6 +1873,7 @@ public class BaseBundle {













        int length = endPos - startPos;















        parcel.writeInt(length);















        parcel.setDataPosition(endPos);













        parcel.writeBoolean(mHasIntent);















    }































    /**










@@ -1904,6 +1917,7 @@ public class BaseBundle {













                mOwnsLazyValues = false;















                initializeFromParcelLocked(parcel, /*ownsParcel*/ false, isNativeBundle);















            }













            mHasIntent = parcel.readBoolean();















            return;















        }

























@@ -1922,6 +1936,7 @@ public class BaseBundle {













        mOwnsLazyValues = true;















        mParcelledByNative = isNativeBundle;















        mParcelledData = p;













        mHasIntent = parcel.readBoolean();















    }































    /** {@hide} */

































core/java/android/os/Bundle.java



+3
−3






























Original line number
Diff line number
Diff line








@@ -398,7 +398,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {













        if ((bundle.mFlags & FLAG_HAS_BINDERS_KNOWN) == 0) {















            mFlags &= ~FLAG_HAS_BINDERS_KNOWN;















        }













        mFlags |= bundle.mFlags & FLAG_HAS_INTENT;













        setHasIntent(hasIntent() || bundle.hasIntent());















    }































    /**










@@ -465,7 +465,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {













     * @hide















     */















    public boolean hasIntent() {













        return (mFlags & FLAG_HAS_INTENT) != 0;













        return super.hasIntent();















    }































    /** {@hide} */










@@ -591,7 +591,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {













        mFlags &= ~FLAG_HAS_FDS_KNOWN;















        mFlags &= ~FLAG_HAS_BINDERS_KNOWN;















        if (intentClass != null && intentClass.isInstance(value)) {













            mFlags |= FLAG_HAS_INTENT;













            setHasIntent(true);















        }















    }

















































core/tests/coretests/src/android/os/BundleTest.java



+32
−14






























Original line number
Diff line number
Diff line








@@ -76,7 +76,7 @@ public class BundleTest {













    /**















     * Create a test bundle, parcel it and return the parcel.















     */













    private Parcel createBundleParcel(boolean withFd) throws Exception {













    private Parcel createBundleParcel(boolean withFd, boolean hasIntent) throws Exception {















        final Bundle source = new Bundle();















        source.putString("string", "abc");















        source.putInt("int", 1);









@@ -85,13 +85,14 @@ public class BundleTest {













            pipe[1].close();















            source.putParcelable("fd", pipe[0]);















        }













        source.setHasIntent(hasIntent);















        return getParcelledBundle(source);















    }































    /**















     * Verify a bundle generated by {@link #createBundleParcel(boolean)}.















     */













    private void checkBundle(Bundle b, boolean withFd) {













    private void checkBundle(Bundle b, boolean withFd, boolean hasIntent) {















        // First, do the checks without actually unparceling the bundle.















        // (Note looking into the contents will unparcel a bundle, so we'll do it later.)















        assertTrue("mParcelledData shouldn't be null here.", b.isParcelled());









@@ -107,6 +108,8 @@ public class BundleTest {













                    b.mFlags & (Bundle.FLAG_HAS_FDS | Bundle.FLAG_HAS_FDS_KNOWN));















        }





























        assertEquals(b.hasIntent(), hasIntent);





























        // Then, check the contents.















        assertEquals("abc", b.getString("string"));















        assertEquals(1, b.getInt("int"));










@@ -139,42 +142,56 @@ public class BundleTest {













        withFd = false;































        // new Bundle with p













        p = createBundleParcel(withFd);













        checkBundle(new Bundle(p), withFd);













        p = createBundleParcel(withFd, false);













        checkBundle(new Bundle(p), withFd, false);















        p.recycle();































        // new Bundle with p and length













        p = createBundleParcel(withFd);













        p = createBundleParcel(withFd, false);















        length = p.readInt();













        checkBundle(new Bundle(p, length), withFd);













        checkBundle(new Bundle(p, length), withFd, false);













        p.recycle();



























        // readFromParcel()













        p = createBundleParcel(withFd, false);













        b = new Bundle();













        b.readFromParcel(p);













        checkBundle(b, withFd, false);















        p.recycle();































        // readFromParcel()













        p = createBundleParcel(withFd);













        p = createBundleParcel(withFd, true);















        b = new Bundle();















        b.readFromParcel(p);













        checkBundle(b, withFd);













        checkBundle(b, withFd, true);















        p.recycle();































        // Same test with FDs.















        withFd = true;































        // new Bundle with p













        p = createBundleParcel(withFd);













        checkBundle(new Bundle(p), withFd);













        p = createBundleParcel(withFd, false);













        checkBundle(new Bundle(p), withFd, false);















        p.recycle();































        // new Bundle with p and length













        p = createBundleParcel(withFd);













        p = createBundleParcel(withFd, false);















        length = p.readInt();













        checkBundle(new Bundle(p, length), withFd);













        checkBundle(new Bundle(p, length), withFd, false);













        p.recycle();



























        // readFromParcel()













        p = createBundleParcel(withFd, false);













        b = new Bundle();













        b.readFromParcel(p);













        checkBundle(b, withFd, false);















        p.recycle();































        // readFromParcel()













        p = createBundleParcel(withFd);













        p = createBundleParcel(withFd, true);















        b = new Bundle();















        b.readFromParcel(p);













        checkBundle(b, withFd);













        checkBundle(b, withFd, true);















        p.recycle();















    }


























@@ -486,6 +503,7 @@ public class BundleTest {













        p.writeInt(131313); // Invalid type















        p.writeInt(0); // Anything, really















        int end = p.dataPosition();













        p.writeBoolean(false);















        p.setDataPosition(0);















        return new Bundle(p, end - start);















    }