Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5507671b authored by Hall Liu's avatar Hall Liu
Browse files

Redo DtmfLocalTonePlayer

The system of locks in DtmfLocalTonePlayer and the constant thread
creation/destruction seems to be behind a system ANR bug. This change
refactors the class to rely on a single Handler and associated
HandlerThread that lives for the entire duration of Telecom.

Bug: 34886553
Test: manual and unit
Change-Id: I1d86c644aaecdb6ff93d31e2b9e97792e3e301e0
parent 4e068b34
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -1183,7 +1183,7 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable {
     *
     * @return The {@link Context}.
     */
    Context getContext() {
    public Context getContext() {
        return mContext;
    }

+2 −1
Original line number Diff line number Diff line
@@ -261,7 +261,8 @@ public class CallsManager extends Call.ListenerBase
        mCallerInfoLookupHelper = new CallerInfoLookupHelper(context, mCallerInfoAsyncQueryFactory,
                mContactsAsyncHelper, mLock);

        mDtmfLocalTonePlayer = new DtmfLocalTonePlayer();
        mDtmfLocalTonePlayer =
                new DtmfLocalTonePlayer(new DtmfLocalTonePlayer.ToneGeneratorProxy());
        mNotificationManager = (NotificationManager) context.getSystemService(
                Context.NOTIFICATION_SERVICE);
        CallAudioRouteStateMachine callAudioRouteStateMachine = new CallAudioRouteStateMachine(
+116 −92
Original line number Diff line number Diff line
@@ -21,10 +21,13 @@ import android.media.AudioManager;
import android.media.ToneGenerator;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.Looper;
import android.os.Message;
import android.provider.Settings;
import android.telecom.Log;
import android.telecom.Logging.Session;

import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;

// TODO: Needed for move to system service: import com.android.internal.R;
@@ -36,23 +39,108 @@ import com.android.internal.util.Preconditions;
 * changes.
 */
public class DtmfLocalTonePlayer {
    public static class ToneGeneratorProxy {
        /** Generator used to actually play the tone. */
        private ToneGenerator mToneGenerator;

        public void create() {
            if (mToneGenerator == null) {
                try {
                    mToneGenerator = new ToneGenerator(AudioManager.STREAM_DTMF, 80);
                } catch (RuntimeException e) {
                    Log.e(this, e, "Error creating local tone generator.");
                    mToneGenerator = null;
                }
            }
        }

        public void release() {
            if (mToneGenerator != null) {
                mToneGenerator.release();
                mToneGenerator = null;
            }
        }

        public boolean isPresent() {
            return mToneGenerator != null;
        }

        public void startTone(int toneType, int durationMs) {
            if (mToneGenerator != null) {
                mToneGenerator.startTone(toneType, durationMs);
            }
        }

        public void stopTone() {
            if (mToneGenerator != null) {
                mToneGenerator.stopTone();
            }
        }
    }

    private final class ToneHandler extends Handler {

        public ToneHandler(Looper looper) {
            super(looper);
        }

        @Override
        public void handleMessage(Message msg) {
            if (msg.obj instanceof Session) {
                Log.continueSession((Session) msg.obj, "DLTP.TH");
            }

            switch(msg.what) {
                case EVENT_START_SESSION:
                    mToneGeneratorProxy.create();
                    break;
                case EVENT_END_SESSION:
                    mToneGeneratorProxy.release();
                    break;
                case EVENT_PLAY_TONE:
                    char c = (char) msg.arg1;
                    if (!mToneGeneratorProxy.isPresent()) {
                        Log.d(this, "playTone: no tone generator, %c.", c);
                    } else {
                        Log.d(this, "starting local tone: %c.", c);
                        int tone = getMappedTone(c);
                        if (tone != ToneGenerator.TONE_UNKNOWN) {
                            mToneGeneratorProxy.startTone(tone, -1 /* toneDuration */);
                        }
                    }
                    break;
                case EVENT_STOP_TONE:
                    if (mToneGeneratorProxy.isPresent()) {
                        mToneGeneratorProxy.stopTone();
                    }
                    break;
                default:
                    Log.w(this, "Unknown message: %d", msg.what);
                    break;
            }
        }
    }

    /** The current call associated with an existing dtmf session. */
    private Call mCall;

    /**
     * Message codes to be used for creating and deleting ToneGenerator object in the tonegenerator
     * thread.
     * thread, as well as for actually playing the tones.
     */
    private static final int EVENT_CREATE_OBJECT = 1;
    private static final int EVENT_DELETE_OBJECT = 2;
    private static final int EVENT_START_SESSION = 1;
    private static final int EVENT_END_SESSION = 2;
    private static final int EVENT_PLAY_TONE = 3;
    private static final int EVENT_STOP_TONE = 4;

    /** Handler running on the tonegenerator thread. */
    private Handler mHandler;
    private ToneHandler mHandler;

    public DtmfLocalTonePlayer() { }
    private final ToneGeneratorProxy mToneGeneratorProxy;

    public DtmfLocalTonePlayer(ToneGeneratorProxy toneGeneratorProxy) {
        mToneGeneratorProxy = toneGeneratorProxy;
    }

    public void onForegroundCallChanged(Call oldForegroundCall, Call newForegroundCall) {
        endDtmfSession(oldForegroundCall);
@@ -65,22 +153,14 @@ public class DtmfLocalTonePlayer {
     * @param call The associated call.
     * @param c The digit to play.
     */
    void playTone(Call call, char c) {
    public void playTone(Call call, char c) {
        // Do nothing if it is not the right call.
        if (mCall != call) {
            return;
        }
        synchronized(this) {
            if (mToneGenerator == null) {
                Log.d(this, "playTone: mToneGenerator == null, %c.", c);
            } else {
                Log.d(this, "starting local tone: %c.", c);
                int tone = getMappedTone(c);
                if (tone != ToneGenerator.TONE_UNKNOWN) {
                    mToneGenerator.startTone(tone, -1 /* toneDuration */);
                }
            }
        }

        getHandler().sendMessage(
                getHandler().obtainMessage(EVENT_PLAY_TONE, (int) c, 0, Log.createSubsession()));
    }

    /**
@@ -88,19 +168,14 @@ public class DtmfLocalTonePlayer {
     *
     * @param call The associated call.
     */
    void stopTone(Call call) {
    public void stopTone(Call call) {
        // Do nothing if it's not the right call.
        if (mCall != call) {
            return;
        }
        synchronized(this) {
            if (mToneGenerator == null) {
                Log.d(this, "stopTone: mToneGenerator == null.");
            } else {
                Log.d(this, "stopping local tone.");
                mToneGenerator.stopTone();
            }
        }

        getHandler().sendMessage(
                getHandler().obtainMessage(EVENT_STOP_TONE, Log.createSubsession()));
    }

    /**
@@ -125,7 +200,8 @@ public class DtmfLocalTonePlayer {

        if (areLocalTonesEnabled) {
            Log.d(this, "Posting create.");
            postMessage(EVENT_CREATE_OBJECT);
            getHandler().sendMessage(
                    getHandler().obtainMessage(EVENT_START_SESSION, Log.createSubsession()));
        }
    }

@@ -141,75 +217,23 @@ public class DtmfLocalTonePlayer {

            mCall = null;
            Log.d(this, "Posting delete.");
            postMessage(EVENT_DELETE_OBJECT);
            getHandler().sendMessage(
                    getHandler().obtainMessage(EVENT_END_SESSION, Log.createSubsession()));
        }
    }

    /**
     * Posts a message to the tonegenerator-thread handler. Creates the handler if the handler
     * has not been instantiated.
     *
     * @param messageCode The message to post.
     * Creates a new ToneHandler on a separate thread if none exists, and returns it.
     * No need for locking, since everything that calls this is protected by the Telecom lock.
     */
    private void postMessage(int messageCode) {
        synchronized(this) {
            if (mHandler == null) {
                mHandler = getNewHandler();
            }

    @VisibleForTesting
    public ToneHandler getHandler() {
        if (mHandler == null) {
                Log.d(this, "Message %d skipped because there is no handler.", messageCode);
            } else {
                mHandler.obtainMessage(messageCode, null).sendToTarget();
            }
        }
    }

    /**
     * Creates a new tonegenerator Handler running in its own thread.
     */
    private Handler getNewHandler() {
        Preconditions.checkState(mHandler == null);

            HandlerThread thread = new HandlerThread("tonegenerator-dtmf");
            thread.start();

        return new Handler(thread.getLooper()) {
            @Override
            public void handleMessage(Message msg) {
                switch(msg.what) {
                    case EVENT_CREATE_OBJECT:
                        synchronized(DtmfLocalTonePlayer.this) {
                            if (mToneGenerator == null) {
                                try {
                                    mToneGenerator = new ToneGenerator(
                                            AudioManager.STREAM_DTMF, 80);
                                } catch (RuntimeException e) {
                                    Log.e(this, e, "Error creating local tone generator.");
                                    mToneGenerator = null;
                                }
                            }
                        }
                        break;
                    case EVENT_DELETE_OBJECT:
                        synchronized(DtmfLocalTonePlayer.this) {
                            if (mToneGenerator != null) {
                                mToneGenerator.release();
                                mToneGenerator = null;
                            }
                            // Delete the handler after the tone generator object is deleted by
                            // the tonegenerator thread.
                            if (mHandler != null && !mHandler.hasMessages(EVENT_CREATE_OBJECT)) {
                                // Stop the handler only if there are no pending CREATE messages.
                                mHandler.removeMessages(EVENT_DELETE_OBJECT);
                                mHandler.getLooper().quitSafely();
                                mHandler = null;
                            }
                        }
                        break;
                }
            mHandler = new ToneHandler(thread.getLooper());
        }
        };
        return mHandler;
    }

    private static int getMappedTone(char digit) {
+115 −0
Original line number Diff line number Diff line
/* * Copyright (C) 2017 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.server.telecom.tests;

import android.media.AudioManager;
import android.media.ToneGenerator;
import android.test.suitebuilder.annotation.SmallTest;

import com.android.server.telecom.Call;
import com.android.server.telecom.DtmfLocalTonePlayer;
import com.android.server.telecom.R;

import org.mockito.Mock;

import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class DtmfLocalTonePlayerTest extends TelecomTestCase {
    private static final int TIMEOUT = 2000;
    @Mock DtmfLocalTonePlayer.ToneGeneratorProxy mToneProxy;
    @Mock Call mCall;

    DtmfLocalTonePlayer mPlayer;

    public void setUp() throws Exception {
        super.setUp();
        mContext = mComponentContextFixture.getTestDouble().getApplicationContext();
        mPlayer = new DtmfLocalTonePlayer(mToneProxy);
        when(mCall.getContext()).thenReturn(mContext);
    }

    @SmallTest
    public void testSupportedStart() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(true);
        when(mToneProxy.isPresent()).thenReturn(true);
        mPlayer.onForegroundCallChanged(null, mCall);
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy).create();
    }

    @SmallTest
    public void testUnsupportedStart() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(false);
        when(mToneProxy.isPresent()).thenReturn(true);
        mPlayer.onForegroundCallChanged(null, mCall);
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy, never()).create();
    }

    @SmallTest
    public void testPlayToneWhenUninitialized() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(false);
        when(mToneProxy.isPresent()).thenReturn(false);
        mPlayer.onForegroundCallChanged(null, mCall);
        mPlayer.playTone(mCall, '9');
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy, never()).startTone(anyInt(), anyInt());
    }

    @SmallTest
    public void testPlayToneWhenInitialized() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(true);
        when(mToneProxy.isPresent()).thenReturn(true);
        mPlayer.onForegroundCallChanged(null, mCall);
        mPlayer.playTone(mCall, '9');
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy).startTone(eq(ToneGenerator.TONE_DTMF_9), eq(-1));
    }

    @SmallTest
    public void testStopToneWhenUninitialized() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(false);
        when(mToneProxy.isPresent()).thenReturn(false);
        mPlayer.onForegroundCallChanged(null, mCall);
        mPlayer.stopTone(mCall);
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy, never()).stopTone();
    }

    @SmallTest
    public void testStopToneWhenInitialized() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(true);
        when(mToneProxy.isPresent()).thenReturn(true);
        mPlayer.onForegroundCallChanged(null, mCall);
        mPlayer.stopTone(mCall);
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy).stopTone();
    }

    @SmallTest
    public void testProperTeardown() {
        when(mContext.getResources().getBoolean(R.bool.allow_local_dtmf_tones)).thenReturn(true);
        when(mToneProxy.isPresent()).thenReturn(true);
        mPlayer.onForegroundCallChanged(null, mCall);
        mPlayer.onForegroundCallChanged(mCall, null);
        waitForHandlerAction(mPlayer.getHandler(), TIMEOUT);
        verify(mToneProxy).release();
    }
}