Bug 1324530 - part 4: improve remote manager init wait time. r=snorp draft
authorJohn Lin <jolin@mozilla.com>
Fri, 23 Dec 2016 16:28:48 +0800
changeset 456916 48845198b720d9bc267b1f79aeb96130efb13b38
parent 456915 4b89cbe8e5874848af343321ae465a97bd2399e3
child 456917 681f3c76bbe3e37c18473bc255ac2c1dc5782f0d
push id40642
push userbmo:jolin@mozilla.com
push dateFri, 06 Jan 2017 14:13:15 +0000
reviewerssnorp
bugs1324530
milestone53.0a1
Bug 1324530 - part 4: improve remote manager init wait time. r=snorp Fix the issue that RemoteManager.init() will be block for 5s when onServiceConnected() is finished before latch creation. MozReview-Commit-ID: CYYpLpY7Ns9
mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
mobile/android/base/java/org/mozilla/gecko/media/RemoteManager.java
--- a/mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
+++ b/mobile/android/base/java/org/mozilla/gecko/media/CodecProxy.java
@@ -183,34 +183,34 @@ public final class CodecProxy {
 
     @WrapForJNI
     public synchronized boolean flush() {
         if (mRemote == null) {
             Log.e(LOGTAG, "cannot flush an ended codec");
             return false;
         }
         try {
-            if (DEBUG) Log.d(LOGTAG, "flush " + this);
+            if (DEBUG) { Log.d(LOGTAG, "flush " + this); }
             mRemote.flush();
         } catch (DeadObjectException e) {
             return false;
         } catch (RemoteException e) {
             e.printStackTrace();
             return false;
         }
         return true;
     }
 
     @WrapForJNI
     public synchronized boolean release() {
         if (mRemote == null) {
             Log.w(LOGTAG, "codec already ended");
             return true;
         }
-        if (DEBUG) Log.d(LOGTAG, "release " + this);
+        if (DEBUG) { Log.d(LOGTAG, "release " + this); }
 
         if (!mSurfaceOutputs.isEmpty()) {
             // Flushing output buffers to surface may cause some frames to be skipped and
             // should not happen unless caller release codec before processing all buffers.
             Log.w(LOGTAG, "release codec when " + mSurfaceOutputs.size() + " output buffers unhandled");
             try {
                 for (Sample s : mSurfaceOutputs) {
                     mRemote.releaseOutput(s, true);
@@ -240,24 +240,25 @@ public final class CodecProxy {
         }
 
         if (mRemote == null) {
             Log.w(LOGTAG, "codec already ended");
             sample.dispose();
             return true;
         }
 
-        if (DEBUG && !render) Log.d(LOGTAG, "drop output:" + sample.info.presentationTimeUs);
+        if (DEBUG && !render) { Log.d(LOGTAG, "drop output:" + sample.info.presentationTimeUs); }
 
         try {
             mRemote.releaseOutput(sample, render);
         } catch (RemoteException e) {
+            Log.e(LOGTAG, "remote fail to render output:" + sample.info.presentationTimeUs);
             e.printStackTrace();
         }
         sample.dispose();
 
         return true;
     }
 
-    /* package */ synchronized void reportError(boolean fatal) {
+    /* package */ void reportError(boolean fatal) {
         mCallbacks.reportError(fatal);
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/media/RemoteManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/media/RemoteManager.java
@@ -13,18 +13,16 @@ import android.content.Intent;
 import android.content.ServiceConnection;
 import android.media.MediaFormat;
 import android.os.DeadObjectException;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.view.Surface;
 import android.util.Log;
 
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 import java.util.LinkedList;
 import java.util.List;
 
 public final class RemoteManager implements IBinder.DeathRecipient {
     private static final String LOGTAG = "GeckoRemoteManager";
     private static final boolean DEBUG = false;
     private static RemoteManager sRemoteManager = null;
 
@@ -34,93 +32,86 @@ public final class RemoteManager impleme
         }
 
         sRemoteManager.init();
         return sRemoteManager;
     }
 
     private List<CodecProxy> mProxies = new LinkedList<CodecProxy>();
     private volatile IMediaManager mRemote;
-    private volatile CountDownLatch mConnectionLatch;
-    private final ServiceConnection mConnection = new ServiceConnection() {
+
+    private final class RemoteConnection implements ServiceConnection {
         @Override
         public void onServiceConnected(ComponentName name, IBinder service) {
             if (DEBUG) Log.d(LOGTAG, "service connected");
             try {
                 service.linkToDeath(RemoteManager.this, 0);
             } catch (RemoteException e) {
                 e.printStackTrace();
             }
-            mRemote = IMediaManager.Stub.asInterface(service);
-            if (mConnectionLatch != null) {
-                mConnectionLatch.countDown();
+            synchronized (this) {
+                mRemote = IMediaManager.Stub.asInterface(service);
+                notify();
             }
         }
 
-        /**
-         * Called when a connection to the Service has been lost.  This typically
-         * happens when the process hosting the service has crashed or been killed.
-         * This does <em>not</em> remove the ServiceConnection itself -- this
-         * binding to the service will remain active, and you will receive a call
-         * to {@link #onServiceConnected} when the Service is next running.
-         *
-         * @param name The concrete component name of the service whose
-         *             connection has been lost.
-         */
         @Override
         public void onServiceDisconnected(ComponentName name) {
             if (DEBUG) Log.d(LOGTAG, "service disconnected");
             mRemote.asBinder().unlinkToDeath(RemoteManager.this, 0);
-            mRemote = null;
-            if (mConnectionLatch != null) {
-                mConnectionLatch.countDown();
+            synchronized (this) {
+                mRemote = null;
+                notify();
+            }
+        }
+
+        private boolean connect() {
+            Context appCtxt = GeckoAppShell.getApplicationContext();
+            appCtxt.bindService(new Intent(appCtxt, MediaManager.class),
+                    mConnection, Context.BIND_AUTO_CREATE);
+            waitConnect();
+            return mRemote != null;
+        }
+
+        // Wait up to 5s.
+        private synchronized void waitConnect() {
+            int waitCount = 0;
+            while (mRemote == null && waitCount < 5) {
+                try {
+                    wait(1000);
+                    waitCount++;
+                } catch (InterruptedException e) {
+                    if (DEBUG) { e.printStackTrace(); }
+                }
+            }
+            if (DEBUG) {
+                Log.d(LOGTAG, "wait ~" + waitCount + "s for connection: " + (mRemote == null ? "fail" : "ok"));
+            }
+        }
+
+        private synchronized void waitDisconnect() {
+            while (mRemote != null) {
+                try {
+                    wait(1000);
+                } catch (InterruptedException e) {
+                    if (DEBUG) { e.printStackTrace(); }
+                }
             }
         }
     };
 
+    RemoteConnection mConnection = new RemoteConnection();
+
     private synchronized boolean init() {
         if (mRemote != null) {
             return true;
         }
 
         if (DEBUG) Log.d(LOGTAG, "init remote manager " + this);
-        Context appCtxt = GeckoAppShell.getApplicationContext();
-        if (DEBUG) Log.d(LOGTAG, "ctxt=" + appCtxt);
-        appCtxt.bindService(new Intent(appCtxt, MediaManager.class),
-                mConnection, Context.BIND_AUTO_CREATE);
-        if (!waitConnection()) {
-            appCtxt.unbindService(mConnection);
-            return false;
-        }
-        return true;
-    }
-
-    private boolean waitConnection() {
-        boolean ok = false;
-
-        mConnectionLatch = new CountDownLatch(1);
-        try {
-            int retryCount = 0;
-            while (retryCount < 5) {
-                if (DEBUG) Log.d(LOGTAG, "waiting for connection latch:" + mConnectionLatch);
-                mConnectionLatch.await(1, TimeUnit.SECONDS);
-                if (mConnectionLatch.getCount() == 0) {
-                    break;
-                }
-                Log.w(LOGTAG, "Creator not connected in 1s. Try again.");
-                retryCount++;
-            }
-            ok = true;
-        } catch (InterruptedException e) {
-            Log.e(LOGTAG, "service not connected in 5 seconds. Stop waiting.");
-            e.printStackTrace();
-        }
-        mConnectionLatch = null;
-
-        return ok;
+        return mConnection.connect();
     }
 
     public synchronized CodecProxy createCodec(MediaFormat format,
                                                Surface surface,
                                                CodecProxy.Callbacks callbacks,
                                                String drmStubId) {
         if (mRemote == null) {
             if (DEBUG) Log.d(LOGTAG, "createCodec failed due to not initialize");
@@ -165,22 +156,18 @@ public final class RemoteManager impleme
     @Override
     public void binderDied() {
         Log.e(LOGTAG, "remote codec is dead");
         reportDecodingProcessCrash();
         handleRemoteDeath();
     }
 
     private synchronized void handleRemoteDeath() {
-        // Wait for onServiceDisconnected()
-        if (!waitConnection()) {
-            notifyError(true);
-            return;
-        }
-        // Restart
+        mConnection.waitDisconnect();
+
         if (init() && recoverRemoteCodec()) {
             notifyError(false);
         } else {
             notifyError(true);
         }
     }
 
     private synchronized void notifyError(boolean fatal) {