MediaSession2: Clean up TODOs under frameworks/base/media

This CL includes following changes
  - Remove outdated TODOs
  - Added buganizer issue if the remaining work take more than 5m

Test: Run MediaComponents tests
Change-Id: I8968e12aabcbc67f69dbf14485b3716d0e95779b

diff --git a/media/java/android/media/MediaController2.java b/media/java/android/media/MediaController2.java
index 2ef2bb3..da2b8e0 100644 (file)
--- a/media/java/android/media/MediaController2.java
+++ b/media/java/android/media/MediaController2.java
@@ -139,13 +139,15 @@ public class MediaController2 implements AutoCloseable, MediaPlaylistController
         /**
          * Called when the playlist is changed.
          * <p>
-         * When it's called, you should invalidate previous playback information such as position,
-         * player state, current item, etc.
+         * If the previously playing media item is gone, you should invalidate previous playback
+         * information and wait for later callbacks.
          *
          * @param controller the controller for this event
          * @param playlist A new playlist set by the session.
+         * @see #onPositionChanged(long, long)
+         * @see #onBufferedPositionChanged(long)
+         * @see #onCurrentPlaylistItemChanged(MediaItem2)
          */
-        // TODO(jaewan): Enhance doc
         public void onPlaylistChanged(@NonNull MediaController2 controller,
                 @NonNull List<MediaItem2> playlist) { }
 
@@ -156,7 +158,7 @@ public class MediaController2 implements AutoCloseable, MediaPlaylistController
          * @param state latest playback state
          * @hide
          */
-        // TODo(jaewan): Remove
+        // TODO(jaewan): Remove (b/73971431)
         public void onPlaybackStateChanged(@NonNull MediaController2 controller,
                 @NonNull PlaybackState2 state) { }
 
@@ -208,9 +210,15 @@ public class MediaController2 implements AutoCloseable, MediaPlaylistController
 
         /**
          * Called when the player's current playing item is changed
+         * <p>
+         * When it's called, you should invalidate previous playback information and wait for later
+         * callbacks.
          *
          * @param controller the controller for this event
          * @param item new item
+         * @see #onPositionChanged(long, long)
+         * @see #onBufferedPositionChanged(long)
+         * @see #onCurrentPlaylistItemChanged(MediaItem2)
          */
         public void onCurrentPlaylistItemChanged(@NonNull MediaController2 controller,
                 @NonNull MediaItem2 item) { }
@@ -325,7 +333,6 @@ public class MediaController2 implements AutoCloseable, MediaPlaylistController
      * @param executor executor to run callbacks on.
      * @param callback controller callback to receive changes in
      */
