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
(cherry picked from commit 19b71275e0)
This commit is contained in:
Amy
2020-02-20 20:02:42 -08:00
parent abdf7b6a57
commit 630fc01bf6
5 changed files with 149 additions and 37 deletions

View File

@@ -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

View File

@@ -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);
}
clientId[0] = mNextUnusedClientId++;
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);

View File

@@ -95,8 +95,10 @@ public class UseCasePriorityHints {
} catch (XmlPullParserException e) {
Slog.e(TAG, "Unable to parse vendor file: " + file, e);
}
} else if (DEBUG) {
Slog.i(TAG, "no vendor priority configuration available. Using default priority");
} 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);

View File

@@ -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();
}
}

View File

@@ -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();
}
}