From 5d4d59c069628396548ce36c5b7c4d95fba34a5b Mon Sep 17 00:00:00 2001 From: sakal Date: Tue, 9 Aug 2016 02:03:37 -0700 Subject: [PATCH] Remove LooperExecutor and replace it with built-in HandlerThread. Review-Url: https://codereview.webrtc.org/2206473003 Cr-Commit-Position: refs/heads/master@{#13688} --- webrtc/examples/BUILD.gn | 2 - .../src/org/appspot/apprtc/CallActivity.java | 3 +- .../apprtc/WebSocketChannelClient.java | 22 +-- .../appspot/apprtc/WebSocketRTCClient.java | 32 ++-- .../appspot/apprtc/util/LooperExecutor.java | 140 ------------------ .../apprtc/util/LooperExecutorTest.java | 122 --------------- 6 files changed, 29 insertions(+), 292 deletions(-) delete mode 100644 webrtc/examples/androidapp/src/org/appspot/apprtc/util/LooperExecutor.java delete mode 100644 webrtc/examples/androidjunit/src/org/appspot/apprtc/util/LooperExecutorTest.java diff --git a/webrtc/examples/BUILD.gn b/webrtc/examples/BUILD.gn index 905672d674..801ae0962f 100644 --- a/webrtc/examples/BUILD.gn +++ b/webrtc/examples/BUILD.gn @@ -76,7 +76,6 @@ if (is_android) { "androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java", "androidapp/src/org/appspot/apprtc/util/AppRTCUtils.java", "androidapp/src/org/appspot/apprtc/util/AsyncHttpURLConnection.java", - "androidapp/src/org/appspot/apprtc/util/LooperExecutor.java", ] deps = [ @@ -110,7 +109,6 @@ if (is_android) { java_files = [ "androidjunit/src/org/appspot/apprtc/DirectRTCClientTest.java", "androidjunit/src/org/appspot/apprtc/TCPChannelClientTest.java", - "androidjunit/src/org/appspot/apprtc/util/LooperExecutorTest.java", ] deps = [ diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java index afa7e44aad..39740a35bb 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/CallActivity.java @@ -13,7 +13,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; @@ -263,7 +262,7 @@ public class CallActivity extends Activity // Create connection client. Use DirectRTCClient if room name is an IP otherwise use the // standard WebSocketRTCClient. if (loopback || !DirectRTCClient.IP_PATTERN.matcher(roomId).matches()) { - appRtcClient = new WebSocketRTCClient(this, new LooperExecutor()); + appRtcClient = new WebSocketRTCClient(this); } else { Log.i(TAG, "Using DirectRTCClient because room name looks like an IP."); appRtcClient = new DirectRTCClient(this); diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketChannelClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketChannelClient.java index 5f8a1fdfcf..139f2bd49b 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketChannelClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketChannelClient.java @@ -12,8 +12,8 @@ package org.appspot.apprtc; import org.appspot.apprtc.util.AsyncHttpURLConnection; import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; -import org.appspot.apprtc.util.LooperExecutor; +import android.os.Handler; import android.util.Log; import de.tavendo.autobahn.WebSocket.WebSocketConnectionObserver; @@ -39,7 +39,7 @@ public class WebSocketChannelClient { private static final String TAG = "WSChannelRTCClient"; private static final int CLOSE_TIMEOUT = 1000; private final WebSocketChannelEvents events; - private final LooperExecutor executor; + private final Handler handler; private WebSocketConnection ws; private WebSocketObserver wsObserver; private String wsServerUrl; @@ -70,8 +70,8 @@ public class WebSocketChannelClient { void onWebSocketError(final String description); } - public WebSocketChannelClient(LooperExecutor executor, WebSocketChannelEvents events) { - this.executor = executor; + public WebSocketChannelClient(Handler handler, WebSocketChannelEvents events) { + this.handler = handler; this.events = events; roomID = null; clientID = null; @@ -204,7 +204,7 @@ public class WebSocketChannelClient { private void reportError(final String errorMessage) { Log.e(TAG, errorMessage); - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (state != WebSocketConnectionState.ERROR) { @@ -233,10 +233,10 @@ public class WebSocketChannelClient { httpConnection.send(); } - // Helper method for debugging purposes. Ensures that WebSocket method is - // called on a looper thread. + // Helper method for debugging purposes. Ensures that WebSocket method is + // called on a looper thread. private void checkIfCalledOnValidThread() { - if (!executor.checkOnLooperThread()) { + if (Thread.currentThread() != handler.getLooper().getThread()) { throw new IllegalStateException( "WebSocket method is not called on valid thread"); } @@ -246,7 +246,7 @@ public class WebSocketChannelClient { @Override public void onOpen() { Log.d(TAG, "WebSocket connection opened to: " + wsServerUrl); - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { state = WebSocketConnectionState.CONNECTED; @@ -266,7 +266,7 @@ public class WebSocketChannelClient { closeEvent = true; closeEventLock.notify(); } - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (state != WebSocketConnectionState.CLOSED) { @@ -281,7 +281,7 @@ public class WebSocketChannelClient { public void onTextMessage(String payload) { Log.d(TAG, "WSS->C: " + payload); final String message = payload; - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (state == WebSocketConnectionState.CONNECTED diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java index 502b8d049d..0178d69f6e 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/WebSocketRTCClient.java @@ -15,8 +15,9 @@ import org.appspot.apprtc.WebSocketChannelClient.WebSocketChannelEvents; import org.appspot.apprtc.WebSocketChannelClient.WebSocketConnectionState; import org.appspot.apprtc.util.AsyncHttpURLConnection; import org.appspot.apprtc.util.AsyncHttpURLConnection.AsyncHttpEvents; -import org.appspot.apprtc.util.LooperExecutor; +import android.os.Handler; +import android.os.HandlerThread; import android.util.Log; import org.json.JSONArray; @@ -48,7 +49,7 @@ public class WebSocketRTCClient implements AppRTCClient, private enum MessageType { MESSAGE, LEAVE }; - private final LooperExecutor executor; + private final Handler handler; private boolean initiator; private SignalingEvents events; private WebSocketChannelClient wsClient; @@ -57,11 +58,12 @@ public class WebSocketRTCClient implements AppRTCClient, private String messageUrl; private String leaveUrl; - public WebSocketRTCClient(SignalingEvents events, LooperExecutor executor) { + public WebSocketRTCClient(SignalingEvents events) { this.events = events; - this.executor = executor; roomState = ConnectionState.NEW; - executor.requestStart(); + final HandlerThread handlerThread = new HandlerThread(TAG); + handlerThread.start(); + handler = new Handler(handlerThread.getLooper()); } // -------------------------------------------------------------------- @@ -71,7 +73,7 @@ public class WebSocketRTCClient implements AppRTCClient, @Override public void connectToRoom(RoomConnectionParameters connectionParameters) { this.connectionParameters = connectionParameters; - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { connectToRoomInternal(); @@ -81,13 +83,13 @@ public class WebSocketRTCClient implements AppRTCClient, @Override public void disconnectFromRoom() { - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { disconnectFromRoomInternal(); + handler.getLooper().quit(); } }); - executor.requestStop(); } // Connects to room - function runs on a local looper thread. @@ -95,13 +97,13 @@ public class WebSocketRTCClient implements AppRTCClient, String connectionUrl = getConnectionUrl(connectionParameters); Log.d(TAG, "Connect to room: " + connectionUrl); roomState = ConnectionState.NEW; - wsClient = new WebSocketChannelClient(executor, this); + wsClient = new WebSocketChannelClient(handler, this); RoomParametersFetcherEvents callbacks = new RoomParametersFetcherEvents() { @Override public void onSignalingParametersReady( final SignalingParameters params) { - WebSocketRTCClient.this.executor.execute(new Runnable() { + WebSocketRTCClient.this.handler.post(new Runnable() { @Override public void run() { WebSocketRTCClient.this.signalingParametersReady(params); @@ -184,7 +186,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Send local offer SDP to the other participant. @Override public void sendOfferSdp(final SessionDescription sdp) { - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (roomState != ConnectionState.CONNECTED) { @@ -209,7 +211,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Send local answer SDP to the other participant. @Override public void sendAnswerSdp(final SessionDescription sdp) { - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (connectionParameters.loopback) { @@ -227,7 +229,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Send Ice candidate to the other participant. @Override public void sendLocalIceCandidate(final IceCandidate candidate) { - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { JSONObject json = new JSONObject(); @@ -256,7 +258,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Send removed Ice candidates to the other participant. @Override public void sendLocalIceCandidateRemovals(final IceCandidate[] candidates) { - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { JSONObject json = new JSONObject(); @@ -359,7 +361,7 @@ public class WebSocketRTCClient implements AppRTCClient, // Helper functions. private void reportError(final String errorMessage) { Log.e(TAG, errorMessage); - executor.execute(new Runnable() { + handler.post(new Runnable() { @Override public void run() { if (roomState != ConnectionState.ERROR) { diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/util/LooperExecutor.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/util/LooperExecutor.java deleted file mode 100644 index 4db807a3a6..0000000000 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/util/LooperExecutor.java +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright 2015 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 android.os.Handler; -import android.os.Looper; -import android.util.Log; - -import java.util.LinkedList; -import java.util.List; -import java.util.concurrent.Executor; - -/** - * 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"; - // Object used to signal that looper thread has started and Handler instance - // associated with looper thread has been allocated. - private final Object looperStartedEvent = new Object(); - private final List scheduledPeriodicRunnables = new LinkedList(); - private Handler handler = null; - private boolean running = false; - private long threadId; - - @Override - public void run() { - Looper.prepare(); - synchronized (looperStartedEvent) { - Log.d(TAG, "Looper thread started."); - handler = new Handler(); - threadId = Thread.currentThread().getId(); - looperStartedEvent.notify(); - } - Looper.loop(); - } - - public synchronized void requestStart() { - if (running) { - return; - } - running = true; - handler = null; - start(); - // Wait for Hander allocation. - synchronized (looperStartedEvent) { - while (handler == null) { - try { - looperStartedEvent.wait(); - } catch (InterruptedException e) { - Log.e(TAG, "Can not start looper thread"); - running = false; - } - } - } - } - - public synchronized void requestStop() { - if (!running) { - return; - } - running = false; - handler.post(new Runnable() { - @Override - public void run() { - handler.getLooper().quit(); - Log.d(TAG, "Looper thread finished."); - } - }); - } - - // Checks if current thread is a looper thread. - public boolean checkOnLooperThread() { - return (Thread.currentThread().getId() == threadId); - } - - public synchronized void scheduleAtFixedRate(final Runnable command, final long periodMillis) { - if (!running) { - Log.w(TAG, "Trying to schedule task for non running executor"); - return; - } - Runnable runnable = new Runnable() { - @Override - public void run() { - if (running) { - command.run(); - if (!handler.postDelayed(this, periodMillis)) { - Log.e(TAG, "Failed to post a delayed runnable in the chain."); - } - } - } - }; - scheduledPeriodicRunnables.add(runnable); - if (!handler.postDelayed(runnable, periodMillis)) { - Log.e(TAG, "Failed to post a delayed runnable."); - } - } - - public synchronized void cancelScheduledTasks() { - if (!running) { - Log.w(TAG, "Trying to cancel schedule tasks for non running executor"); - return; - } - - // Stop scheduled periodic tasks. - for (Runnable r : scheduledPeriodicRunnables) { - handler.removeCallbacks(r); - } - scheduledPeriodicRunnables.clear(); - } - - @Override - public synchronized void execute(final Runnable runnable) { - if (!running) { - Log.w(TAG, "Running looper executor without calling requestStart()"); - return; - } - if (Thread.currentThread().getId() == threadId) { - runnable.run(); - } else { - handler.post(runnable); - } - } - - /** - * Access to the handler for testing purposes. - */ - Handler getHandler() { - return handler; - } -} diff --git a/webrtc/examples/androidjunit/src/org/appspot/apprtc/util/LooperExecutorTest.java b/webrtc/examples/androidjunit/src/org/appspot/apprtc/util/LooperExecutorTest.java deleted file mode 100644 index aecf6b168a..0000000000 --- a/webrtc/examples/androidjunit/src/org/appspot/apprtc/util/LooperExecutorTest.java +++ /dev/null @@ -1,122 +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 static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.robolectric.Robolectric.shadowOf; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.robolectric.RobolectricTestRunner; -import org.robolectric.annotation.Config; -import org.robolectric.shadows.ShadowLooper; - -@RunWith(RobolectricTestRunner.class) -@Config(manifest = Config.NONE) -public class LooperExecutorTest { - private final static int RUN_TIMES = 10; - - @Mock private Runnable mockRunnable; - private LooperExecutor executor; - - @Before - public void setUp() { - MockitoAnnotations.initMocks(this); - executor = new LooperExecutor(); - } - - @After - public void tearDown() { - executor.requestStop(); - executePendingRunnables(); - } - - @Test - public void testExecute() { - executor.requestStart(); - - for (int i = 0; i < RUN_TIMES; i++) { - executor.execute(mockRunnable); - } - - verifyNoMoreInteractions(mockRunnable); - executePendingRunnables(); - verify(mockRunnable, times(RUN_TIMES)).run(); - } - - /** - * Test that runnables executed before requestStart are ignored. - */ - @Test - public void testExecuteBeforeStart() { - executor.execute(mockRunnable); - - executor.requestStart(); - executePendingRunnables(); - - verifyNoMoreInteractions(mockRunnable); - } - - /** - * Test that runnables executed after requestStop are not executed. - */ - @Test - public void testExecuteAfterStop() { - executor.requestStart(); - executor.requestStop(); - - executor.execute(mockRunnable); - executePendingRunnables(); - - verifyNoMoreInteractions(mockRunnable); - } - - /** - * Test multiple requestStart calls are just ignored. - */ - @Test - public void testMultipleStarts() { - executor.requestStart(); - testExecute(); - } - - /** - * Test multiple requestStop calls are just ignored. - */ - @Test - public void testMultipleStops() { - executor.requestStart(); - executor.requestStop(); - executor.requestStop(); - executePendingRunnables(); - } - - /** - * Calls ShadowLooper's idle method in order to execute pending runnables. - */ - private void executePendingRunnables() { - ShadowLooper shadowLooper = getShadowLooper(); - shadowLooper.idle(); - } - - /** - * Get ShadowLooper of the executor thread. - */ - private ShadowLooper getShadowLooper() { - return shadowOf(executor.getHandler().getLooper()); - } -}