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

Commit af9d667c authored by Kenny Root's avatar Kenny Root
Browse files

OBB: rearrange to be entirely asynchronous 

Rearrange structure of MountService handling of OBBs to be entirely
asynchronous so we don't rely on locking as much. We still need the
locking to support dumpsys which has been improved to output all the
data structures for OBBs.

Added more tests to cover more of the error return codes.

Oh and fix a logic inversion bug.

Change-Id: I34f541192dbbb1903b24825889b8fa8f43e6e2a9
parent ce2f1909

api/current.xml

+89 −3
Original line number Diff line number Diff line
@@ -131704,9 +131704,97 @@ 
>

<parameter name="path" type="java.lang.String">

</parameter>

<parameter name="state" type="java.lang.String">

<parameter name="state" type="int">

</parameter>

</method>

<field name="ERROR_ALREADY_MOUNTED"

 type="int"

 transient="false"

 volatile="false"

 value="24"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="ERROR_COULD_NOT_MOUNT"

 type="int"

 transient="false"

 volatile="false"

 value="21"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="ERROR_COULD_NOT_UNMOUNT"

 type="int"

 transient="false"

 volatile="false"

 value="22"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="ERROR_INTERNAL"

 type="int"

 transient="false"

 volatile="false"

 value="20"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="ERROR_NOT_MOUNTED"

 type="int"

 transient="false"

 volatile="false"

 value="23"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="ERROR_PERMISSION_DENIED"

 type="int"

 transient="false"

 volatile="false"

 value="25"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="MOUNTED"

 type="int"

 transient="false"

 volatile="false"

 value="1"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

<field name="UNMOUNTED"

 type="int"

 transient="false"

 volatile="false"

 value="2"

 static="true"

 final="true"

 deprecated="not deprecated"

 visibility="public"

>

</field>

</class>

<class name="StorageManager"

 extends="java.lang.Object"
@@ -131741,8 +131829,6 @@ 
>

<parameter name="filename" type="java.lang.String">

</parameter>

<exception name="IllegalArgumentException" type="java.lang.IllegalArgumentException">

</exception>

</method>

<method name="mountObb"

 return="boolean"

core/java/android/os/storage/IMountService.java

