Fix some voicemail playback bugs

* Don't fetch voicemail until playback is started
* Prevent multiple voicemail fetches for the same voicemail
* Fixed an issue where the seekbar would be stuck in the same
position even after seeking
* Fix for playback position getting reset after the voicemail
view is scrolled out of view and back in
* Respect the invariant that if mMediaPlayer is null, mPrepared
is always false. This fixes potential NPEs when resuming playback.

Bug: 22127956
Bug: 22333494

Change-Id: I5c419424950c0e21317cbd133ca6f7e1edd9fd31

2 files changed, 50 insertions, 26 deletions

diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java b/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
index 2d65504e8..2017bc578 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
@@ -75,6 +75,21 @@ public class VoicemailPlaybackLayout extends LinearLayout
         private final Object mLock = new Object();
         @GuardedBy("mLock") private ScheduledFuture<?> mScheduledFuture;
 
+        private Runnable mUpdateClipPositionRunnable = new Runnable() {
+            @Override
+            public void run() {
+                int currentPositionMs = 0;
+                synchronized (mLock) {
+                    if (mScheduledFuture == null || mPresenter == null) {
+                        // This task has been canceled. Just stop now.
+                        return;
+                    }
+                    currentPositionMs = mPresenter.getMediaPlayerPosition();
+                }
+                setClipPosition(currentPositionMs, mDurationMs);
+            }
+        };
+
         public PositionUpdater(int durationMs, ScheduledExecutorService executorService) {
             mDurationMs = durationMs;
             mExecutorService = executorService;
@@ -82,28 +97,12 @@ public class VoicemailPlaybackLayout extends LinearLayout
 
         @Override
         public void run() {
-            post(new Runnable() {
-                @Override
-                public void run() {
-                    int currentPositionMs = 0;
-                    synchronized (mLock) {
-                        if (mScheduledFuture == null || mPresenter == null) {
-                            // This task has been canceled. Just stop now.
-                            return;
-                        }
-                        currentPositionMs = mPresenter.getMediaPlayerPosition();
-                    }
-                    setClipPosition(currentPositionMs, mDurationMs);
-                }
-            });
+            post(mUpdateClipPositionRunnable);
         }
 
         public void startUpdating() {
             synchronized (mLock) {
-                if (mScheduledFuture != null) {
-                    mScheduledFuture.cancel(false);
-                    mScheduledFuture = null;
-                }
+                cancelPendingRunnables();
                 mScheduledFuture = mExecutorService.scheduleAtFixedRate(
                         this, 0, SLIDER_UPDATE_PERIOD_MILLIS, TimeUnit.MILLISECONDS);
             }
@@ -111,12 +110,17 @@ public class VoicemailPlaybackLayout extends LinearLayout
 
         public void stopUpdating() {
             synchronized (mLock) {
-                if (mScheduledFuture != null) {
-                    mScheduledFuture.cancel(false);
-                    mScheduledFuture = null;
-                }
+                cancelPendingRunnables();
             }
         }
+
+        private void cancelPendingRunnables() {
+            if (mScheduledFuture != null) {
+                mScheduledFuture.cancel(true);
+                mScheduledFuture = null;
+            }
+            removeCallbacks(mUpdateClipPositionRunnable);
+        }
     }
 
     /**
@@ -139,7 +143,7 @@ public class VoicemailPlaybackLayout extends LinearLayout
 
         @Override
         public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) {
-            setClipPosition(seekBar.getProgress(), seekBar.getMax());
+            setClipPosition(progress, seekBar.getMax());
         }
     };
 
@@ -246,6 +250,10 @@ public class VoicemailPlaybackLayout extends LinearLayout
             onSpeakerphoneOn(mPresenter.isSpeakerphoneOn());
         }
 
+        if (mPositionUpdater != null) {
+            mPositionUpdater.stopUpdating();
+            mPositionUpdater = null;
+        }
         mPositionUpdater = new PositionUpdater(duration, executorService);
         mPositionUpdater.startUpdating();
     }
diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
index f2621059b..f76af59a4 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
@@ -207,6 +207,11 @@ public class VoicemailPlaybackPresenter
             mPosition = savedInstanceState.getInt(CLIP_POSITION_KEY, 0);
             mIsPlaying = savedInstanceState.getBoolean(IS_PLAYING_STATE_KEY, false);
         }
+
+        if (mMediaPlayer == null) {
+            mIsPrepared = false;
+            mIsPlaying = false;
+        }
     }
 
     /**
@@ -240,9 +245,14 @@ public class VoicemailPlaybackPresenter
 
             mVoicemailUri = voicemailUri;
             mDuration.set(0);
-            mIsPlaying = startPlayingImmediately;
 
-            checkForContent();
+            if (startPlayingImmediately) {
+                // Since setPlaybackView can get called during the view binding process, we don't
+                // want to reset mIsPlaying to false if the user is currently playing the
+                // voicemail and the view is rebound.
+                mIsPlaying = startPlayingImmediately;
+                checkForContent();
+            }
 
             // Default to earpiece.
             mView.onSpeakerphoneOn(false);
@@ -284,6 +294,7 @@ public class VoicemailPlaybackPresenter
         if (mMediaPlayer != null) {
             mMediaPlayer.release();
             mMediaPlayer = null;
+            mIsPrepared = false;
         }
 
         if (mActivity != null) {
@@ -489,6 +500,7 @@ public class VoicemailPlaybackPresenter
         mIsPrepared = true;
 
         mDuration.set(mMediaPlayer.getDuration());
+        mPosition = mMediaPlayer.getCurrentPosition();
 
         mView.enableUiElements();
         Log.d(TAG, "onPrepared: mPosition=" + mPosition);
@@ -561,6 +573,10 @@ public class VoicemailPlaybackPresenter
      */
     public void resumePlayback() {
         if (!mIsPrepared) {
+            // If we haven't downloaded the voicemail yet, attempt to download it.
+            checkForContent();
+            mIsPlaying = true;
+
             return;
         }
 
@@ -607,7 +623,7 @@ public class VoicemailPlaybackPresenter
 
         mIsPlaying = false;
 
-        if (mMediaPlayer.isPlaying()) {
+        if (mMediaPlayer != null && mMediaPlayer.isPlaying()) {
             mMediaPlayer.pause();
         }