Bug 1373162 - Make sure GeckoHlsPlayer java callback runnable won't access a null object when player is removed. draft
authorKilik Kuo <kikuo@mozilla.com>
Fri, 16 Jun 2017 12:18:49 +0800
changeset 595178 1d18bf5da7b59f461dc8ca8a685f7899f3e8258c
parent 595097 79cdd4893c4607b8ad19c41ffd4ddde2f11d0151
child 633646 1c13bdbce8c5fc254217085584adc62f4f5cc47d
push id64272
push userbmo:kikuo@mozilla.com
push dateFri, 16 Jun 2017 04:19:43 +0000
bugs1373162
milestone56.0a1
Bug 1373162 - Make sure GeckoHlsPlayer java callback runnable won't access a null object when player is removed. MozReview-Commit-ID: CsKSwLOHmlJ
mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java
@@ -42,17 +42,17 @@ import org.mozilla.gecko.GeckoAppShell;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
 @ReflectionTarget
 public class GeckoHlsPlayer implements BaseHlsPlayer, ExoPlayer.EventListener {
     private static final String LOGTAG = "GeckoHlsPlayer";
     private static final DefaultBandwidthMeter BANDWIDTH_METER = new DefaultBandwidthMeter();
     private static final int MAX_TIMELINE_ITEM_LINES = 3;
-    private static boolean DEBUG = false;
+    private static boolean DEBUG = AppConstants.NIGHTLY_BUILD || AppConstants.DEBUG_BUILD;
 
     private static AtomicInteger sPlayerId = new AtomicInteger(0);
     /*
      *  Because we treat GeckoHlsPlayer as a source data provider.
      *  It will be created and initialized with a URL by HLSResource in
      *  Gecko media pipleine (in cpp). Once HLSDemuxer is created later, we
      *  need to bridge this HLSResource to the created demuxer. And they share
      *  the same GeckoHlsPlayer.
@@ -131,21 +131,22 @@ public class GeckoHlsPlayer implements B
 
     private static void assertTrue(boolean condition) {
       if (DEBUG && !condition) {
         throw new AssertionError("Expected condition to be true");
       }
     }
 
     protected void checkInitDone() {
-        assertTrue(mDemuxerCallbacks != null);
-        assertTrue(mTracksInfo != null);
         if (mIsDemuxerInitDone) {
             return;
         }
+        assertTrue(mDemuxerCallbacks != null);
+        assertTrue(mTracksInfo != null);
+
         if (DEBUG) {
             Log.d(LOGTAG, "[checkInitDone] VReady:" + mTracksInfo.videoReady() +
                     ",AReady:" + mTracksInfo.audioReady() +
                     ",hasV:" + mTracksInfo.hasVideo() +
                     ",hasA:" + mTracksInfo.hasAudio());
         }
         if (mTracksInfo.videoReady() && mTracksInfo.audioReady()) {
             mDemuxerCallbacks.onInitialized(mTracksInfo.hasAudio(), mTracksInfo.hasVideo());
@@ -201,40 +202,49 @@ public class GeckoHlsPlayer implements B
                 });
             }
         }
     }
 
     public final class ComponentListener {
 
         // General purpose implementation
-        public void onDataArrived(int trackType) {
+        public synchronized void onDataArrived(int trackType) {
+            if (DEBUG) { Log.d(LOGTAG, "[CB][onDataArrived] id " + mPlayerId); }
+            if (!mIsPlayerInitDone) {
+                return;
+            }
             assertTrue(mResourceCallbacks != null);
             assertTrue(mTracksInfo != null);
-            Log.d(LOGTAG, "[CB][onDataArrived]");
             mTracksInfo.onDataArrived(trackType);
             mResourceCallbacks.onDataArrived();
             checkInitDone();
         }
 
-        public void onVideoInputFormatChanged(Format format) {
-            assertTrue(mTracksInfo != null);
+        public synchronized void onVideoInputFormatChanged(Format format) {
             if (DEBUG) {
                 Log.d(LOGTAG, "[CB] onVideoInputFormatChanged [" + format + "]");
                 Log.d(LOGTAG, "[CB] SampleMIMEType [" +
                               format.sampleMimeType + "], ContainerMIMEType [" +
-                              format.containerMimeType + "]");
+                              format.containerMimeType + "], id : " + mPlayerId);
             }
+            if (!mIsPlayerInitDone) {
+                return;
+            }
+            assertTrue(mTracksInfo != null);
             mTracksInfo.onVideoInfoUpdated();
             checkInitDone();
         }
 
-        public void onAudioInputFormatChanged(Format format) {
+        public synchronized void onAudioInputFormatChanged(Format format) {
+            if (DEBUG) { Log.d(LOGTAG, "[CB] onAudioInputFormatChanged [" + format + "], mPlayerId :" + mPlayerId); }
+            if (!mIsPlayerInitDone) {
+                return;
+            }
             assertTrue(mTracksInfo != null);
-            if (DEBUG) { Log.d(LOGTAG, "[CB] onAudioInputFormatChanged [" + format + "]"); }
             mTracksInfo.onAudioInfoUpdated();
             checkInitDone();
         }
     }
 
     private DataSource.Factory buildDataSourceFactory(Context ctx, DefaultBandwidthMeter bandwidthMeter) {
         return new DefaultDataSourceFactory(ctx, bandwidthMeter,
                 buildHttpDataSourceFactory(bandwidthMeter));
@@ -543,17 +553,16 @@ public class GeckoHlsPlayer implements B
     }
 
     // =======================================================================
     // API for GeckoHLSDemuxerWrapper
     // =======================================================================
     @Override
     public ConcurrentLinkedQueue<GeckoHLSSample> getSamples(TrackType trackType,
                                                             int number) {
-        if (DEBUG) { Log.d(LOGTAG, "(" + trackType + ") getSamples : " + number); }
         if (trackType == TrackType.VIDEO) {
             return mVRenderer != null ? mVRenderer.getQueuedSamples(number) :
                                         new ConcurrentLinkedQueue<GeckoHLSSample>();
         } else if (trackType == TrackType.AUDIO) {
             return mARenderer != null ? mARenderer.getQueuedSamples(number) :
                                         new ConcurrentLinkedQueue<GeckoHLSSample>();
         } else {
             return new ConcurrentLinkedQueue<GeckoHLSSample>();
@@ -677,18 +686,18 @@ public class GeckoHlsPlayer implements B
     public long getNextKeyFrameTime() {
         long nextKeyFrameTime = mVRenderer != null
             ? mVRenderer.getNextKeyFrameTime()
             : Long.MAX_VALUE;
         return nextKeyFrameTime;
     }
 
     @Override
-    public void release() {
-        if (DEBUG) { Log.d(LOGTAG, "releasing  ..."); }
+    public synchronized void release() {
+        if (DEBUG) { Log.d(LOGTAG, "releasing  ... id : " + mPlayerId); }
         if (mPlayer != null) {
             mPlayer.removeListener(this);
             mPlayer.stop();
             mPlayer.release();
             mVRenderer = null;
             mARenderer = null;
             mPlayer = null;
         }