Replace LooperExecutor with built-in class in Android AppRTC Demo

LooperExecutor is only truly needed in WebSocketChannelClient because of
WebSocketClient from autobanh requiring thread to have a looper. So
LooperExecutor was left there but replaced everywhere else with built-in
singleThreadExecutor/singleThreadScheduledExecutor.

Motivation behind this change is that built-in class behaves better
under testing environment and doesn't require hacky
RobolectricLooperExecutor.

Review-Url: https://codereview.webrtc.org/1992213002
Cr-Commit-Position: refs/heads/master@{#12823}

webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java

@@ -10,11 +10,6 @@
 
 package org.appspot.apprtc;
 
-import org.appspot.apprtc.AppRTCClient.RoomConnectionParameters;
-import org.appspot.apprtc.AppRTCClient.SignalingParameters;
-import org.appspot.apprtc.PeerConnectionClient.PeerConnectionParameters;
-import org.appspot.apprtc.util.LooperExecutor;
-
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.app.FragmentTransaction;
@@ -30,12 +25,16 @@ import android.view.Window;
 import android.view.WindowManager.LayoutParams;
 import android.widget.Toast;
 
+import org.appspot.apprtc.AppRTCClient.RoomConnectionParameters;
+import org.appspot.apprtc.AppRTCClient.SignalingParameters;
+import org.appspot.apprtc.PeerConnectionClient.PeerConnectionParameters;
+import org.appspot.apprtc.util.LooperExecutor;
 import org.webrtc.EglBase;
 import org.webrtc.IceCandidate;
 import org.webrtc.PeerConnectionFactory;
+import org.webrtc.RendererCommon.ScalingType;
 import org.webrtc.SessionDescription;
 import org.webrtc.StatsReport;
-import org.webrtc.RendererCommon.ScalingType;
 import org.webrtc.SurfaceViewRenderer;
 
 /**
@@ -245,7 +244,7 @@ public class CallActivity extends Activity
       appRtcClient = new WebSocketRTCClient(this, new LooperExecutor());
     } else {
       Log.i(TAG, "Using DirectRTCClient because room name looks like an IP.");
-      appRtcClient = new DirectRTCClient(this, new LooperExecutor());
+      appRtcClient = new DirectRTCClient(this);
     }
     // Create connection parameters.
     roomConnectionParameters = new RoomConnectionParameters(
@@ -314,9 +313,6 @@ public class CallActivity extends Activity
     }
     activityRunning = false;
     rootEglBase.release();
-    if (cpuMonitor != null) {
-      cpuMonitor.release();
-    }
     super.onDestroy();
   }
 

webrtc/examples/androidapp/src/org/appspot/apprtc/CpuMonitor.java

@@ -23,8 +23,9 @@ import java.io.FileReader;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Scanner;
-
-import org.appspot.apprtc.util.LooperExecutor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Simple CPU monitor.  The caller creates a CpuMonitor object which can then
@@ -85,7 +86,7 @@ class CpuMonitor {
   // CPU frequency in percentage from maximum.
   private final MovingAverage frequencyScale;
 
-  private LooperExecutor executor;
+  private ScheduledExecutorService executor;
   private long lastStatLogTimeMs;
   private long[] cpuFreqMax;
   private int cpusPresent;
@@ -159,34 +160,22 @@ class CpuMonitor {
     frequencyScale = new MovingAverage(MOVING_AVERAGE_SAMPLES);
     lastStatLogTimeMs = SystemClock.elapsedRealtime();
 
-    executor = new LooperExecutor();
-    executor.requestStart();
     scheduleCpuUtilizationTask();
   }
 
-  public void release() {
-    if (executor != null) {
-      Log.d(TAG, "release");
-      executor.cancelScheduledTasks();
-      executor.requestStop();
-      executor = null;
-    }
-  }
-
   public void pause() {
     if (executor != null) {
       Log.d(TAG, "pause");
-      executor.cancelScheduledTasks();
+      executor.shutdownNow();
+      executor = null;
     }
   }
 
   public void resume() {
-    if (executor != null) {
     Log.d(TAG, "resume");
     resetStat();
     scheduleCpuUtilizationTask();
   }
-  }
 
   public synchronized void reset() {
     if (executor != null) {
@@ -209,13 +198,18 @@ class CpuMonitor {
   }
 
   private void scheduleCpuUtilizationTask() {
-    executor.cancelScheduledTasks();
+    if (executor != null) {
+      executor.shutdownNow();
+      executor = null;
+    }
+
+    executor = Executors.newSingleThreadScheduledExecutor();
     executor.scheduleAtFixedRate(new Runnable() {
       @Override
       public void run() {
         cpuUtilizationTask();
       }
-    }, CPU_STAT_SAMPLE_PERIOD_MS);
+    }, 0, CPU_STAT_SAMPLE_PERIOD_MS, TimeUnit.MILLISECONDS);
   }
 
   private void cpuUtilizationTask() {

webrtc/examples/androidapp/src/org/appspot/apprtc/DirectRTCClient.java

@@ -12,7 +12,6 @@ package org.appspot.apprtc;
 
 import android.util.Log;
 
-import org.appspot.apprtc.util.LooperExecutor;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -21,6 +20,8 @@ import org.webrtc.PeerConnection;
 import org.webrtc.SessionDescription;
 
 import java.util.LinkedList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -52,7 +53,7 @@ public class DirectRTCClient implements AppRTCClient, TCPChannelClient.TCPChanne
       + "(:(\\d+))?"
   );
 
-  private final LooperExecutor executor;
+  private final ExecutorService executor;
   private final SignalingEvents events;
   private TCPChannelClient tcpClient;
   private RoomConnectionParameters connectionParameters;
@@ -64,11 +65,10 @@ public class DirectRTCClient implements AppRTCClient, TCPChannelClient.TCPChanne
   // All alterations of the room state should be done from inside the looper thread.
   private ConnectionState roomState;
 
-  public DirectRTCClient(SignalingEvents events, LooperExecutor looperExecutor) {
+  public DirectRTCClient(SignalingEvents events) {
     this.events = events;
-    executor = looperExecutor;
 
-    executor.requestStart();
+    executor = Executors.newSingleThreadExecutor();
     roomState = ConnectionState.NEW;
   }
 
@@ -148,6 +148,7 @@ public class DirectRTCClient implements AppRTCClient, TCPChannelClient.TCPChanne
       tcpClient.disconnect();
       tcpClient = null;
     }
+    executor.shutdown();
   }
 
   @Override
@@ -295,13 +296,11 @@ public class DirectRTCClient implements AppRTCClient, TCPChannelClient.TCPChanne
   @Override
   public void onTCPError(String description) {
     reportError("TCP connection error: " + description);
-    executor.requestStop();
   }
 
   @Override
   public void onTCPClose() {
     events.onChannelClose();
-    executor.requestStop();
   }
 
   // --------------------------------------------------------------------

webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java

@@ -16,7 +16,6 @@ import android.os.Environment;
 import android.util.Log;
 
 import org.appspot.apprtc.AppRTCClient.SignalingParameters;
-import org.appspot.apprtc.util.LooperExecutor;
 import org.webrtc.AudioTrack;
 import org.webrtc.CameraEnumerationAndroid;
 import org.webrtc.DataChannel;
@@ -46,6 +45,9 @@ import java.util.EnumSet;
 import java.util.LinkedList;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -88,7 +90,7 @@ public class PeerConnectionClient {
   private static final PeerConnectionClient instance = new PeerConnectionClient();
   private final PCObserver pcObserver = new PCObserver();
   private final SDPObserver sdpObserver = new SDPObserver();
-  private final LooperExecutor executor;
+  private final ScheduledExecutorService executor;
 
   private PeerConnectionFactory factory;
   private PeerConnection peerConnection;
@@ -219,11 +221,10 @@ public class PeerConnectionClient {
   }
 
   private PeerConnectionClient() {
-    executor = new LooperExecutor();
-    // Looper thread is started once in private ctor and is used for all
+    // Executor thread is started once in private ctor and is used for all
     // peer connection API calls to ensure new peer connection factory is
     // created on the same thread as previously destroyed factory.
-    executor.requestStart();
+    executor = Executors.newSingleThreadScheduledExecutor();
   }
 
   public static PeerConnectionClient getInstance() {

webrtc/examples/androidapp/src/org/appspot/apprtc/TCPChannelClient.java

@@ -12,7 +12,7 @@ package org.appspot.apprtc;
 
 import android.util.Log;
 
-import org.appspot.apprtc.util.LooperExecutor;
+import org.webrtc.ThreadUtils;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -22,6 +22,7 @@ import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.UnknownHostException;
+import java.util.concurrent.ExecutorService;
 
 /**
  * Replacement for WebSocketChannelClient for direct communication between two IP addresses. Handles
@@ -34,7 +35,8 @@ import java.net.UnknownHostException;
 public class TCPChannelClient {
   private static final String TAG = "TCPChannelClient";
 
-  private final LooperExecutor executor;
+  private final ExecutorService executor;
+  private final ThreadUtils.ThreadChecker executorThreadCheck;
   private final TCPChannelEvents eventListener;
   private TCPSocket socket;
 
@@ -58,8 +60,10 @@ public class TCPChannelClient {
    * @param port Port to listen on or connect to.
    */
   public TCPChannelClient(
-      LooperExecutor executor, TCPChannelEvents eventListener, String ip, int port) {
+      ExecutorService executor, TCPChannelEvents eventListener, String ip, int port) {
     this.executor = executor;
+    executorThreadCheck = new ThreadUtils.ThreadChecker();
+    executorThreadCheck.detachThread();
     this.eventListener = eventListener;
 
     InetAddress address;
@@ -83,7 +87,7 @@ public class TCPChannelClient {
    * Disconnects the client if not already disconnected. This will fire the onTCPClose event.
    */
   public void disconnect() {
-    checkIfCalledOnValidThread();
+    executorThreadCheck.checkIsOnValidThread();
 
     socket.disconnect();
   }
@@ -94,7 +98,7 @@ public class TCPChannelClient {
    * @param message Message to be sent.
    */
   public void send(String message) {
-    checkIfCalledOnValidThread();
+    executorThreadCheck.checkIsOnValidThread();
 
     socket.send(message);
   }
@@ -112,17 +116,6 @@ public class TCPChannelClient {
     });
   }
 
-  /**
-   * Helper method for debugging purposes.
-   * Ensures that TCPChannelClient method is called on a looper thread.
-   */
-  private void checkIfCalledOnValidThread() {
-    if (!executor.checkOnLooperThread()) {
-      throw new IllegalStateException(
-          "TCPChannelClient method is not called on valid thread");
-    }
-  }
-
 
   /**
    * Base class for server and client sockets. Contains a listening thread that will call

webrtc/examples/androidapp/src/org/appspot/apprtc/util/LooperExecutor.java

@@ -19,7 +19,8 @@ import java.util.List;
 import java.util.concurrent.Executor;
 
 /**
- * Looper based executor class.
+ * Looper based executor class. This is needed because WebSocketClient from autobanh requires the
+ * thread to have a looper. The class is used in WebSocketRTCClient/WebSocketChannelClient.
  */
 public class LooperExecutor extends Thread implements Executor {
   private static final String TAG = "LooperExecutor";

webrtc/examples/androidjunit/src/org/appspot/apprtc/DirectRTCClientTest.java

@@ -10,9 +10,6 @@
 
 package org.appspot.apprtc;
 
-import android.util.Log;
-
-import org.appspot.apprtc.util.RobolectricLooperExecutor;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -56,8 +53,8 @@ public class DirectRTCClientTest {
     clientEvents = mock(AppRTCClient.SignalingEvents.class);
     serverEvents = mock(AppRTCClient.SignalingEvents.class);
 
-    client = new DirectRTCClient(clientEvents, new RobolectricLooperExecutor());
-    server = new DirectRTCClient(serverEvents, new RobolectricLooperExecutor());
+    client = new DirectRTCClient(clientEvents);
+    server = new DirectRTCClient(serverEvents);
   }
 
   @Test

webrtc/examples/androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java

@@ -10,7 +10,7 @@
 
 package org.appspot.apprtc;
 
-import org.appspot.apprtc.util.RobolectricLooperExecutor;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -21,6 +21,11 @@ import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowLog;
 
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
@@ -38,13 +43,14 @@ public class TCPChannelClientTest {
   private static final int CONNECT_TIMEOUT = 100;
   private static final int SEND_TIMEOUT = 100;
   private static final int DISCONNECT_TIMEOUT = 100;
+  private static final int TERMINATION_TIMEOUT = 1000;
   private static final String TEST_MESSAGE_SERVER = "Hello, Server!";
   private static final String TEST_MESSAGE_CLIENT = "Hello, Client!";
 
   @Mock TCPChannelClient.TCPChannelEvents serverEvents;
   @Mock TCPChannelClient.TCPChannelEvents clientEvents;
 
-  private RobolectricLooperExecutor executor;
+  private ExecutorService executor;
   private TCPChannelClient server;
   private TCPChannelClient client;
 
@@ -55,15 +61,14 @@ public class TCPChannelClientTest {
 
     MockitoAnnotations.initMocks(this);
 
-    executor = new RobolectricLooperExecutor();
-    executor.requestStart();
+    executor = Executors.newSingleThreadExecutor();
   }
 
   @After
   public void tearDown() {
     verifyNoMoreEvents();
 
-    executor.executeAndWait(new Runnable() {
+    executeAndWait(new Runnable() {
       @Override
       public void run() {
         client.disconnect();
@@ -72,9 +77,9 @@ public class TCPChannelClientTest {
     });
 
     // Stop the executor thread
-    executor.requestStop();
+    executor.shutdown();
     try {
-      executor.join();
+      executor.awaitTermination(TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS);
     } catch (InterruptedException e) {
       fail(e.getMessage());
     }
@@ -112,7 +117,7 @@ public class TCPChannelClientTest {
   public void testSendData() {
     testConnectIPv4();
 
-    executor.executeAndWait(new Runnable() {
+    executeAndWait(new Runnable() {
       @Override
       public void run() {
         client.send(TEST_MESSAGE_SERVER);
@@ -127,7 +132,7 @@ public class TCPChannelClientTest {
   @Test
   public void testDisconnectServer() {
     testConnectIPv4();
-    executor.executeAndWait(new Runnable() {
+    executeAndWait(new Runnable() {
       @Override
       public void run() {
         server.disconnect();
@@ -141,7 +146,7 @@ public class TCPChannelClientTest {
   @Test
   public void testDisconnectClient() {
     testConnectIPv4();
-    executor.executeAndWait(new Runnable() {
+    executeAndWait(new Runnable() {
       @Override
       public void run() {
         client.disconnect();
@@ -183,4 +188,15 @@ public class TCPChannelClientTest {
     verifyNoMoreInteractions(serverEvents);
     verifyNoMoreInteractions(clientEvents);
   }
+
+  /**
+   * Queues runnable to be run and waits for it to be executed by the executor thread
+   */
+  public void executeAndWait(Runnable runnable) {
+    try {
+      executor.submit(runnable).get();
+    } catch (Exception e) {
+      fail(e.getMessage());
+    }
+  }
 }

webrtc/examples/androidjunit/src/org/appspot/apprtc/util/RobolectricLooperExecutor.java

@@ -1,118 +0,0 @@
-/*
- *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
- *
- *  Use of this source code is governed by a BSD-style license
- *  that can be found in the LICENSE file in the root of the source
- *  tree. An additional intellectual property rights grant can be found
- *  in the file PATENTS.  All contributing project authors may
- *  be found in the AUTHORS file in the root of the source tree.
- */
-
-package org.appspot.apprtc.util;
-
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.BlockingQueue;
-
-import static org.junit.Assert.fail;
-
-/**
- * LooperExecutor that doesn't use Looper because its implementation in Robolectric is not suited
- * for our needs. Also implements executeAndWait that can be used to wait until the runnable has
- * been executed.
- */
-public class RobolectricLooperExecutor extends LooperExecutor {
-  private volatile boolean running = false;
-  private static final int RUNNABLE_QUEUE_CAPACITY = 256;
-  private final BlockingQueue<Runnable> runnableQueue
-      = new ArrayBlockingQueue<>(RUNNABLE_QUEUE_CAPACITY);
-  private long threadId;
-
-  /**
-   * Executes the runnable passed to the constructor and sets isDone flag afterwards.
-   */
-  private static class ExecuteAndWaitRunnable implements Runnable {
-    public boolean isDone = false;
-    private final Runnable runnable;
-
-    ExecuteAndWaitRunnable(Runnable runnable) {
-      this.runnable = runnable;
-    }
-
-    @Override
-    public void run() {
-      runnable.run();
-
-      synchronized (this) {
-        isDone = true;
-        notifyAll();
-      }
-    }
-  }
-
-  @Override
-  public void run() {
-    threadId = Thread.currentThread().getId();
-
-    while (running) {
-      final Runnable runnable;
-
-      try {
-        runnable = runnableQueue.take();
-      } catch (InterruptedException e) {
-        if (running) {
-          fail(e.getMessage());
-        }
-        return;
-      }
-
-      runnable.run();
-    }
-  }
-
-  @Override
-  public synchronized void requestStart() {
-    if (running) {
-      return;
-    }
-    running = true;
-    start();
-  }
-
-  @Override
-  public synchronized void requestStop() {
-    running = false;
-    interrupt();
-  }
-
-  @Override
-  public synchronized void execute(Runnable runnable) {
-    try {
-      runnableQueue.put(runnable);
-    } catch (InterruptedException e) {
-      fail(e.getMessage());
-    }
-  }
-
-  /**
-   * Queues runnable to be run and waits for it to be executed by the executor thread
-   */
-  public void executeAndWait(Runnable runnable) {
-    ExecuteAndWaitRunnable executeAndWaitRunnable = new ExecuteAndWaitRunnable(runnable);
-    execute(executeAndWaitRunnable);
-
-    synchronized (executeAndWaitRunnable) {
-      while (!executeAndWaitRunnable.isDone) {
-        try {
-          executeAndWaitRunnable.wait();
-        } catch (InterruptedException e) {
-          fail(e.getMessage());
-        }
-      }
-    }
-  }
-
-  @Override
-  public boolean checkOnLooperThread() {
-    return (Thread.currentThread().getId() == threadId);
-  }
-}

webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java

@@ -13,13 +13,15 @@ package org.appspot.apprtc.test;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
 import org.appspot.apprtc.AppRTCClient.SignalingParameters;
 import org.appspot.apprtc.PeerConnectionClient;
 import org.appspot.apprtc.PeerConnectionClient.PeerConnectionEvents;
 import org.appspot.apprtc.PeerConnectionClient.PeerConnectionParameters;
-import org.appspot.apprtc.util.LooperExecutor;
 import org.webrtc.EglBase;
 import org.webrtc.IceCandidate;
 import org.webrtc.MediaCodecVideoEncoder;
@@ -66,7 +68,7 @@ public class PeerConnectionClientTest extends InstrumentationTestCase
   private EglBase eglBase;
 
   // These are protected by their respective event objects.
-  private LooperExecutor signalingExecutor;
+  private ExecutorService signalingExecutor;
   private boolean isClosed;
   private boolean isIceConnected;
   private SessionDescription localSdp;
@@ -279,8 +281,7 @@ public class PeerConnectionClientTest extends InstrumentationTestCase
 
   @Override
   public void setUp() {
-    signalingExecutor = new LooperExecutor();
-    signalingExecutor.requestStart();
+    signalingExecutor = Executors.newSingleThreadExecutor();
     if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
       eglBase = EglBase.create();
     }
@@ -288,7 +289,7 @@ public class PeerConnectionClientTest extends InstrumentationTestCase
 
   @Override
   public void tearDown() {
-    signalingExecutor.requestStop();
+    signalingExecutor.shutdown();
     if (eglBase != null) {
       eglBase.release();
     }