-    // TODO(jaewan): Put @CallbackExecutor to the constructor.
     public MediaController2(@NonNull Context context, @NonNull SessionToken2 token,
             @NonNull @CallbackExecutor Executor executor, @NonNull ControllerCallback callback) {
         super();
@@ -656,7 +663,7 @@ public class MediaController2 implements AutoCloseable, MediaPlaylistController
      * Set the playback speed.
      */
     public void setPlaybackSpeed(float speed) {
-        // TODO: implement this
+        // TODO(jaewan): implement this (b/74093080)
     }
 
     /**

diff --git a/media/java/android/media/MediaItem2.java b/media/java/android/media/MediaItem2.java
index f9eceab..f6edd0c 100644 (file)
--- a/media/java/android/media/MediaItem2.java
+++ b/media/java/android/media/MediaItem2.java
@@ -70,7 +70,6 @@ public class MediaItem2 {
      * @return a new bundle instance
      */
     public Bundle toBundle() {
-        // TODO(jaewan): Fill here when we rebase.
         return mProvider.toBundle_impl();
     }
 

diff --git a/media/java/android/media/MediaMetadata2.java b/media/java/android/media/MediaMetadata2.java
index 07367bb..502f929 100644 (file)
--- a/media/java/android/media/MediaMetadata2.java
+++ b/media/java/android/media/MediaMetadata2.java
@@ -37,7 +37,6 @@ import java.util.Set;
 //   - Don't implement Parcelable for updatable support.
 //   - Also support MediaDescription features. MediaDescription is deprecated instead because
 //     it was insufficient for controller to display media contents.
-// TODO(jaewan): Add @see for APIs from MediaDescription
 public final class MediaMetadata2 {
     /**
      * The metadata key for a {@link CharSequence} or {@link String} typed value to retrieve the

diff --git a/media/java/android/media/MediaSession2.java b/media/java/android/media/MediaSession2.java
index bbc4299..d94be66 100644 (file)
--- a/media/java/android/media/MediaSession2.java
+++ b/media/java/android/media/MediaSession2.java
@@ -81,10 +81,6 @@ import java.util.concurrent.Executor;
 public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
     private final MediaSession2Provider mProvider;
 
-    // TODO(jaewan): Should we define IntDef? Currently we don't have to allow subclass to add more.
-    // TODO(jaewan): Shouldn't we pull out?
-    // TODO(jaewan): Should we also protect getters not related with metadata?
-    //               Getters are getPlaybackState(), getSessionActivity(), getPlaylistParams()
     // Next ID: 23
     /**
      * Command code for the custom command which can be defined by string action in the
@@ -338,7 +334,6 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * If {@link #getCommandCode()} is {@link #COMMAND_CODE_CUSTOM}), it's custom command and
      * {@link #getCustomCommand()} shouldn't be {@code null}.
      */
-    // TODO(jaewan): Move this into the updatable.
     public static final class Command {
         private final CommandProvider mProvider;
 
@@ -434,8 +429,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
         }
 
         public List<Command> getCommands() {
-            // TODO: implement this
-            return null;
+            return mProvider.getCommands_impl();
         }
 
         /**
@@ -469,7 +463,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * If it's not set, the session will accept all controllers and all incoming commands by
      * default.
      */
-    // TODO(jaewan): Can we move this inside of the updatable for default implementation.
+    // TODO(jaewan): Move this to updatable for default implementation (b/74091963)
     public static abstract class SessionCallback {
         private final Context mContext;
 
@@ -489,7 +483,6 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
          * @param controller controller information.
          * @return allowed commands. Can be {@code null} to reject coonnection.
          */
-        // TODO(jaewan): Change return type. Once we do, null is for reject.
         public @Nullable CommandGroup onConnect(@NonNull MediaSession2 session,
                 @NonNull ControllerInfo controller) {
             CommandGroup commands = new CommandGroup(mContext);
@@ -716,7 +709,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
          * @param player a {@link MediaPlayerBase} that handles actual media playback in your app.
          */
         U setPlayer(@NonNull MediaPlayerBase player) {
-            // TODO: Change the provider properly
+            // TODO(jaewan): Change the provider properly (b/74093082)
             mProvider.setPlayer_impl(player, null, null);
             return (U) this;
         }
@@ -729,7 +722,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
          * {@code player.}
          */
         U setPlaylistController(@NonNull MediaPlaylistController mplc) {
-            // TODO: implement this
+            // TODO(jaewan): implement this (b/74093082)
             return (U) this;
         }
 
@@ -740,7 +733,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
          * @param volumeProvider The provider that will receive volume button events.
          */
         U setVolumeProvider(@NonNull VolumeProvider2 volumeProvider) {
-            // TODO: implement this
+            // TODO(jaewan): implement this (b/74093082)
             return (U) this;
         }
 
@@ -865,7 +858,6 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
         /**
          * @hide
          */
-        // TODO(jaewan): Also accept componentName to check notificaiton listener.
         public ControllerInfo(Context context, int uid, int pid, String packageName,
                 IInterface callback) {
             mProvider = ApiLoader.getProvider(context)
@@ -912,18 +904,12 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
 
         @Override
         public boolean equals(Object obj) {
-            if (!(obj instanceof ControllerInfo)) {
-                return false;
-            }
-            ControllerInfo other = (ControllerInfo) obj;
-            return mProvider.equals_impl(other.mProvider);
+            return mProvider.equals_impl(obj);
         }
 
         @Override
         public String toString() {
-            // TODO(jaewan): Move this to updatable.
-            return "ControllerInfo {pkg=" + getPackageName() + ", uid=" + getUid() + ", trusted="
-                    + isTrusted() + "}";
+            return mProvider.toString_impl();
         }
     }
 
@@ -1193,14 +1179,13 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      *
      * @param player a {@link MediaPlayerBase} that handles actual media playback in your app.
      * @param mplc a {@link MediaPlaylistController} that manages playlist of the
-     * {@code player.}
+     * {@code player}
      * @param volumeProvider The provider that will receive volume button events. If
      * {@code null}, system will adjust the appropriate stream volume for this session's player.
      */
     public void updatePlayer(@NonNull MediaPlayerBase player,
-            @Nullable MediaPlaylistController mplc, @NonNull VolumeProvider2 volumeProvider) {
-        // TODO: rename setPlayer_impl to updatePlayer_impl
-        mProvider.setPlayer_impl(player, mplc, volumeProvider);
+            @Nullable MediaPlaylistController mplc, @Nullable VolumeProvider2 volumeProvider) {
+        mProvider.updatePlayer_impl(player, mplc, volumeProvider);
     }
 
     @Override
@@ -1219,19 +1204,16 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
     /**
      * @return playlist controller
      */
-    public @Nullable
-    MediaPlaylistController getMediaPlaylistController() {
-        // TODO: implement this
+    public @Nullable MediaPlaylistController getMediaPlaylistController() {
+        // TODO(jaewan): implement this (b/74090741)
         return null;
     }
 
     /**
      * @return volume provider
      */
-    public @Nullable
-    VolumeProvider2 getVolumeProvider() {
-     // TODO: implement this
-        return null;
+    public @Nullable VolumeProvider2 getVolumeProvider() {
+        return mProvider.getVolumeProvider_impl();
     }
 
     /**
@@ -1252,7 +1234,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * @param afr the full request parameters
      */
     public void setAudioFocusRequest(AudioFocusRequest afr) {
-        // TODO: implement this
+        // TODO(jaewan): implement this (b/72529899)
         // mProvider.setAudioFocusRequest_impl(focusGain);
     }
 
@@ -1426,8 +1408,6 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      *
      * @throws IllegalArgumentException if the play list is null
      */
-    // TODO(jaewan): Remove with index was previously rejected by council (b/36524925)
-    // TODO(jaewan): Should we also add movePlaylistItem from index to index?
     public void removePlaylistItem(MediaItem2 item) {
         mProvider.removePlaylistItem_impl(item);
     }
@@ -1518,7 +1498,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * @throws IllegalArgumentException if executor or callback is {@code null}.
      * @hide
      */
-    // TODO(jaewan): Unhide or remove
+    // TODO(jaewan): Remove (b/74157064)
     public void registerPlayerEventCallback(@NonNull @CallbackExecutor Executor executor,
             @NonNull PlayerEventCallback callback) {
         mProvider.registerPlayerEventCallback_impl(executor, callback);
@@ -1531,7 +1511,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * @throws IllegalArgumentException if the callback is {@code null}.
      * @hide
      */
-    // TODO(jaewan): Unhide or remove
+    // TODO(jaewan): Remove (b/74157064)
     public void unregisterPlayerEventCallback(@NonNull PlayerEventCallback callback) {
         mProvider.unregisterPlayerEventCallback_impl(callback);
     }
@@ -1552,7 +1532,7 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * @return speed
      */
     public float getPlaybackSpeed() {
-        // TODO: implement this
+        // TODO(jaewan): implement this (b/74093080)
         return -1;
     }
 
@@ -1560,6 +1540,6 @@ public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
      * Set the playback speed.
      */
     public void setPlaybackSpeed(float speed) {
-        // TODO: implement this
+        // TODO(jaewan): implement this (b/74093080)
     }
 }

diff --git a/media/java/android/media/PlaybackState2.java b/media/java/android/media/PlaybackState2.java
index 7afb579..afc2bfa 100644 (file)
--- a/media/java/android/media/PlaybackState2.java
+++ b/media/java/android/media/PlaybackState2.java
@@ -33,7 +33,7 @@ import java.lang.annotation.RetentionPolicy;
  * the current playback position and extra.
  * @hide
  */
-// TODO(jaewan): Remove this.
+// TODO(jaewan): Remove this (b/73971431)
 public final class PlaybackState2 {
     // Similar to the PlaybackState with following changes
     //    - Not implement Parcelable and added from/toBundle()
@@ -76,7 +76,6 @@ public final class PlaybackState2 {
     //    - Removed actions and custom actions.
     //    - Removed error string
     //    - Repeat mode / shuffle mode is now in the PlaylistParams
-    // TODO(jaewan): Replace states from MediaPlayer2
     /**
      * @hide
      */

diff --git a/media/java/android/media/update/MediaSession2Provider.java b/media/java/android/media/update/MediaSession2Provider.java
index d0ec104..f97a6f0 100644 (file)
--- a/media/java/android/media/update/MediaSession2Provider.java
+++ b/media/java/android/media/update/MediaSession2Provider.java
@@ -44,9 +44,10 @@ import java.util.concurrent.Executor;
  */
 public interface MediaSession2Provider extends TransportControlProvider {
     void close_impl();
-    void setPlayer_impl(MediaPlayerBase player, MediaPlaylistController mplc,
+    void updatePlayer_impl(MediaPlayerBase player, MediaPlaylistController mplc,
             VolumeProvider2 volumeProvider);
     MediaPlayerBase getPlayer_impl();
+    VolumeProvider2 getVolumeProvider_impl();
     SessionToken2 getToken_impl();
     List<ControllerInfo> getConnectedControllers_impl();
     void setCustomLayout_impl(ControllerInfo controller, List<CommandButton> layout);
@@ -84,6 +85,7 @@ public interface MediaSession2Provider extends TransportControlProvider {
         void removeCommand_impl(Command command);
         boolean hasCommand_impl(Command command);
         boolean hasCommand_impl(int code);
+        List<Command> getCommands_impl();
         Bundle toBundle_impl();
     }
 
@@ -109,7 +111,8 @@ public interface MediaSession2Provider extends TransportControlProvider {
         int getUid_impl();
         boolean isTrusted_impl();
         int hashCode_impl();
-        boolean equals_impl(ControllerInfoProvider obj);
+        boolean equals_impl(Object obj);
+        String toString_impl();
     }
 
     interface PlaylistParamsProvider {