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

Commit 19b71275 authored by Amy's avatar Amy
Browse files

Fix issues in register/unregister client API implementation

Issues include:
1. clientId array size checking too hard
2. missed use case validation
3. missed in use resources update when unregistering client(same in on
binder dead, will be in separate CL)
4. Calling PID should have only be used when tvinputsession id is null
5. Adding resources list to client profile

Test: atest TunerResourceManagerServiceTest
Bug: 147380513
Change-Id: I09b4105659a491b3c07e3b737fe0617b83d24447
parent 2f62bf64
Loading
Loading
Loading
Loading
+32 −0
Original line number Diff line number Diff line
@@ -15,6 +15,9 @@
 */
package com.android.server.tv.tunerresourcemanager;

import java.util.ArrayList;
import java.util.List;

/**
  * A client profile object used by the Tuner Resource Manager to record the registered clients'
  * information.
@@ -59,6 +62,11 @@ public final class ClientProfile {
     */
    private int mNiceValue;

    /**
     * List of the frontend ids that are used by the current client.
     */
    private List<Integer> mUsingFrontendIds = new ArrayList<>();

    /**
     * Optional arbitrary priority value given by the client.
     *
@@ -114,6 +122,30 @@ public final class ClientProfile {
        mNiceValue = niceValue;
    }

    /**
     * Set when the client starts to use a frontend.
     *
     * @param frontendId being used.
     */
    public void useFrontend(int frontendId) {
        mUsingFrontendIds.add(frontendId);
    }

    public List<Integer> getInUseFrontendIds() {
        return mUsingFrontendIds;
    }

    /**
     * Called when the client released a frontend.
     *
     * <p>This could happen when client resource reclaimed.
     *
     * @param frontendId being released.
     */
    public void releaseFrontend(int frontendId) {
        mUsingFrontendIds.remove(frontendId);
    }

    @Override
    public String toString() {
        return "ClientProfile[id=" + this.mId + ", tvInputSessionId=" + this.mTvInputSessionId
+56 −31
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import android.media.tv.tunerresourcemanager.TunerFrontendInfo;
import android.media.tv.tunerresourcemanager.TunerFrontendRequest;
import android.media.tv.tunerresourcemanager.TunerLnbRequest;
import android.media.tv.tunerresourcemanager.TunerResourceManager;
import android.os.Binder;
import android.os.RemoteException;
import android.util.Log;
import android.util.Slog;
@@ -54,7 +55,7 @@ public class TunerResourceManagerService extends SystemService {
    // Array of the registered client profiles
    @VisibleForTesting private SparseArray<ClientProfile> mClientProfiles = new SparseArray<>();
    private int mNextUnusedClientId = 0;
    private List<Integer> mReleasedClientId = new ArrayList<Integer>();
    private List<Integer> mRegisteredClientIds = new ArrayList<Integer>();

    // Array of the current available frontend resources
    @VisibleForTesting
@@ -98,8 +99,12 @@ public class TunerResourceManagerService extends SystemService {
                throw new RemoteException("ResourceClientProfile can't be null");
            }

            if (clientId == null || clientId.length != 1) {
                throw new RemoteException("clientId must be a size 1 array!");
            if (clientId == null) {
                throw new RemoteException("clientId can't be null!");
            }

            if (!mPriorityCongfig.isDefinedUseCase(profile.getUseCase())) {
                throw new RemoteException("Use undefined client use case:" + profile.getUseCase());
            }

            synchronized (mLock) {
@@ -108,20 +113,20 @@ public class TunerResourceManagerService extends SystemService {
        }

        @Override
        public void unregisterClientProfile(int clientId) {
            if (DEBUG) {
                Slog.d(TAG, "unregisterClientProfile(clientId=" + clientId + ")");
        public void unregisterClientProfile(int clientId) throws RemoteException {
            enforceAccessPermission();
            synchronized (mLock) {
                if (!checkClientExists(clientId)) {
                    Slog.e(TAG, "Unregistering non exists client:" + clientId);
                    return;
                }
                unregisterClientProfileInternal(clientId);
            }

            mClientProfiles.remove(clientId);
            mListeners.remove(clientId);
            mReleasedClientId.add(clientId);
        }

        @Override
        public boolean updateClientPriority(int clientId, int priority, int niceValue) {
            enforceAccessPermission();

            synchronized (mLock) {
                return updateClientPriorityInternal(clientId, priority, niceValue);
            }
@@ -243,24 +248,38 @@ public class TunerResourceManagerService extends SystemService {
            return;
        }
        // TODO tell if the client already exists
        if (mReleasedClientId.isEmpty()) {
        clientId[0] = mNextUnusedClientId++;
        } else {
            clientId[0] = mReleasedClientId.get(0);
            mReleasedClientId.remove(0);
        }

        int callingPid = mManager.getClientPid(profile.getTvInputSessionId());
        int pid = profile.getTvInputSessionId() == null
                ? Binder.getCallingPid() /*callingPid*/
                : mManager.getClientPid(profile.getTvInputSessionId()); /*tvAppId*/

        ClientProfile clientProfile = new ClientProfile.Builder(clientId[0])
                                              .tvInputSessionId(profile.getTvInputSessionId())
                                              .useCase(profile.getUseCase())
                                              .processId(callingPid)
                                              .processId(pid)
                                              .build();
        clientProfile.setPriority(getClientPriority(profile.getUseCase(), callingPid));
        clientProfile.setPriority(getClientPriority(profile.getUseCase(), pid));

        mClientProfiles.append(clientId[0], clientProfile);
        mListeners.append(clientId[0], listener);
        mRegisteredClientIds.add(clientId[0]);
    }

    @VisibleForTesting
    protected void unregisterClientProfileInternal(int clientId) {
        if (DEBUG) {
            Slog.d(TAG, "unregisterClientProfile(clientId=" + clientId + ")");
        }
        for (int id : getClientProfile(clientId).getInUseFrontendIds()) {
            getFrontendResource(id).removeOwner();
            for (int groupMemberId : getFrontendResource(id).getExclusiveGroupMemberFeIds()) {
                getFrontendResource(groupMemberId).removeOwner();
            }
        }
        mClientProfiles.remove(clientId);
        mListeners.remove(clientId);
        mRegisteredClientIds.remove(clientId);
    }

    @VisibleForTesting
@@ -355,12 +374,11 @@ public class TunerResourceManagerService extends SystemService {
        }

        frontendId[0] = TunerResourceManager.INVALID_FRONTEND_ID;
        ClientProfile requestClient = getClientProfile(request.getClientId());
        if (requestClient == null) {
            Slog.e(TAG, "Request from unregistered client. Id: " + request.getClientId());
        if (!checkClientExists(request.getClientId())) {
            Slog.e(TAG, "Request frontend from unregistered client:" + request.getClientId());
            return false;
        }

        ClientProfile requestClient = getClientProfile(request.getClientId());
        int grantingFrontendId = -1;
        int inUseLowestPriorityFrId = -1;
        // Priority max value is 1000
@@ -393,7 +411,7 @@ public class TunerResourceManagerService extends SystemService {
        // Grant frontend when there is unused resource.
        if (grantingFrontendId > -1) {
            frontendId[0] = grantingFrontendId;
            updateFrontendResourcesOnNewGrant(frontendId[0], request.getClientId());
            updateFrontendClientMappingOnNewGrant(frontendId[0], request.getClientId());
            return true;
        }

@@ -402,7 +420,7 @@ public class TunerResourceManagerService extends SystemService {
        if (inUseLowestPriorityFrId > -1 && (requestClient.getPriority() > currentLowestPriority)) {
            frontendId[0] = inUseLowestPriorityFrId;
            reclaimFrontendResource(getFrontendResource(frontendId[0]).getOwnerClientId());
            updateFrontendResourcesOnNewGrant(frontendId[0], request.getClientId());
            updateFrontendClientMappingOnNewGrant(frontendId[0], request.getClientId());
            return true;
        }

@@ -410,20 +428,20 @@ public class TunerResourceManagerService extends SystemService {
    }

    @VisibleForTesting
    protected int getClientPriority(int useCase, int callingPid) {
    protected int getClientPriority(int useCase, int pid) {
        if (DEBUG) {
            Slog.d(TAG, "getClientPriority useCase=" + useCase
                    + ", calling Pid=" + callingPid + ")");
                    + ", pid=" + pid + ")");
        }

        if (isForeground(callingPid)) {
        if (isForeground(pid)) {
            return mPriorityCongfig.getForegroundPriority(useCase);
        }
        return mPriorityCongfig.getBackgroundPriority(useCase);
    }

    @VisibleForTesting
    protected boolean isForeground(int callingPid) {
    protected boolean isForeground(int pid) {
        // TODO: how to get fg/bg information from pid
        return true;
    }
@@ -439,11 +457,14 @@ public class TunerResourceManagerService extends SystemService {
        }
    }

    private void updateFrontendResourcesOnNewGrant(int grantingId, int ownerClientId) {
    private void updateFrontendClientMappingOnNewGrant(int grantingId, int ownerClientId) {
        FrontendResource grantingFrontend = getFrontendResource(grantingId);
        ClientProfile ownerProfile = getClientProfile(ownerClientId);
        grantingFrontend.setOwner(ownerClientId);
        ownerProfile.useFrontend(grantingId);
        for (int exclusiveGroupMember : grantingFrontend.getExclusiveGroupMemberFeIds()) {
            getFrontendResource(exclusiveGroupMember).setOwner(ownerClientId);
            ownerProfile.useFrontend(exclusiveGroupMember);
        }
    }

@@ -475,6 +496,10 @@ public class TunerResourceManagerService extends SystemService {
        return mFrontendResources;
    }

    private boolean checkClientExists(int clientId) {
        return mRegisteredClientIds.contains(clientId);
    }

    private void enforceAccessPermission() {
        getContext().enforceCallingOrSelfPermission(
                "android.permission.TUNER_RESOURCE_ACCESS", TAG);
+4 −2
Original line number Diff line number Diff line
@@ -95,8 +95,10 @@ public class UseCasePriorityHints {
            } catch (XmlPullParserException e) {
                Slog.e(TAG, "Unable to parse vendor file: " + file, e);
            }
        } else if (DEBUG) {
        } else {
            if (DEBUG) {
                Slog.i(TAG, "no vendor priority configuration available. Using default priority");
            }
            addNewUseCasePriority(TvInputService.PRIORITY_HINT_USE_CASE_TYPE_BACKGROUND, 180, 100);
            addNewUseCasePriority(TvInputService.PRIORITY_HINT_USE_CASE_TYPE_SCAN, 450, 200);
            addNewUseCasePriority(TvInputService.PRIORITY_HINT_USE_CASE_TYPE_PLAYBACK, 480, 300);
+42 −0
Original line number Diff line number Diff line
@@ -452,4 +452,46 @@ public class TunerResourceManagerServiceTest {
                .get(infos[1].getId()).getOwnerClientId()).isEqualTo(clientId1[0]);
        assertThat(mReclaimingId).isEqualTo(clientId0[0]);
    }

    @Test
    public void unregisterClientTest_usingFrontend() {
        // Register client
        ResourceClientProfile profile = new ResourceClientProfile("0" /*sessionId*/,
                TvInputService.PRIORITY_HINT_USE_CASE_TYPE_PLAYBACK);
        int[] clientId = new int[1];
        mTunerResourceManagerService.registerClientProfileInternal(
                profile, null /*listener*/, clientId);
        assertThat(clientId[0]).isNotEqualTo(TunerResourceManagerService.INVALID_CLIENT_ID);

        // Init frontend resources.
        TunerFrontendInfo[] infos = new TunerFrontendInfo[2];
        infos[0] =
                new TunerFrontendInfo(0 /*id*/, FrontendSettings.TYPE_DVBT, 1 /*exclusiveGroupId*/);
        infos[1] =
                new TunerFrontendInfo(1 /*id*/, FrontendSettings.TYPE_DVBS, 1 /*exclusiveGroupId*/);
        mTunerResourceManagerService.setFrontendInfoListInternal(infos);

        TunerFrontendRequest request =
                new TunerFrontendRequest(clientId[0] /*clientId*/, FrontendSettings.TYPE_DVBT);
        int[] frontendId = new int[1];
        try {
            assertThat(mTunerResourceManagerService.requestFrontendInternal(request, frontendId))
                    .isTrue();
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        }
        assertThat(frontendId[0]).isEqualTo(infos[0].getId());
        assertThat(mTunerResourceManagerService.getFrontendResources().get(infos[0].getId())
                .isInUse()).isTrue();
        assertThat(mTunerResourceManagerService.getFrontendResources().get(infos[1].getId())
                .isInUse()).isTrue();

        // Unregister client when using frontend
        mTunerResourceManagerService.unregisterClientProfileInternal(clientId[0]);
        assertThat(mTunerResourceManagerService.getFrontendResources().get(infos[0].getId())
                .isInUse()).isFalse();
        assertThat(mTunerResourceManagerService.getFrontendResources().get(infos[1].getId())
                .isInUse()).isFalse();

    }
}
+15 −4
Original line number Diff line number Diff line
@@ -58,17 +58,16 @@ public class UseCasePriorityHintsTest {
    @Before
    public void setUp() throws Exception {
        mPriorityHints = new UseCasePriorityHints();
    }

    @Test
    public void parseTest_parseSampleXml() {
        try {
            mPriorityHints.parseInternal(
                    new ByteArrayInputStream(mExampleXML.getBytes(StandardCharsets.UTF_8)));
        } catch (IOException | XmlPullParserException e) {
            Slog.e(TAG, "Error parse xml.", e);
        }
    }

    @Test
    public void parseTest_parseSampleXml() {
        // Pre-defined foreground
        assertThat(mPriorityHints.getForegroundPriority(
                TvInputService.PRIORITY_HINT_USE_CASE_TYPE_BACKGROUND)).isEqualTo(180);
@@ -97,4 +96,16 @@ public class UseCasePriorityHintsTest {
        assertThat(mPriorityHints.getForegroundPriority(1001)).isEqualTo(300);
        assertThat(mPriorityHints.getBackgroundPriority(1001)).isEqualTo(80);
    }

    @Test
    public void isDefinedUseCaseTest_invalidUseCase() {
        assertThat(mPriorityHints.isDefinedUseCase(1992)).isFalse();
    }

    @Test
    public void isDefinedUseCaseTest_validUseCase() {
        assertThat(mPriorityHints.isDefinedUseCase(1001)).isTrue();
        assertThat(mPriorityHints.isDefinedUseCase(
                TvInputService.PRIORITY_HINT_USE_CASE_TYPE_RECORD)).isTrue();
    }
}