+13 −7
Original line number Diff line number Diff line
@@ -484,7 +484,7 @@ public interface IMountService extends IInterface { 
             * IObbActionListener to inform it of the terminal state of the

             * call.

             */

            public void mountObb(String filename, String key, IObbActionListener token)

            public void mountObb(String filename, String key, IObbActionListener token, int nonce)

                    throws RemoteException {

                Parcel _data = Parcel.obtain();

                Parcel _reply = Parcel.obtain();
@@ -493,6 +493,7 @@ public interface IMountService extends IInterface { 
                    _data.writeString(filename);

                    _data.writeString(key);

                    _data.writeStrongBinder((token != null ? token.asBinder() : null));

                    _data.writeInt(nonce);

                    mRemote.transact(Stub.TRANSACTION_mountObb, _data, _reply, 0);

                    _reply.readException();

                } finally {
@@ -508,8 +509,8 @@ public interface IMountService extends IInterface { 
             * IObbActionListener to inform it of the terminal state of the

             * call.

             */

            public void unmountObb(String filename, boolean force, IObbActionListener token)

                    throws RemoteException {

            public void unmountObb(String filename, boolean force, IObbActionListener token,

                    int nonce) throws RemoteException {

                Parcel _data = Parcel.obtain();

                Parcel _reply = Parcel.obtain();

                try {
@@ -517,6 +518,7 @@ public interface IMountService extends IInterface { 
                    _data.writeString(filename);

                    _data.writeInt((force ? 1 : 0));

                    _data.writeStrongBinder((token != null ? token.asBinder() : null));

                    _data.writeInt(nonce);

                    mRemote.transact(Stub.TRANSACTION_unmountObb, _data, _reply, 0);

                    _reply.readException();

                } finally {
@@ -855,7 +857,9 @@ public interface IMountService extends IInterface { 
                    key = data.readString();

                    IObbActionListener observer;

                    observer = IObbActionListener.Stub.asInterface(data.readStrongBinder());

                    mountObb(filename, key, observer);

                    int nonce;

                    nonce = data.readInt();

                    mountObb(filename, key, observer, nonce);

                    reply.writeNoException();

                    return true;

                }
@@ -867,7 +871,9 @@ public interface IMountService extends IInterface { 
                    force = 0 != data.readInt();

                    IObbActionListener observer;

                    observer = IObbActionListener.Stub.asInterface(data.readStrongBinder());

                    unmountObb(filename, force, observer);

                    int nonce;

                    nonce = data.readInt();

                    unmountObb(filename, force, observer, nonce);

                    reply.writeNoException();

                    return true;

                }
@@ -979,7 +985,7 @@ public interface IMountService extends IInterface { 
     * MountService will call back to the supplied IObbActionListener to inform

     * it of the terminal state of the call.

     */

    public void mountObb(String filename, String key, IObbActionListener token)

    public void mountObb(String filename, String key, IObbActionListener token, int nonce)

            throws RemoteException;



    /*
@@ -1023,7 +1029,7 @@ public interface IMountService extends IInterface { 
     * MountService will call back to the supplied IObbActionListener to inform

     * it of the terminal state of the call.

     */

    public void unmountObb(String filename, boolean force, IObbActionListener token)

    public void unmountObb(String filename, boolean force, IObbActionListener token, int nonce)

            throws RemoteException;



    /*

core/java/android/os/storage/IObbActionListener.java

+12 −7
Original line number Diff line number Diff line
@@ -69,9 +69,11 @@ public interface IObbActionListener extends IInterface { 
                    data.enforceInterface(DESCRIPTOR);

                    String filename;

                    filename = data.readString();

                    String status;

                    status = data.readString();

                    this.onObbResult(filename, status);

                    int nonce;

                    nonce = data.readInt();

                    int status;

                    status = data.readInt();

                    this.onObbResult(filename, nonce, status);

                    reply.writeNoException();

                    return true;

                }
@@ -101,13 +103,15 @@ public interface IObbActionListener extends IInterface { 
             *            on

             * @param returnCode status of the operation

             */

            public void onObbResult(String filename, String status) throws RemoteException {

            public void onObbResult(String filename, int nonce, int status)

                    throws RemoteException {

                Parcel _data = Parcel.obtain();

                Parcel _reply = Parcel.obtain();

                try {

                    _data.writeInterfaceToken(DESCRIPTOR);

                    _data.writeString(filename);

                    _data.writeString(status);

                    _data.writeInt(nonce);

                    _data.writeInt(status);

                    mRemote.transact(Stub.TRANSACTION_onObbResult, _data, _reply, 0);

                    _reply.readException();

                } finally {
@@ -124,7 +128,8 @@ public interface IObbActionListener extends IInterface { 
     * Return from an OBB action result.

     * 

     * @param filename the path to the OBB the operation was performed on

     * @param returnCode status of the operation

     * @param nonce identifier that is meaningful to the receiver

     * @param status status code as defined in {@link OnObbStateChangeListener}

     */

    public void onObbResult(String filename, String status) throws RemoteException;

    public void onObbResult(String filename, int nonce, int status) throws RemoteException;

}

core/java/android/os/storage/OnObbStateChangeListener.java

+56 −2
Original line number Diff line number Diff line
@@ -17,15 +17,69 @@ 
package android.os.storage;



/**

 * Used for receiving notifications from {@link StorageManager}.

 * Used for receiving notifications from {@link StorageManager} about OBB file

 * states.

 */

public abstract class OnObbStateChangeListener {



    /**

     * The OBB container is now mounted and ready for use. Returned in status

     * messages from calls made via {@link StorageManager}

     */

    public static final int MOUNTED = 1;



    /**

     * The OBB container is now unmounted and not usable. Returned in status

     * messages from calls made via {@link StorageManager}

     */

    public static final int UNMOUNTED = 2;



    /**

     * There was an internal system error encountered while trying to mount the

     * OBB. Returned in status messages from calls made via

     * {@link StorageManager}

     */

    public static final int ERROR_INTERNAL = 20;



    /**

     * The OBB could not be mounted by the system. Returned in status messages

     * from calls made via {@link StorageManager}

     */

    public static final int ERROR_COULD_NOT_MOUNT = 21;



    /**

     * The OBB could not be unmounted. This most likely indicates that a file is

     * in use on the OBB. Returned in status messages from calls made via

     * {@link StorageManager}

     */

    public static final int ERROR_COULD_NOT_UNMOUNT = 22;



    /**

     * A call was made to unmount the OBB when it was not mounted. Returned in

     * status messages from calls made via {@link StorageManager}

     */

    public static final int ERROR_NOT_MOUNTED = 23;



    /**

     * The OBB has already been mounted. Returned in status messages from calls

     * made via {@link StorageManager}

     */

    public static final int ERROR_ALREADY_MOUNTED = 24;



    /**

     * The current application does not have permission to use this OBB because

     * the OBB indicates it's owned by a different package or the key used to

     * open it is incorrect. Returned in status messages from calls made via

     * {@link StorageManager}

     */

    public static final int ERROR_PERMISSION_DENIED = 25;



    /**

     * Called when an OBB has changed states.

     * 

     * @param path path to the OBB file the state change has happened on

     * @param state the current state of the OBB

     */

    public void onObbStateChange(String path, String state) {

    public void onObbStateChange(String path, int state) {

    }

}

core/java/android/os/storage/StorageManager.java

+70 −59
Original line number Diff line number Diff line
@@ -22,12 +22,13 @@ import android.os.Message; 
import android.os.RemoteException;

import android.os.ServiceManager;

import android.util.Log;

import android.util.Slog;

import android.util.SparseArray;



import java.lang.ref.WeakReference;

import java.util.ArrayList;

import java.util.Iterator;

import java.util.LinkedList;

import java.util.List;

import java.util.concurrent.atomic.AtomicInteger;



/**

 * StorageManager is the interface to the systems storage service. The storage
@@ -69,7 +70,12 @@ public class StorageManager 
    /*

     * List of our listeners

     */

    private ArrayList<ListenerDelegate> mListeners = new ArrayList<ListenerDelegate>();

    private List<ListenerDelegate> mListeners = new ArrayList<ListenerDelegate>();



    /*

     * Next available nonce

     */

    final private AtomicInteger mNextNonce = new AtomicInteger(0);



    private class MountServiceBinderListener extends IMountServiceListener.Stub {

        public void onUsbMassStorageConnectionChanged(boolean available) {
@@ -93,55 +99,36 @@ public class StorageManager 
    private final ObbActionListener mObbActionListener = new ObbActionListener();



    private class ObbActionListener extends IObbActionListener.Stub {

        private List<WeakReference<ObbListenerDelegate>> mListeners = new LinkedList<WeakReference<ObbListenerDelegate>>();

        private SparseArray<ObbListenerDelegate> mListeners = new SparseArray<ObbListenerDelegate>();



        @Override

        public void onObbResult(String filename, String status) throws RemoteException {

        public void onObbResult(String filename, int nonce, int status) throws RemoteException {

            final ObbListenerDelegate delegate;

            synchronized (mListeners) {

                final Iterator<WeakReference<ObbListenerDelegate>> iter = mListeners.iterator();

                while (iter.hasNext()) {

                    final WeakReference<ObbListenerDelegate> ref = iter.next();



                    final ObbListenerDelegate delegate = (ref == null) ? null : ref.get();

                    if (delegate == null) {

                        iter.remove();

                        continue;

                delegate = mListeners.get(nonce);

                if (delegate != null) {

                    mListeners.remove(nonce);

                }

            }



            if (delegate != null) {

                delegate.sendObbStateChanged(filename, status);

            }

        }

        }



        public void addListener(OnObbStateChangeListener listener) {

            if (listener == null) {

                return;

            }

        public int addListener(OnObbStateChangeListener listener) {

            final ObbListenerDelegate delegate = new ObbListenerDelegate(listener);



            synchronized (mListeners) {

                final Iterator<WeakReference<ObbListenerDelegate>> iter = mListeners.iterator();

                while (iter.hasNext()) {

                    final WeakReference<ObbListenerDelegate> ref = iter.next();



                    final ObbListenerDelegate delegate = (ref == null) ? null : ref.get();

                    if (delegate == null) {

                        iter.remove();

                        continue;

                mListeners.put(delegate.nonce, delegate);

            }



                    /*

                     * If we're already in the listeners, we don't need to be in

                     * there again.

                     */

                    if (listener.equals(delegate.getListener())) {

                        return;

            return delegate.nonce;

        }

    }



                final ObbListenerDelegate delegate = new ObbListenerDelegate(listener);

                mListeners.add(new WeakReference<ObbListenerDelegate>(delegate));

            }

        }

    private int getNextNonce() {

        return mNextNonce.getAndIncrement();

    }



    /**
@@ -151,7 +138,10 @@ public class StorageManager 
        private final WeakReference<OnObbStateChangeListener> mObbEventListenerRef;

        private final Handler mHandler;



        private final int nonce;



        ObbListenerDelegate(OnObbStateChangeListener listener) {

            nonce = getNextNonce();

            mObbEventListenerRef = new WeakReference<OnObbStateChangeListener>(listener);

            mHandler = new Handler(mTgtLooper) {

                @Override
@@ -180,7 +170,7 @@ public class StorageManager 
            return mObbEventListenerRef.get();

        }



        void sendObbStateChanged(String path, String state) {

        void sendObbStateChanged(String path, int state) {

            ObbStateChangedStorageEvent e = new ObbStateChangedStorageEvent(path, state);

            mHandler.sendMessage(e.getMessage());

        }
@@ -191,9 +181,10 @@ public class StorageManager 
     */

    private class ObbStateChangedStorageEvent extends StorageEvent {

        public final String path;

        public final String state;



        public ObbStateChangedStorageEvent(String path, String state) {

        public final int state;



        public ObbStateChangedStorageEvent(String path, int state) {

            super(EVENT_OBB_STATE_CHANGED);

            this.path = path;

            this.state = state;
@@ -420,10 +411,8 @@ public class StorageManager 
     * <p>

     * The OBB will remain mounted for as long as the StorageManager reference

     * is held by the application. As soon as this reference is lost, the OBBs

     * in use will be unmounted. The {@link OnObbStateChangeListener} registered with

     * this call will receive all further OBB-related events until it goes out

     * of scope. If the caller is not interested in whether the call succeeds,

     * the <code>listener</code> may be specified as <code>null</code>.

     * in use will be unmounted. The {@link OnObbStateChangeListener} registered

     * with this call will receive the success or failure of this operation.

     * <p>

     * <em>Note:</em> you can only mount OBB files for which the OBB tag on the

     * file matches a package ID that is owned by the calling program's UID.
@@ -433,12 +422,21 @@ public class StorageManager 
     * @param filename the path to the OBB file

     * @param key secret used to encrypt the OBB; may be <code>null</code> if no

     *            encryption was used on the OBB.

     * @param listener will receive the success or failure of the operation

     * @return whether the mount call was successfully queued or not

     */

    public boolean mountObb(String filename, String key, OnObbStateChangeListener listener) {

        if (filename == null) {

            throw new IllegalArgumentException("filename cannot be null");

        }



        if (listener == null) {

            throw new IllegalArgumentException("listener cannot be null");

        }



        try {

            mObbActionListener.addListener(listener);

            mMountService.mountObb(filename, key, mObbActionListener);

            final int nonce = mObbActionListener.addListener(listener);

            mMountService.mountObb(filename, key, mObbActionListener, nonce);

            return true;

        } catch (RemoteException e) {

            Log.e(TAG, "Failed to mount OBB", e);
@@ -452,10 +450,8 @@ public class StorageManager 
     * <code>force</code> flag is true, it will kill any application needed to

     * unmount the given OBB (even the calling application).

     * <p>

     * The {@link OnObbStateChangeListener} registered with this call will receive all

     * further OBB-related events until it goes out of scope. If the caller is

     * not interested in whether the call succeeded, the listener may be

     * specified as <code>null</code>.

     * The {@link OnObbStateChangeListener} registered with this call will

     * receive the success or failure of this operation.

     * <p>

     * <em>Note:</em> you can only mount OBB files for which the OBB tag on the

     * file matches a package ID that is owned by the calling program's UID.
@@ -466,12 +462,21 @@ public class StorageManager 
     * @param filename path to the OBB file

     * @param force whether to kill any programs using this in order to unmount

     *            it

     * @param listener will receive the success or failure of the operation

     * @return whether the unmount call was successfully queued or not

     */

    public boolean unmountObb(String filename, boolean force, OnObbStateChangeListener listener) {

        if (filename == null) {

            throw new IllegalArgumentException("filename cannot be null");

        }



        if (listener == null) {

            throw new IllegalArgumentException("listener cannot be null");

        }



        try {

            mObbActionListener.addListener(listener);

            mMountService.unmountObb(filename, force, mObbActionListener);

            final int nonce = mObbActionListener.addListener(listener);

            mMountService.unmountObb(filename, force, mObbActionListener, nonce);

            return true;

        } catch (RemoteException e) {

            Log.e(TAG, "Failed to mount OBB", e);
@@ -486,7 +491,11 @@ public class StorageManager 
     * @param filename path to OBB image

     * @return true if OBB is mounted; false if not mounted or on error

     */

    public boolean isObbMounted(String filename) throws IllegalArgumentException {

    public boolean isObbMounted(String filename) {

        if (filename == null) {

            throw new IllegalArgumentException("filename cannot be null");

        }



        try {

            return mMountService.isObbMounted(filename);

        } catch (RemoteException e) {
@@ -506,12 +515,14 @@ public class StorageManager 
     *         not mounted or exception encountered trying to read status

     */

    public String getMountedObbPath(String filename) {

        if (filename == null) {

            throw new IllegalArgumentException("filename cannot be null");

        }



        try {

            return mMountService.getMountedObbPath(filename);

        } catch (RemoteException e) {

            Log.e(TAG, "Failed to find mounted path for OBB", e);

        } catch (IllegalArgumentException e) {

            Log.d(TAG, "Couldn't read OBB file", e);

        }



        return null;