Fix threading problems in voicemail code

Hold libvariablespeed handle in a reference counted singleton. Don't
always reinitialize the engine twice in quick succession on startup.
Catch and properly handle RejectedExecutionException exceptions thrown
by the executor service.

Bug:11511992
Change-Id: I6198dec303209445a0efd0f410a67332d75c7507

2 files changed, 99 insertions, 48 deletions

diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java b/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java
index 1dbae65d1..826dec074 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java
@@ -74,7 +74,9 @@ public class VoicemailPlaybackFragment extends Fragment {
     };
 
     private VoicemailPlaybackPresenter mPresenter;
-    private ScheduledExecutorService mScheduledExecutorService;
+    private static int mMediaPlayerRefCount = 0;
+    private static MediaPlayerProxy mMediaPlayerInstance;
+    private static ScheduledExecutorService mScheduledExecutorService;
     private View mPlaybackLayout;
 
     @Override
@@ -87,7 +89,6 @@ public class VoicemailPlaybackFragment extends Fragment {
     @Override
     public void onActivityCreated(Bundle savedInstanceState) {
         super.onActivityCreated(savedInstanceState);
-        mScheduledExecutorService = createScheduledExecutorService();
         Bundle arguments = getArguments();
         Preconditions.checkNotNull(arguments, "fragment must be started with arguments");
         Uri voicemailUri = arguments.getParcelable(EXTRA_VOICEMAIL_URI);
@@ -99,8 +100,8 @@ public class VoicemailPlaybackFragment extends Fragment {
                 powerManager.newWakeLock(
                         PowerManager.SCREEN_DIM_WAKE_LOCK, getClass().getSimpleName());
         mPresenter = new VoicemailPlaybackPresenter(createPlaybackViewImpl(),
-                createMediaPlayer(mScheduledExecutorService), voicemailUri,
-                mScheduledExecutorService, startPlayback,
+                getMediaPlayerInstance(), voicemailUri,
+                getScheduledExecutorServiceInstance(), startPlayback,
                 AsyncTaskExecutors.createAsyncTaskExecutor(), wakeLock);
         mPresenter.onCreate(savedInstanceState);
     }
@@ -113,8 +114,8 @@ public class VoicemailPlaybackFragment extends Fragment {
 
     @Override
     public void onDestroy() {
+        shutdownMediaPlayer();
         mPresenter.onDestroy();
-        mScheduledExecutorService.shutdown();
         super.onDestroy();
     }
 
@@ -129,12 +130,36 @@ public class VoicemailPlaybackFragment extends Fragment {
                 mPlaybackLayout);
     }
 
-    private MediaPlayerProxy createMediaPlayer(ExecutorService executorService) {
-        return VariableSpeed.createVariableSpeed(executorService);
+    private static synchronized MediaPlayerProxy getMediaPlayerInstance() {
+        ++mMediaPlayerRefCount;
+        if (mMediaPlayerInstance == null) {
+            mMediaPlayerInstance = VariableSpeed.createVariableSpeed(
+                    getScheduledExecutorServiceInstance());
+        }
+        return mMediaPlayerInstance;
+    }
+
+    private static synchronized ScheduledExecutorService getScheduledExecutorServiceInstance() {
+        if (mScheduledExecutorService == null) {
+            mScheduledExecutorService = Executors.newScheduledThreadPool(
+                    NUMBER_OF_THREADS_IN_POOL);
+        }
+        return mScheduledExecutorService;
     }
 
-    private ScheduledExecutorService createScheduledExecutorService() {
-        return Executors.newScheduledThreadPool(NUMBER_OF_THREADS_IN_POOL);
+    private static synchronized void shutdownMediaPlayer() {
+        --mMediaPlayerRefCount;
+        if (mMediaPlayerRefCount > 0) {
+            return;
+        }
+        if (mScheduledExecutorService != null) {
+            mScheduledExecutorService.shutdown();
+            mScheduledExecutorService = null;
+        }
+        if (mMediaPlayerInstance != null) {
+            mMediaPlayerInstance.release();
+            mMediaPlayerInstance = null;
+        }
     }
 
     /**
diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
index ebda0eb3c..085ef669a 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
@@ -35,6 +35,7 @@ import com.android.ex.variablespeed.SingleThreadedMediaPlayerProxy;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -311,6 +312,7 @@ public class VoicemailPlaybackPresenter {
                             mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri);
                             mPlayer.setAudioStreamType(PLAYBACK_STREAM);
                             mPlayer.prepare();
+                            mDuration.set(mPlayer.getDuration());
                             return null;
                         } catch (Exception e) {
                             return e;
@@ -344,7 +346,7 @@ public class VoicemailPlaybackPresenter {
         mView.setSpeakerPhoneOn(mView.isSpeakerPhoneOn());
         mView.setRateDecreaseButtonListener(createRateDecreaseListener());
         mView.setRateIncreaseButtonListener(createRateIncreaseListener());
-        mView.setClipPosition(0, mPlayer.getDuration());
+        mView.setClipPosition(0, mDuration.get());
         mView.playbackStopped();
         // Always disable on stop.
         mView.disableProximitySensor();
@@ -363,6 +365,10 @@ public class VoicemailPlaybackPresenter {
     }
 
     public void onDestroy() {
+        if (mPrepareTask != null) {
+            mPrepareTask.cancel(false);
+            mPrepareTask = null;
+        }
         mPlayer.release();
         if (mFetchResultHandler != null) {
             mFetchResultHandler.destroy();
@@ -430,49 +436,67 @@ public class VoicemailPlaybackPresenter {
         }
     }
 
+    private class AsyncPrepareTask extends AsyncTask<Void, Void, Exception> {
+        private int mClipPositionInMillis;
+
+        AsyncPrepareTask(int clipPositionInMillis) {
+            mClipPositionInMillis = clipPositionInMillis;
+        }
+
+        @Override
+        public Exception doInBackground(Void... params) {
+            try {
+                if (!mPlayer.isReadyToPlay()) {
+                    mPlayer.reset();
+                    mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri);
+                    mPlayer.setAudioStreamType(PLAYBACK_STREAM);
+                    mPlayer.prepare();
+                }
+                return null;
+            } catch (Exception e) {
+                return e;
+            }
+        }
+
+        @Override
+        public void onPostExecute(Exception exception) {
+            mPrepareTask = null;
+            if (exception == null) {
+                final int duration = mPlayer.getDuration();
+                mDuration.set(duration);
+                int startPosition =
+                    constrain(mClipPositionInMillis, 0, duration);
+                mPlayer.seekTo(startPosition);
+                mView.setClipPosition(startPosition, duration);
+                try {
+                    // Can throw RejectedExecutionException
+                    mPlayer.start();
+                    mView.playbackStarted();
+                    if (!mWakeLock.isHeld()) {
+                        mWakeLock.acquire();
+                    }
+                    // Only enable if we are not currently using the speaker phone.
+                    if (!mView.isSpeakerPhoneOn()) {
+                        mView.enableProximitySensor();
+                    }
+                    // Can throw RejectedExecutionException
+                    mPositionUpdater.startUpdating(startPosition, duration);
+                } catch (RejectedExecutionException e) {
+                    handleError(e);
+                }
+            } else {
+                handleError(exception);
+            }
+        }
+    }
+
     private void resetPrepareStartPlaying(final int clipPositionInMillis) {
         if (mPrepareTask != null) {
             mPrepareTask.cancel(false);
+            mPrepareTask = null;
         }
         mPrepareTask = mAsyncTaskExecutor.submit(Tasks.RESET_PREPARE_START_MEDIA_PLAYER,
-                new AsyncTask<Void, Void, Exception>() {
-                    @Override
-                    public Exception doInBackground(Void... params) {
-                        try {
-                            mPlayer.reset();
-                            mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri);
-                            mPlayer.setAudioStreamType(PLAYBACK_STREAM);
-                            mPlayer.prepare();
-                            return null;
-                        } catch (Exception e) {
-                            return e;
-                        }
-                    }
-
-                    @Override
-                    public void onPostExecute(Exception exception) {
-                        mPrepareTask = null;
-                        if (exception == null) {
-                            mDuration.set(mPlayer.getDuration());
-                            int startPosition =
-                                    constrain(clipPositionInMillis, 0, mDuration.get());
-                            mView.setClipPosition(startPosition, mDuration.get());
-                            mPlayer.seekTo(startPosition);
-                            mPlayer.start();
-                            mView.playbackStarted();
-                            if (!mWakeLock.isHeld()) {
-                                mWakeLock.acquire();
-                            }
-                            // Only enable if we are not currently using the speaker phone.
-                            if (!mView.isSpeakerPhoneOn()) {
-                                mView.enableProximitySensor();
-                            }
-                            mPositionUpdater.startUpdating(startPosition, mDuration.get());
-                        } else {
-                            handleError(exception);
-                        }
-                    }
-                });
+                new AsyncPrepareTask(clipPositionInMillis));
     }
 
     private void handleError(Exception e) {
@@ -598,6 +622,7 @@ public class VoicemailPlaybackPresenter {
             synchronized (mLock) {
                 if (mScheduledFuture != null) {
                     mScheduledFuture.cancel(false);
+                    mScheduledFuture = null;
                 }
                 mScheduledFuture = mExecutorService.scheduleAtFixedRate(this, 0, mPeriodMillis,
                         TimeUnit.MILLISECONDS);
@@ -620,6 +645,7 @@ public class VoicemailPlaybackPresenter {
         }
         if (mPrepareTask != null) {
             mPrepareTask.cancel(false);
+            mPrepareTask = null;
         }
         if (mWakeLock.isHeld()) {
             mWakeLock.release();