Remove the synchronization
All of the callbacks happen on the same thread; no need to
synchronize. Also move away from TimerTask and use Handler to post
the termination callback.
Change-Id: If684e29686f2c7557cc56057c7f1ec6a1dfd3bcb
diff --git a/src/com/android/telecomm/CallServiceFinder.java b/src/com/android/telecomm/CallServiceFinder.java
index 43e2485..fa5ee14 100644
--- a/src/com/android/telecomm/CallServiceFinder.java
+++ b/src/com/android/telecomm/CallServiceFinder.java
@@ -23,8 +23,9 @@
import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo;
import android.content.pm.ServiceInfo;
+import android.os.Handler;
import android.os.IBinder;
-import android.os.Bundle;
+import android.os.Looper;
import android.os.RemoteException;
import android.telecomm.ICallService;
import android.telecomm.ICallServiceLookupResponse;
@@ -37,8 +38,6 @@
import java.util.List;
import java.util.Set;
-import java.util.Timer;
-import java.util.TimerTask;
/**
* Finds {@link ICallService} and {@link ICallServiceProvider} implementations on the device.
@@ -111,49 +110,40 @@
* @param registrar The registrar with which to register and unregister this provider.
*/
ProviderWrapper(Context context, final ProviderRegistrar registrar) {
- ComponentName name = registrar.getProviderName();
- Preconditions.checkNotNull(name);
- Preconditions.checkNotNull(context);
+ ComponentName name = registrar.getProviderName();
+ Preconditions.checkNotNull(name);
+ Preconditions.checkNotNull(context);
- Intent serviceIntent = new Intent(CALL_SERVICE_PROVIDER_CLASS_NAME).setComponent(name);
- Log.i(TAG, "Binding to ICallServiceProvider through " + serviceIntent);
+ Intent serviceIntent = new Intent(CALL_SERVICE_PROVIDER_CLASS_NAME).setComponent(name);
+ Log.i(TAG, "Binding to ICallServiceProvider through " + serviceIntent);
- // Connection object for the service binding.
- ServiceConnection connection = new ServiceConnection() {
- @Override
- public void onServiceConnected(ComponentName className, IBinder service) {
- registrar.register(ICallServiceProvider.Stub.asInterface(service));
- }
+ // Connection object for the service binding.
+ ServiceConnection connection = new ServiceConnection() {
+ @Override
+ public void onServiceConnected(ComponentName className, IBinder service) {
+ registrar.register(ICallServiceProvider.Stub.asInterface(service));
+ }
- @Override
- public void onServiceDisconnected(ComponentName className) {
- registrar.unregister();
- }
- };
+ @Override
+ public void onServiceDisconnected(ComponentName className) {
+ registrar.unregister();
+ }
+ };
- if (!context.bindService(serviceIntent, connection, Context.BIND_AUTO_CREATE)) {
- // TODO(santoscordon): Handle error.
- }
- }
- }
-
- /**
- * A timer task to ensure each lookup cycle is time-bound, see LOOKUP_TIMEOUT.
- */
- private class LookupTerminator extends TimerTask {
- @Override
- public void run() {
- terminateLookup();
+ if (!context.bindService(serviceIntent, connection, Context.BIND_AUTO_CREATE)) {
+ // TODO(santoscordon): Handle error.
+ }
}
}
private static final String TAG = CallServiceFinder.class.getSimpleName();
/**
- * The longest period in milliseconds each lookup cycle is allowed to span over, see mTimer.
+ * The longest period in milliseconds each lookup cycle is allowed to span over, see
+ * {@link #mLookupTerminator}.
* TODO(gilad): Likely requires tuning.
*/
- private static final int LOOKUP_TIMEOUT = 100;
+ private static final int LOOKUP_TIMEOUT_MS = 100;
/**
* Used to retrieve all known ICallServiceProvider implementations from the framework.
@@ -195,7 +185,15 @@
* Used to interrupt lookup cycles that didn't terminate naturally within the allowed
* period, see LOOKUP_TIMEOUT.
*/
- private Timer mTimer;
+ private final Runnable mLookupTerminator = new Runnable() {
+ @Override
+ public void run() {
+ terminateLookup();
+ }
+ };
+
+ /** Used to run code (e.g. messages, Runnables) on the main (UI) thread. */
+ private final Handler mHandler = new Handler(Looper.getMainLooper());
/**
* Persists the specified parameters.
@@ -207,12 +205,13 @@
}
/**
- * Initiates a lookup cycle for call-service providers.
+ * Initiates a lookup cycle for call-service providers. Must be called from the UI thread.
* TODO(gilad): Expand this comment to describe the lookup flow in more detail.
*
* @param context The relevant application context.
*/
- synchronized void initiateLookup(Context context) {
+ void initiateLookup(Context context) {
+ ThreadUtil.checkOnMainThread();
if (mIsLookupInProgress) {
// At most one active lookup is allowed at any given time, bail out.
return;
@@ -250,14 +249,9 @@
// back to the switchboard.
updateSwitchboard();
} else {
- // Start the timeout for this lookup cycle.
- // TODO(gilad): Consider reusing the same timer instead of creating new ones.
- if (mTimer != null) {
- // Shouldn't be running but better safe than sorry.
- mTimer.cancel();
- }
- mTimer = new Timer();
- mTimer.schedule(new LookupTerminator(), LOOKUP_TIMEOUT);
+ // Schedule a lookup terminator to run after LOOKUP_TIMEOUT_MS milliseconds.
+ mHandler.removeCallbacks(mLookupTerminator);
+ mHandler.postDelayed(mLookupTerminator, LOOKUP_TIMEOUT_MS);
}
}
@@ -361,9 +355,7 @@
* Timeouts the current lookup cycle, see LookupTerminator.
*/
private void terminateLookup() {
- if (mTimer != null) {
- mTimer.cancel(); // Terminate the timer thread.
- }
+ mHandler.removeCallbacks(mLookupTerminator);
updateSwitchboard();
mIsLookupInProgress = false;
@@ -374,9 +366,8 @@
* to call-service providers).
*/
private void updateSwitchboard() {
- synchronized (mProviderRegistry) {
- // TODO(gilad): More here.
- mSwitchboard.setCallServices(null);
- }
+ ThreadUtil.checkOnMainThread();
+ // TODO(gilad): More here.
+ mSwitchboard.setCallServices(null);
}
}
diff --git a/src/com/android/telecomm/Switchboard.java b/src/com/android/telecomm/Switchboard.java
index cc01941..e9f9033 100644
--- a/src/com/android/telecomm/Switchboard.java
+++ b/src/com/android/telecomm/Switchboard.java
@@ -56,9 +56,8 @@
* @param context The application context.
*/
void placeOutgoingCall(String handle, ContactInfo contactInfo, Context context) {
- synchronized (mPendingOutgoingCalls) {
- mPendingOutgoingCalls.add(new Call(handle, contactInfo));
- }
+ ThreadUtil.checkOnMainThread();
+ mPendingOutgoingCalls.add(new Call(handle, contactInfo));
callServiceFinder.initiateLookup(context);
}
@@ -73,23 +72,22 @@
* call-services, it should be at liberty to use these just as well.
*/
void setCallServices(List<ICallService> callServices) {
- synchronized (mPendingOutgoingCalls) {
- for (Call pendingCall : mPendingOutgoingCalls) {
- // TODO(gilad): Iterate through the prioritized list of switchboard policies passing
- // to each policy the call object as well as all known call services. Break out of
- // the inner/policy loop as soon as the first matching policy for the call is found.
- // Calls for which no matching policy can be found will be killed by cleanup/monitor
- // thread, see the "monitor" to-do at the top of the file.
+ ThreadUtil.checkOnMainThread();
+ for (Call pendingCall : mPendingOutgoingCalls) {
+ // TODO(gilad): Iterate through the prioritized list of switchboard policies passing
+ // to each policy the call object as well as all known call services. Break out of
+ // the inner/policy loop as soon as the first matching policy for the call is found.
+ // Calls for which no matching policy can be found will be killed by cleanup/monitor
+ // thread, see the "monitor" to-do at the top of the file.
- // Psuedo code (assuming connect to be a future switchboard method):
- //
- // FOR policy IN prioritizedPolicies:
- // IF policy.is_applicable_to(pendingCall, callServices):
- // TRY
- // connect(pendingCall, callServices, policy)
- // mPendingOutgoingCalls.remove(pendingCall)
- // BREAK
- }
+ // Psuedo code (assuming connect to be a future switchboard method):
+ //
+ // FOR policy IN prioritizedPolicies:
+ // IF policy.is_applicable_to(pendingCall, callServices):
+ // TRY
+ // connect(pendingCall, callServices, policy)
+ // mPendingOutgoingCalls.remove(pendingCall)
+ // BREAK
}
}
diff --git a/src/com/android/telecomm/ThreadUtil.java b/src/com/android/telecomm/ThreadUtil.java
new file mode 100644
index 0000000..b826f58
--- /dev/null
+++ b/src/com/android/telecomm/ThreadUtil.java
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2014, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.telecomm;
+
+import android.os.Looper;
+import android.util.Log;
+
+/**
+ * A utility class which helps organize callers' threads. This class cannot be instantiated; callers
+ * should use the static methods.
+ */
+public final class ThreadUtil {
+ private static final String TAG = ThreadUtil.class.getSimpleName();
+
+ private ThreadUtil() {}
+
+ /** @return whether this method is being called from the main (UI) thread. */
+ public static boolean isOnMainThread() {
+ return Looper.getMainLooper() == Looper.myLooper();
+ }
+
+ /**
+ * Checks that this is being executed on the main thread. If not, a message is logged at
+ * WTF-level priority.
+ */
+ public static void checkOnMainThread() {
+ if (!isOnMainThread()) {
+ Log.wtf(TAG, "Must be on the main thread!", new IllegalStateException());
+ }
+ }
+
+ /**
+ * Checks that this is not being executed on the main thread. If so, a message is logged at
+ * WTF-level priority.
+ */
+ public static void checkNotOnMainThread() {
+ if (isOnMainThread()) {
+ Log.wtf(TAG, "Must not be on the main thread!", new IllegalStateException());
+ }
+ }
+}