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

Commit e001621f authored by Hyundo Moon's avatar Hyundo Moon Committed by Automerger Merge Worker
Browse files

Merge "Add bug numbers for TODOs in MediaRouter2 related classes" into rvc-dev am: 5c8f3b20

Original change: undetermined

Change-Id: Ib6810b65fe7dc144f419635523cd86349d8bcf5d
parents 9d813a07 5c8f3b20
Loading
Loading
Loading
Loading
+2 −3
Original line number Original line Diff line number Diff line
@@ -233,7 +233,6 @@ public abstract class MediaRoute2ProviderService extends Service {
        String sessionId = sessionInfo.getId();
        String sessionId = sessionInfo.getId();
        synchronized (mSessionLock) {
        synchronized (mSessionLock) {
            if (mSessionInfo.containsKey(sessionId)) {
            if (mSessionInfo.containsKey(sessionId)) {
                // TODO: Notify failure to the requester, and throw exception if needed.
                Log.w(TAG, "Ignoring duplicate session id.");
                Log.w(TAG, "Ignoring duplicate session id.");
                return;
                return;
            }
            }
@@ -244,7 +243,7 @@ public abstract class MediaRoute2ProviderService extends Service {
            return;
            return;
        }
        }
        try {
        try {
            // TODO: Calling binder calls in multiple thread may cause timing issue.
            // TODO(b/157873487): Calling binder calls in multiple thread may cause timing issue.
            //       Consider to change implementations to avoid the problems.
            //       Consider to change implementations to avoid the problems.
            //       For example, post binder calls, always send all sessions at once, etc.
            //       For example, post binder calls, always send all sessions at once, etc.
            mRemoteCallback.notifySessionCreated(requestId, sessionInfo);
            mRemoteCallback.notifySessionCreated(requestId, sessionInfo);
@@ -519,7 +518,7 @@ public abstract class MediaRoute2ProviderService extends Service {
                    requestCreateSession));
                    requestCreateSession));
        }
        }


        //TODO: Ignore requests with unknown session ID.
        //TODO(b/157873546): Ignore requests with unknown session ID. -> For all similar commands.
        @Override
        @Override
        public void selectRoute(long requestId, String sessionId, String routeId) {
        public void selectRoute(long requestId, String sessionId, String routeId) {
            if (!checkCallerisSystem()) {
            if (!checkCallerisSystem()) {
+4 −4
Original line number Original line Diff line number Diff line
@@ -54,7 +54,7 @@ import java.util.stream.Collectors;
 * Media Router 2 allows applications to control the routing of media channels
 * Media Router 2 allows applications to control the routing of media channels
 * and streams from the current device to remote speakers and devices.
 * and streams from the current device to remote speakers and devices.
 */
 */
// TODO: Add method names at the beginning of log messages. (e.g. updateControllerOnHandler)
// TODO(b/157873330): Add method names at the beginning of log messages. (e.g. selectRoute)
//       Not only MediaRouter2, but also to service / manager / provider.
//       Not only MediaRouter2, but also to service / manager / provider.
// TODO: ensure thread-safe and document it
// TODO: ensure thread-safe and document it
public final class MediaRouter2 {
public final class MediaRouter2 {
@@ -399,7 +399,7 @@ public final class MediaRouter2 {
        Objects.requireNonNull(controller, "controller must not be null");
        Objects.requireNonNull(controller, "controller must not be null");
        Objects.requireNonNull(route, "route must not be null");
        Objects.requireNonNull(route, "route must not be null");


        // TODO: Check thread-safety
        // TODO(b/157873496): Check thread-safety, at least check "sRouterLock" for every variable
        if (!mRoutes.containsKey(route.getId())) {
        if (!mRoutes.containsKey(route.getId())) {
            notifyTransferFailure(route);
            notifyTransferFailure(route);
            return;
            return;
@@ -501,7 +501,7 @@ public final class MediaRouter2 {
    }
    }


    void addRoutesOnHandler(List<MediaRoute2Info> routes) {
    void addRoutesOnHandler(List<MediaRoute2Info> routes) {
        // TODO: When onRoutesAdded is first called,
        // TODO(b/157874065): When onRoutesAdded is first called,
        //  1) clear mRoutes before adding the routes
        //  1) clear mRoutes before adding the routes
        //  2) Call onRouteSelected(system_route, reason_fallback) if previously selected route
        //  2) Call onRouteSelected(system_route, reason_fallback) if previously selected route
        //     does not exist anymore. => We may need 'boolean MediaRoute2Info#isSystemRoute()'.
        //     does not exist anymore. => We may need 'boolean MediaRoute2Info#isSystemRoute()'.
@@ -1214,7 +1214,7 @@ public final class MediaRouter2 {
         * Any operations on this controller after calling this method will be ignored.
         * Any operations on this controller after calling this method will be ignored.
         * The devices that are playing media will stop playing it.
         * The devices that are playing media will stop playing it.
         */
         */
        // TODO: Add tests using {@link MediaRouter2Manager#getActiveSessions()}.
        // TODO(b/157872573): Add tests using {@link MediaRouter2Manager#getActiveSessions()}.
        public void release() {
        public void release() {
            releaseInternal(/* shouldReleaseSession= */ true, /* shouldNotifyStop= */ true);
            releaseInternal(/* shouldReleaseSession= */ true, /* shouldNotifyStop= */ true);
        }
        }
+2 −4
Original line number Original line Diff line number Diff line
@@ -151,8 +151,6 @@ public final class MediaRouter2Manager {
        return null;
        return null;
    }
    }


    //TODO: Use cache not to create array. For now, it's unclear when to purge the cache.
    //Do this when we finalize how to set control categories.
    /**
    /**
     * Gets available routes for an application.
     * Gets available routes for an application.
     *
     *
@@ -339,7 +337,7 @@ public final class MediaRouter2Manager {
        Objects.requireNonNull(sessionInfo, "sessionInfo must not be null");
        Objects.requireNonNull(sessionInfo, "sessionInfo must not be null");
        Objects.requireNonNull(route, "route must not be null");
        Objects.requireNonNull(route, "route must not be null");


        //TODO: Ignore unknown route.
        //TODO(b/157875504): Ignore unknown route.
        if (sessionInfo.getTransferableRoutes().contains(route.getId())) {
        if (sessionInfo.getTransferableRoutes().contains(route.getId())) {
            transferToRoute(sessionInfo, route);
            transferToRoute(sessionInfo, route);
            return;
            return;
@@ -355,7 +353,7 @@ public final class MediaRouter2Manager {
        if (client != null) {
        if (client != null) {
            try {
            try {
                int requestId = mNextRequestId.getAndIncrement();
                int requestId = mNextRequestId.getAndIncrement();
                //TODO: Ensure that every request is eventually removed.
                //TODO(b/157875723): Ensure that every request is eventually removed. (Memory leak)
                mTransferRequests.add(new TransferRequest(requestId, sessionInfo, route));
                mTransferRequests.add(new TransferRequest(requestId, sessionInfo, route));


                mMediaRouterService.requestCreateSessionWithManager(
                mMediaRouterService.requestCreateSessionWithManager(
+1 −2
Original line number Original line Diff line number Diff line
@@ -109,7 +109,7 @@ public class MediaRouter2ManagerTest {
        mContext = InstrumentationRegistry.getTargetContext();
        mContext = InstrumentationRegistry.getTargetContext();
        mManager = MediaRouter2Manager.getInstance(mContext);
        mManager = MediaRouter2Manager.getInstance(mContext);
        mRouter2 = MediaRouter2.getInstance(mContext);
        mRouter2 = MediaRouter2.getInstance(mContext);
        //TODO: If we need to support thread pool executors, change this to thread pool executor.
        // If we need to support thread pool executors, change this to thread pool executor.
        mExecutor = Executors.newSingleThreadExecutor();
        mExecutor = Executors.newSingleThreadExecutor();
        mPackageName = mContext.getPackageName();
        mPackageName = mContext.getPackageName();
    }
    }
@@ -253,7 +253,6 @@ public class MediaRouter2ManagerTest {
        CountDownLatch latch = new CountDownLatch(1);
        CountDownLatch latch = new CountDownLatch(1);


        addManagerCallback(new MediaRouter2Manager.Callback());
        addManagerCallback(new MediaRouter2Manager.Callback());
        //TODO: remove this when it's not necessary.
        addRouterCallback(new MediaRouter2.RouteCallback() {});
        addRouterCallback(new MediaRouter2.RouteCallback() {});
        addTransferCallback(new MediaRouter2.TransferCallback() {
        addTransferCallback(new MediaRouter2.TransferCallback() {
            @Override
            @Override
+0 −3
Original line number Original line Diff line number Diff line
@@ -44,7 +44,6 @@ import java.util.Objects;
/**
/**
 * Maintains a connection to a particular {@link MediaRoute2ProviderService}.
 * Maintains a connection to a particular {@link MediaRoute2ProviderService}.
 */
 */
// TODO: Need to revisit the bind/unbind/connect/disconnect logic in this class.
final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider
final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider
        implements ServiceConnection {
        implements ServiceConnection {
    private static final String TAG = "MR2ProviderSvcProxy";
    private static final String TAG = "MR2ProviderSvcProxy";
@@ -265,8 +264,6 @@ final class MediaRoute2ProviderServiceProxy extends MediaRoute2Provider
        if (DEBUG) {
        if (DEBUG) {
            Slog.d(TAG, this + ": Service binding died");
            Slog.d(TAG, this + ": Service binding died");
        }
        }
        // TODO: Investigate whether it tries to bind endlessly when the service is
        //       badly implemented.
        if (shouldBind()) {
        if (shouldBind()) {
            unbind();
            unbind();
            bind();
            bind();
Loading