Video Player : Fixed media observer triggering logic

This CL cleans up the media session observer logic for triggering video player UI and metrics update.

Change-Id: Ib6dcb8173206f7c0f0a5d53ddfbb5bf17654b893
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465093
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820040}

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/PlaybackStateObserver.java

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.video_tutorials;
 
+import org.chromium.base.supplier.Supplier;
+import org.chromium.chrome.browser.video_tutorials.PlaybackStateObserver.WatchStateInfo.State;
 import org.chromium.content_public.browser.MediaSession;
 import org.chromium.content_public.browser.MediaSessionObserver;
 import org.chromium.content_public.browser.WebContents;
@@ -14,6 +16,41 @@ import org.chromium.services.media_session.MediaPosition;
  * events.
  */
 public class PlaybackStateObserver extends MediaSessionObserver {
+    /**
+     * A ratio used for collecting metrics used to determine whether a video was sufficiently
+     * watched by the user.
+     */
+    private static final float WATCH_COMPLETION_RATIO_THRESHOLD = 0.5f;
+
+    /** Contains playback info about currently playing media. */
+    public static class WatchStateInfo {
+        /** Contains various states during the media playback. */
+        public enum State {
+            INITIAL,
+            PLAYING,
+            PAUSED,
+            ENDED,
+            ERROR,
+        }
+
+        /** The current state. */
+        public State state;
+
+        /** The duration of the video. */
+        public long videoLength;
+
+        /** The current position of the video. */
+        public long currentPosition;
+
+        /**
+         * Whether the video has been watched up to a certain point so that it can be considered as
+         * completed.
+         */
+        public boolean videoWatched() {
+            return currentPosition > videoLength * WATCH_COMPLETION_RATIO_THRESHOLD;
+        }
+    }
+
     /**
      * Interface to be notified of playback state updates.
      */
@@ -26,38 +63,104 @@ public class PlaybackStateObserver extends MediaSessionObserver {
 
         /** Called when the player has completed playing the video. */
         void onEnded();
+
+        /** Called when an error has occurred. */
+        void onError();
     }
 
-    private final Observer mObserver;
-    private MediaPosition mMediaPosition;
+    private final Supplier<Observer> mObserver;
+    private long mLastUpdateTime;
+    private WatchStateInfo mWatchStateInfo = new WatchStateInfo();
+    private MediaPosition mLastPosition;
+    private boolean mIsControllable;
+    private boolean mIsSuspended;
 
     /** Constructor. */
-    public PlaybackStateObserver(WebContents webContents, Observer observer) {
+    public PlaybackStateObserver(WebContents webContents, Supplier<Observer> observer) {
         super(MediaSession.fromWebContents(webContents));
         mObserver = observer;
     }
 
+    /**
+     * Called to get the current media playback info, such as duration, current progress, playback
+     * state etc.
+     * @return The current watch state info.
+     */
+    public WatchStateInfo getWatchStateInfo() {
+        return mWatchStateInfo;
+    }
+
+    /** Reset internal state. */
+    public void reset() {
+        mLastPosition = null;
+        mIsControllable = false;
+        mIsSuspended = false;
+        mLastUpdateTime = 0;
+        mWatchStateInfo = new WatchStateInfo();
+    }
+
     @Override
     public void mediaSessionPositionChanged(MediaPosition position) {
-        if (position == null) return;
-        mMediaPosition = position;
+        updateState(position);
+        mWatchStateInfo.currentPosition =
+                computeCurrentTime(position == null ? mLastPosition : position, mLastUpdateTime);
+        mLastUpdateTime = System.currentTimeMillis();
+        mLastPosition = position;
     }
 
     @Override
     public void mediaSessionStateChanged(boolean isControllable, boolean isSuspended) {
-        boolean playerEnded = !isControllable && isSuspended && mMediaPosition != null
-                && mMediaPosition.getPosition() > 0.5 * mMediaPosition.getDuration();
-        boolean playerPaused = isControllable && isSuspended;
-        boolean isPlaying = isControllable && !isSuspended;
-        // TODO(shaktisahu): Fix these signals and logic in another CL.
-        if (isPlaying) {
-            mObserver.onPlay();
-        }
-        if (playerPaused) {
-            mObserver.onPause();
+        mIsControllable = isControllable;
+        mIsSuspended = isSuspended;
+    }
+
+    private void updateState(MediaPosition newPosition) {
+        State nextState = mWatchStateInfo.state;
+        if (mIsControllable) {
+            nextState = mIsSuspended ? State.PAUSED : State.PLAYING;
+        } else if (newPosition == null) {
+            // TODO(shaktisahu): Determine error state.
+            if (mLastPosition == null) {
+                nextState = State.INITIAL;
+            } else if (mLastPosition.getDuration()
+                    == computeCurrentTime(mLastPosition, mLastUpdateTime)) {
+                nextState = State.ENDED;
+            }
         }
-        if (playerEnded) {
-            mObserver.onEnded();
+
+        updateObservers(nextState);
+    }
+
+    private void updateObservers(State nextState) {
+        if (nextState == mWatchStateInfo.state) return;
+
+        mWatchStateInfo.state = nextState;
+        switch (nextState) {
+            case INITIAL:
+                break;
+            case PLAYING:
+                mObserver.get().onPlay();
+                break;
+            case PAUSED:
+                mObserver.get().onPause();
+                break;
+            case ENDED:
+                mObserver.get().onEnded();
+                break;
+            case ERROR:
+                mObserver.get().onError();
+                break;
+            default:
+                assert false : "Unknown media playback state";
         }
     }
+
+    private static long computeCurrentTime(MediaPosition mediaPosition, long lastUpdateTime) {
+        if (mediaPosition == null) return 0;
+        long elapsedTime = System.currentTimeMillis() - lastUpdateTime;
+        long updatedPosition = (long) (mediaPosition.getPosition()
+                + (elapsedTime * mediaPosition.getPlaybackRate()));
+        updatedPosition = Math.min(updatedPosition, mediaPosition.getDuration());
+        return updatedPosition;
+    }
 }

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/metrics/VideoTutorialMetrics.java

@@ -18,14 +18,16 @@ import java.lang.annotation.RetentionPolicy;
 public class VideoTutorialMetrics {
     // Please treat this list as append only and keep it in sync with
     // VideoTutorials.WatchState in enums.xml.
-    @IntDef({WatchState.STARTED, WatchState.COMPLETED, WatchState.PAUSED, WatchState.RESUMED})
+    @IntDef({WatchState.STARTED, WatchState.COMPLETED, WatchState.PAUSED, WatchState.RESUMED,
+            WatchState.WATCHED})
     @Retention(RetentionPolicy.SOURCE)
     public @interface WatchState {
         int STARTED = 0;
         int COMPLETED = 1;
         int PAUSED = 2;
         int RESUMED = 3;
-        int NUM_ENTRIES = 4;
+        int WATCHED = 4;
+        int NUM_ENTRIES = 5;
     }
 
     // Please treat this list as append only and keep it in sync with
@@ -60,7 +62,7 @@ public class VideoTutorialMetrics {
         String histogramSuffix = histogramSuffixFromFeatureType(feature);
         RecordHistogram.recordEnumeratedHistogram(
                 "VideoTutorials." + histogramSuffix + ".Player.Progress", state,
-                FeatureType.MAX_VALUE);
+                WatchState.NUM_ENTRIES);
     }
 
     private static String histogramSuffixFromFeatureType(@FeatureType int feature) {

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/player/VideoPlayerCoordinatorImpl.java

@@ -61,7 +61,8 @@ public class VideoPlayerCoordinatorImpl implements VideoPlayerCoordinator {
                 new LanguagePickerCoordinator(mView.getView().findViewById(R.id.language_picker),
                         mVideoTutorialService, languageInfoProvider);
         mMediator = new VideoPlayerMediator(mContext, mModel, videoTutorialService, mLanguagePicker,
-                languageInfoProvider, mWebContents, tryNowCallback, closeCallback);
+                languageInfoProvider, mWebContents, mMediaSessionObserver, tryNowCallback,
+                closeCallback);
         PropertyModelChangeProcessor.create(mModel, mView, new VideoPlayerViewBinder());
     }
 
@@ -94,7 +95,8 @@ public class VideoPlayerCoordinatorImpl implements VideoPlayerCoordinator {
         mWebContents = pair.first;
         ContentView webContentView = pair.second;
         mWebContentsDelegate = new WebContentsDelegateAndroid();
-        mMediaSessionObserver = new PlaybackStateObserver(mWebContents, mMediator);
+        mMediaSessionObserver =
+                new PlaybackStateObserver(mWebContents, () -> { return mMediator; });
 
         ThinWebView thinWebView = ThinWebViewFactory.create(mContext, new ThinWebViewConstraints());
         thinWebView.attachWebContents(mWebContents, webContentView, mWebContentsDelegate);

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/player/VideoPlayerMediator.java

@@ -36,13 +36,15 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
     private final Callback<Tutorial> mTryNowCallback;
     private final Runnable mCloseCallback;
     private final LanguageInfoProvider mLanguageInfoProvider;
+    private final PlaybackStateObserver mPlaybackStateObserver;
     private long mVideoStartTime;
 
     /** Constructor. */
     public VideoPlayerMediator(Context context, PropertyModel model,
             VideoTutorialService videoTutorialService, LanguagePickerCoordinator languagePicker,
             LanguageInfoProvider languageInfoProvider, WebContents webContents,
-            Callback<Tutorial> tryNowCallback, Runnable closeCallback) {
+            PlaybackStateObserver playbackStateObserver, Callback<Tutorial> tryNowCallback,
+            Runnable closeCallback) {
         mContext = context;
         mModel = model;
         mVideoTutorialService = videoTutorialService;
@@ -51,6 +53,7 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
         mWebContents = webContents;
         mTryNowCallback = tryNowCallback;
         mCloseCallback = closeCallback;
+        mPlaybackStateObserver = playbackStateObserver;
 
         mModel.set(VideoPlayerProperties.SHOW_LOADING_SCREEN, false);
         mModel.set(VideoPlayerProperties.SHOW_LANGUAGE_PICKER, false);
@@ -62,6 +65,13 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
         mModel.set(VideoPlayerProperties.CALLBACK_CLOSE, this::close);
     }
 
+    /** Called when the player is getting destroyed. */
+    public void destroy() {
+        if (mPlaybackStateObserver.getWatchStateInfo().videoWatched()) {
+            VideoTutorialMetrics.recordWatchStateUpdate(mTutorial.featureType, WatchState.WATCHED);
+        }
+    }
+
     boolean handleBackPressed() {
         boolean isShowingLanguagePicker = mModel.get(VideoPlayerProperties.SHOW_LANGUAGE_PICKER);
         boolean isShowingLoadingScreen = mModel.get(VideoPlayerProperties.SHOW_LOADING_SCREEN);
@@ -72,7 +82,7 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
                     mTutorial.featureType, UserAction.BACK_PRESS_WHEN_SHOWING_VIDEO_PLAYER);
         }
 
-        return true;
+        return false;
     }
 
     /**
@@ -125,6 +135,11 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
         updateChangeLanguageButtonText();
     }
 
+    @Override
+    public void onError() {
+        // TODO(shaktisahu): Determine UI for error state.
+    }
+
     private void changeLanguage() {
         mModel.set(VideoPlayerProperties.SHOW_LANGUAGE_PICKER, true);
         mLanguagePicker.showLanguagePicker(this::onLanguageSelected, () -> {} /* closeCallback */);
@@ -162,6 +177,7 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
     }
 
     private void startVideo(Tutorial tutorial) {
+        mPlaybackStateObserver.reset();
         VideoTutorialMetrics.recordWatchStateUpdate(mTutorial.featureType, WatchState.STARTED);
         mVideoStartTime = System.currentTimeMillis();
         mTutorial = tutorial;
@@ -180,6 +196,9 @@ class VideoPlayerMediator implements PlaybackStateObserver.Observer {
     }
 
     private void onWatchNextClicked() {
+        if (mPlaybackStateObserver.getWatchStateInfo().videoWatched()) {
+            VideoTutorialMetrics.recordWatchStateUpdate(mTutorial.featureType, WatchState.WATCHED);
+        }
         VideoTutorialMetrics.recordUserAction(mTutorial.featureType, UserAction.WATCH_NEXT_VIDEO);
         VideoTutorialUtils.getNextTutorial(mVideoTutorialService, mTutorial, this::startVideo);
     }

chrome/browser/video_tutorials/internal/android/java/src/org/chromium/chrome/browser/video_tutorials/player/VideoPlayerMediatorUnitTest.java

@@ -24,6 +24,7 @@ import org.chromium.base.Callback;
 import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.video_tutorials.LanguageInfoProvider;
+import org.chromium.chrome.browser.video_tutorials.PlaybackStateObserver;
 import org.chromium.chrome.browser.video_tutorials.Tutorial;
 import org.chromium.chrome.browser.video_tutorials.VideoTutorialUtils;
 import org.chromium.chrome.browser.video_tutorials.languages.LanguagePickerCoordinator;
@@ -63,6 +64,8 @@ public class VideoPlayerMediatorUnitTest {
     Callback<Tutorial> mTryNowCallback;
     @Mock
     private LanguageInfoProvider mLanguageProvider;
+    @Mock
+    PlaybackStateObserver mPlaybackStateObserver;
 
     @Before
     public void setUp() {
@@ -76,7 +79,8 @@ public class VideoPlayerMediatorUnitTest {
 
         mTestVideoTutorialService = new TestVideoTutorialService();
         mMediator = new VideoPlayerMediator(mContext, mModel, mTestVideoTutorialService,
-                mLanguagePicker, mLanguageProvider, mWebContents, mTryNowCallback, mCloseCallback);
+                mLanguagePicker, mLanguageProvider, mWebContents, mPlaybackStateObserver,
+                mTryNowCallback, mCloseCallback);
     }
 
     @Test

tools/metrics/histograms/enums.xml

@@ -74773,6 +74773,7 @@ Full version information for the fingerprint enum values:
   <int value="1" label="Completed"/>
   <int value="2" label="Paused"/>
   <int value="3" label="Resumed"/>
+  <int value="4" label="Watched"/>
 </enum>
 
 <enum name="ViewFileType">