Initialization of audio manager for Android is now done on the audio thread

media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java

Index: media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
diff --git a/media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java b/media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
index a60b69018cf97e598308ce0ab34b0eb989ba217f..3182e17f1c7451b6bdf2225a11adcc1585ae7d7f 100644
--- a/media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
+++ b/media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
@@ -32,13 +32,122 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+/*

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

I'm assuming you're going to remove all of this :)

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Done.

+(10:48:47) [336600] # adb logcat -c && adb logcat | grep AudioManagerAndroid
+
+=> In total 4 different threads <=
+
+**** BEFORE CHANGE *****
+
+// Main
+D/AudioManagerAndroid( 7554): @AudioManagerAndroid: main (*)
+D/AudioManagerAndroid( 7554): @init: main (*)
+D/AudioManagerAndroid( 7554): @onReceive: main (*)
+D/AudioManagerAndroid( 7554): @close: main (*)
+
+// IO-thread
+D/AudioManagerAndroid( 7554): @getNativeOutputSampleRate: Thread-227
+D/AudioManagerAndroid( 7554): @isAudioLowLatencySupported: Thread-227
+D/AudioManagerAndroid( 7554): @getAudioLowLatencyOutputFrameSize: Thread-227
+D/AudioManagerAndroid( 7554): @getMinInputFrameSize: Thread-227
+D/AudioManagerAndroid( 7554): @shouldUseAcousticEchoCanceler: Thread-227
+
+// Device thread
+D/AudioManagerAndroid( 7554): @getAudioInputDeviceNames: Thread-238 (*)
+D/AudioManagerAndroid( 7554): @initAudioDeviceList: Thread-238
+D/AudioManagerAndroid( 7554): @getNativeOutputSampleRate: Thread-238
+D/AudioManagerAndroid( 7554): @getMinInputFrameSize: Thread-238
+D/AudioManagerAndroid( 7554): @shouldUseAcousticEchoCanceler: Thread-238
+
+// Audio thread
+D/AudioManagerAndroid( 7554): @setDevice: Thread-239 (*)
+D/AudioManagerAndroid( 7554): @initAudioDeviceList: Thread-239
+D/AudioManagerAndroid( 7554): @setCommunicationAudioModeOn: Thread-239
+
+(*) <=> single thread access only
+
+Accessed by more than one thread:
+
+D/AudioManagerAndroid( 7554): @initAudioDeviceList: Thread-238
+D/AudioManagerAndroid( 7554): @initAudioDeviceList: Thread-239
+
+D/AudioManagerAndroid( 7554): @shouldUseAcousticEchoCanceler: Thread-227
+D/AudioManagerAndroid( 7554): @shouldUseAcousticEchoCanceler: Thread-238
+
+D/AudioManagerAndroid( 7554): @getNativeOutputSampleRate: Thread-227
+D/AudioManagerAndroid( 7554): @getNativeOutputSampleRate: Thread-238
+
+D/AudioManagerAndroid( 7554): @getMinInputFrameSize: Thread-227
+D/AudioManagerAndroid( 7554): @getMinInputFrameSize: Thread-238
+
+**** AFTER CHANGE *****
+
+D/AudioManagerAndroid(25995): @AudioManagerAndroid: main
+D/AudioManagerAndroid(25995): @init: main
+D/AudioManagerAndroid(25995): @onReceive: main
+
+D/AudioManagerAndroid(25995): @getNativeOutputSampleRate: Thread-353
+D/AudioManagerAndroid(25995): @isAudioLowLatencySupported: Thread-353
+D/AudioManagerAndroid(25995): @getAudioLowLatencyOutputFrameSize: Thread-353
+D/AudioManagerAndroid(25995): @getMinInputFrameSize: Thread-353
+D/AudioManagerAndroid(25995): @shouldUseAcousticEchoCanceler: Thread-353
+
+// Audio thread
+D/AudioManagerAndroid(25995): @initAudioDeviceList: Thread-370
+D/AudioManagerAndroid(25995): @getAudioInputDeviceNames: Thread-370
+D/AudioManagerAndroid(25995): @setDevice: Thread-370
+D/AudioManagerAndroid(25995): @setCommunicationAudioModeOn: Thread-370
+
+D/AudioManagerAndroid(25995): @getNativeOutputSampleRate: Thread-371
+D/AudioManagerAndroid(25995): @getMinInputFrameSize: Thread-371
+D/AudioManagerAndroid(25995): @shouldUseAcousticEchoCanceler: Thread-371
+D/AudioManagerAndroid(25995): @getNativeOutputSampleRate: Thread-371
+
+=> initAudioDeviceList is now only accessed on the audio thread (370) and
+   same goes for setDevice.
+*/
+
 @JNINamespace("media")
 class AudioManagerAndroid {
     private static final String TAG = "AudioManagerAndroid";
 
     // Set to true to enable debug logs. Avoid in production builds.
     // NOTE: always check in as false.
-    private static final boolean DEBUG = false;
+    private static final boolean DEBUG = true;
+
+    /**
+     * NonThreadSafe is a helper class used to help verify that methods of a
+     * class are called from the same thread.
+     * Inspired by class in package com.google.android.apps.chrome.utilities.
+     * Is only utilized when DEBUG is set to true.
+     */
+    private static class NonThreadSafe {
+        private Long mThreadId = null;

[tommi (sloooow) - chröme, 2014/01/30 13:41:27]

nit: use 0 instead of null since this is a Long and not a reference.

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Actually, I need 0L here.

+
+        public NonThreadSafe() {
+            if (DEBUG) {
+                ensureThreadIdAssigned();
+            }
+        }
+
+        /**
+         * Checks if the method is called on the valid thread.
+         * Assigns the current thread if no thread was assigned.
+         */
+        public boolean calledOnValidThread() {
+            if (DEBUG) {
+                ensureThreadIdAssigned();
+                return mThreadId.equals(Thread.currentThread().getId());
+            }
+            return true;
+        }
+
+        private void ensureThreadIdAssigned() {
+            if (DEBUG) {
+                if (mThreadId == null) mThreadId = Thread.currentThread().getId();
+            }
+        }
+    }
 
     /** Simple container for device information. */
     private static class AudioDeviceName {
@@ -116,6 +225,7 @@ class AudioManagerAndroid {
     private int mBluetoothScoState = STATE_BLUETOOTH_SCO_INVALID;
 
     private boolean mIsInitialized = false;
+    private boolean mIsAudioListIsInitialized = false;
     private boolean mSavedIsSpeakerphoneOn;
     private boolean mSavedIsMicrophoneMute;
 
@@ -123,6 +233,12 @@ class AudioManagerAndroid {
     // call to setDevice().
     private int mRequestedAudioDevice = DEVICE_INVALID;
 
+    // This class should be created, initialized and closed on the main
+    // Java thread. In addition, BroadcastReceiver.onReceive should also
+    // happen on this thread. We use |mNonThreadSafe| to ensure that this is
+    // the case. Only active when |DEBUG| is set to true.
+    private final NonThreadSafe mNonThreadSafe = new NonThreadSafe();
+
     // Lock to protect |mAudioDevices| and |mRequestedAudioDevice| which can
     // be accessed from the main thread and the audio manager thread.
     private final Object mLock = new Object();
@@ -155,6 +271,7 @@ class AudioManagerAndroid {
     }
 
     private AudioManagerAndroid(Context context, long nativeAudioManagerAndroid) {
+        logd("@AudioManagerAndroid: " + Thread.currentThread().getName());
         mContext = context;
         mNativeAudioManagerAndroid = nativeAudioManagerAndroid;
         mAudioManager = (AudioManager) mContext.getSystemService(Context.AUDIO_SERVICE);
@@ -165,38 +282,27 @@ class AudioManagerAndroid {
      * Saves the initial speakerphone and microphone state.
      * Populates the list of available audio devices and registers receivers
      * for broadcast intents related to wired headset and Bluetooth devices.
+     * TODO(henrika): investigate if it would be possible to move code in
+     * init() to the constructor and code in close() to finalize().
      */
     @CalledByNative
     private void init() {
+        logd("@init: " + Thread.currentThread().getName());
+        assert false;
+        if (!mNonThreadSafe.calledOnValidThread()) {
+            logwtf("init is not called on valid thread!");
+            return;
+        }
         if (DEBUG) logd("init");
         if (mIsInitialized)
             return;
 
-        for (int i = 0; i < DEVICE_COUNT; ++i) {
-            mAudioDevices[i] = false;
-        }
-
-        // Initialize audio device list with things we know is always available.
-        if (hasEarpiece()) {
-            mAudioDevices[DEVICE_EARPIECE] = true;
-        }
-        mAudioDevices[DEVICE_SPEAKERPHONE] = true;
-
-        // Register receivers for broadcast intents related to Bluetooth device
-        // and Bluetooth SCO notifications. Requires BLUETOOTH permission.
-        registerBluetoothIntentsIfNeeded();
-
-        // Register receiver for broadcast intents related to adding/
-        // removing a wired headset (Intent.ACTION_HEADSET_PLUG).
-        registerForWiredHeadsetIntentBroadcast();
-
         mSettingsObserverThread = new HandlerThread("SettingsObserver");
         mSettingsObserverThread.start();
         mSettingsObserver = new SettingsObserver(
             new Handler(mSettingsObserverThread.getLooper()));
 
         mIsInitialized = true;
-        if (DEBUG) reportUpdate();
     }
 
     /**
@@ -205,6 +311,11 @@ class AudioManagerAndroid {
      */
     @CalledByNative
     private void close() {
+        logd("@close: " + Thread.currentThread().getName());
+        if (!mNonThreadSafe.calledOnValidThread()) {
+            logwtf("close is not called on valid thread!");
+            return;
+        }
         if (DEBUG) logd("close");
         if (!mIsInitialized)
             return;
@@ -219,9 +330,6 @@ class AudioManagerAndroid {
         mContentResolver.unregisterContentObserver(mSettingsObserver);
         mSettingsObserver = null;
 
-        unregisterForWiredHeadsetIntentBroadcast();
-        unregisterBluetoothIntentsIfNeeded();
-
         mIsInitialized = false;
     }
 
@@ -232,6 +340,7 @@ class AudioManagerAndroid {
      */
     @CalledByNative
     private void setCommunicationAudioModeOn(boolean on) {
+        logd("@setCommunicationAudioModeOn: " + Thread.currentThread().getName());
         if (DEBUG) logd("setCommunicationAudioModeOn(" + on + ")");
 
         if (on) {
@@ -291,7 +400,9 @@ class AudioManagerAndroid {
      */
     @CalledByNative
     private boolean setDevice(String deviceId) {
+        logd("@setDevice: " + Thread.currentThread().getName());
         if (DEBUG) logd("setDevice: " + deviceId);
+
         int intDeviceId = deviceId.isEmpty() ?
             DEVICE_DEFAULT : Integer.parseInt(deviceId);
 
@@ -326,6 +437,8 @@ class AudioManagerAndroid {
      */
     @CalledByNative
     private AudioDeviceName[] getAudioInputDeviceNames() {
+        logd("@getAudioInputDeviceNames: " + Thread.currentThread().getName());
+        if (DEBUG) logd("getAudioInputDeviceNames");
         boolean devices[] = null;
         synchronized (mLock) {
             devices = mAudioDevices.clone();
@@ -347,6 +460,7 @@ class AudioManagerAndroid {
 
     @CalledByNative
     private int getNativeOutputSampleRate() {
+        logd("@getNativeOutputSampleRate: " + Thread.currentThread().getName());
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
             String sampleRateString = mAudioManager.getProperty(
                     AudioManager.PROPERTY_OUTPUT_SAMPLE_RATE);
@@ -365,6 +479,7 @@ class AudioManagerAndroid {
    */
     @CalledByNative
     private static int getMinInputFrameSize(int sampleRate, int channels) {
+        logd("@getMinInputFrameSize: " + Thread.currentThread().getName());
         int channelConfig;
         if (channels == 1) {
             channelConfig = AudioFormat.CHANNEL_IN_MONO;
@@ -385,6 +500,7 @@ class AudioManagerAndroid {
    */
     @CalledByNative
     private static int getMinOutputFrameSize(int sampleRate, int channels) {
+        logd("@getMinOutputFrameSize: " + Thread.currentThread().getName());
         int channelConfig;
         if (channels == 1) {
             channelConfig = AudioFormat.CHANNEL_OUT_MONO;
@@ -399,12 +515,14 @@ class AudioManagerAndroid {
 
     @CalledByNative
     private boolean isAudioLowLatencySupported() {
+        logd("@isAudioLowLatencySupported: " + Thread.currentThread().getName());
         return mContext.getPackageManager().hasSystemFeature(
                 PackageManager.FEATURE_AUDIO_LOW_LATENCY);
     }
 
     @CalledByNative
     private int getAudioLowLatencyOutputFrameSize() {
+        logd("@getAudioLowLatencyOutputFrameSize: " + Thread.currentThread().getName());
         String framesPerBuffer =
                 mAudioManager.getProperty(AudioManager.PROPERTY_OUTPUT_FRAMES_PER_BUFFER);
         return (framesPerBuffer == null ?
@@ -412,7 +530,8 @@ class AudioManagerAndroid {
     }
 
     @CalledByNative
-    public static boolean shouldUseAcousticEchoCanceler() {
+    private static boolean shouldUseAcousticEchoCanceler() {
+        logd("@shouldUseAcousticEchoCanceler: " + Thread.currentThread().getName());
         // AcousticEchoCanceler was added in API level 16 (Jelly Bean).
         // Next is a list of device models which have been vetted for good
         // quality platform echo cancellation.
@@ -429,6 +548,69 @@ class AudioManagerAndroid {
     }
 
     /**
+     * Sets up the initial list of audio devices and registers intents to be
+     * able to receive notifications about added or removed devices.
+     * This method should only be called once and not during construction to
+     * avoid accessing any audio hardware.
+     * Locks are not needed here since we know that all calls will be done on
+     * the audio manager thread given the design of the C++ version of this
+     * class.
+     */
+    @CalledByNative
+    private void initAudioDeviceList() {
+        logd("@initAudioDeviceList: " + Thread.currentThread().getName());
+        if (DEBUG) logd("initAudioDeviceList");
+        if (mIsAudioListIsInitialized) {

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

no {} needed

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Done.

+            return;
+        }
+        for (int i = 0; i < DEVICE_COUNT; ++i) {

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

ditto.

Also ping on this loop being unnecessary :)

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Done.

+            mAudioDevices[i] = false;
+        }
+
+        // Initialize audio device list with things we know is always available.
+        mAudioDevices[DEVICE_EARPIECE] = hasEarpiece();
+        mAudioDevices[DEVICE_WIRED_HEADSET] = hasWiredHeadset();
+        mAudioDevices[DEVICE_SPEAKERPHONE] = true;
+
+        // Register receivers for broadcast intents related to Bluetooth device
+        // and Bluetooth SCO notifications. Requires BLUETOOTH permission.
+        registerBluetoothIntentsIfNeeded();
+
+        // Register receiver for broadcast intents related to adding/
+        // removing a wired headset (Intent.ACTION_HEADSET_PLUG).
+        registerForWiredHeadsetIntentBroadcast();
+
+        mIsAudioListIsInitialized = true;
+
+        if (DEBUG) reportUpdate();
+    }
+
+    /**
+     * Unregisters receivers for broadcasted events related to changes in
+     * audio devices and clears the list of available devices.
+     * Locks are not needed here since we know that all calls will be done on
+     * the audio manager thread given the design of the C++ version of this
+     * class.
+     */
+    @CalledByNative
+    private void closeAudioDeviceList() {
+        logd("@closeAudioDeviceList: " + Thread.currentThread().getName());
+        if (DEBUG) logd("closeAudioDeviceList");
+        if (!mIsAudioListIsInitialized) {

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

no {}

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Done.

+            return;
+        }
+
+        unregisterForWiredHeadsetIntentBroadcast();
+        unregisterBluetoothIntentsIfNeeded();
+
+        for (int i = 0; i < DEVICE_COUNT; ++i) {

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

no {}

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Removed all of it.

+            mAudioDevices[i] = false;
+        }
+
+        mIsAudioListIsInitialized = false;
+    }
+
+    /**
      * Register for BT intents if we have the BLUETOOTH permission.
      * Also extends the list of available devices with a BT device if one exists.
      */
@@ -445,9 +627,7 @@ class AudioManagerAndroid {
         if (!mHasBluetoothPermission) {
             return;
         }
-        if (hasBluetoothHeadset()) {
-            mAudioDevices[DEVICE_BLUETOOTH_HEADSET] = true;
-        }
+        mAudioDevices[DEVICE_BLUETOOTH_HEADSET] = hasBluetoothHeadset();
 
         // Register receivers for broadcast intents related to changes in
         // Bluetooth headset availability and usage of the SCO channel.
@@ -487,12 +667,24 @@ class AudioManagerAndroid {
         return mAudioManager.isMicrophoneMute();
     }
 
-    /** Gets the current earpice state. */
+    /** Gets the current earpiece state. */
     private boolean hasEarpiece() {
         return mContext.getPackageManager().hasSystemFeature(
             PackageManager.FEATURE_TELEPHONY);
     }
 
+    /**
+     * Checks whether a wired headset is connected or not.
+     * This is not a valid indication that audio playback is actually over
+     * the wired headset as audio routing depends on other conditions. We
+     * only use it as an early indicator (during initialization) of an attached
+     * wired headset.
+     */
+    @Deprecated
+    private boolean hasWiredHeadset() {

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

Does this have to be a method? Why not just call
mAudioManager.isWiredHeadsetOn() where you need it?
Is it do reduce the scope of @Deprecated? (If so, sgtm).

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

Correct, I wanted to make it more clear that it was this API only that was
deprecated. Keeping, thanks.

+        return mAudioManager.isWiredHeadsetOn();
+    }
+
     /** Checks if the process has BLUETOOTH permission or not. */
     private boolean hasBluetoothPermission() {
         boolean hasBluetooth = mContext.checkPermission(
@@ -559,6 +751,11 @@ class AudioManagerAndroid {
 
             @Override
             public void onReceive(Context context, Intent intent) {
+                logd("@onReceive: " + Thread.currentThread().getName());
+                if (!mNonThreadSafe.calledOnValidThread()) {
+                    logwtf("onReceive is not called on valid thread!");
+                    return;

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

nit: Is the 'return' statement necessary?  I think logwtf will basically throw
an exception.

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

I have not been able to verify that actually. Also, think you asked to a return
in a previous CL. There are examples of both styles in Chrome today. Most add
return. 

OK with keeping?

[tommi (sloooow) - chröme, 2014/01/30 15:03:31]

OK.

+                }
                 int state = intent.getIntExtra("state", STATE_UNPLUGGED);
                 if (DEBUG) {
                     int microphone = intent.getIntExtra("microphone", HAS_NO_MIC);
@@ -627,6 +824,10 @@ class AudioManagerAndroid {
         mBluetoothHeadsetReceiver = new BroadcastReceiver() {
             @Override
             public void onReceive(Context context, Intent intent) {
+                if (!mNonThreadSafe.calledOnValidThread()) {
+                    logwtf("onReceive is not called on valid thread!");
+                    return;

[tommi (sloooow) - chröme, 2014/01/30 12:51:11]

same here (and if you remove the return, also remove {})

[henrika (OOO until Aug 14), 2014/01/30 14:48:24]

See comment above.

+                }
                 // A change in connection state of the Headset profile has
                 // been detected, e.g. BT headset has been connected or
                 // disconnected. This broadcast is *not* sticky.