From 0bb5f09432567296b41890a39f5928f7e2f4f762 Mon Sep 17 00:00:00 2001 From: Yorke Lee Date: Tue, 7 Jul 2015 17:55:14 -0700 Subject: 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 --- .../dialer/voicemail/VoicemailPlaybackLayout.java | 54 +++++++++++++--------- .../voicemail/VoicemailPlaybackPresenter.java | 22 +++++++-- 2 files changed, 50 insertions(+), 26 deletions(-) (limited to 'src') 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(); } -- cgit v1.2.3