Add speaker on/off control on Android for WebRTC

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

Index: media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
===================================================================
--- media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java	(revision 189519)
+++ media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java	(working copy)
@@ -4,20 +4,119 @@
 
 package org.chromium.media;
 
+import android.content.BroadcastReceiver;
 import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
 import android.media.AudioManager;
+import android.telephony.TelephonyManager;
+import android.util.Log;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 
 @JNINamespace("media")
 class AudioManagerAndroid {
+    private static final String TAG = AudioManagerAndroid.class.getSimpleName();
+
+    private BroadcastReceiver mReceiver = null;
+    private Context mContext = null;

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

The Java language does an implicit =null for you; these aren't necessary.

[leozwang1, 2013/03/21 23:10:07]

Done.

+
+    private boolean mIsSpeakerOn;
+
+    private static final int kPhoneTypeError = -1;

[qinmin, 2013/03/21 21:19:38]

java don't use this style. constants should be defined as PHONE_TYPE_ERROR. same
for everything below.

[leozwang1, 2013/03/21 23:10:07]

Done.

+    private static final int kPhoneTypeNone = 0;
+    private static final int kPhoneTypePhone = 1;

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

"phonetypephone"?  Really?

[leozwang1, 2013/03/21 23:10:07]

Done.

[leozwang1, 2013/03/21 23:10:07]

these are removed.

+
+    private static final int kPlugged = 1;
+    private static final int kUnplugged = 0;
+
     @CalledByNative
-    public static void setMode(Context context, int mode) {
-        AudioManager audioManager =
-                (AudioManager)context.getSystemService(Context.AUDIO_SERVICE);
+    public void setMode(int mode) {
+        AudioManager audioManager = getAudioManager();
         if (null != audioManager) {
             audioManager.setMode(mode);
         }
     }
+
+    @CalledByNative
+    private static AudioManagerAndroid createAudioManagerAndroid(Context context) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

Why not just use the ctor?  Are there auto-binding implications?

[leozwang1, 2013/03/21 23:10:07]

ctor binding doesn't exist in auto-generated jni file.

+        return new AudioManagerAndroid(context);
+    }
+
+    private AudioManagerAndroid(Context context) {
+        mContext = context;
+    }
+
+    @CalledByNative
+    public void registerHeadsetReceiver() {
+        AudioManager audioManager = getAudioManager();

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

If this was final you could drop the copy/pasta at l.67.
(unless it's possible for getAudioManager()'s return value to change between
calls; is that possible?)

[leozwang1, 2013/03/21 23:10:07]

Done.

+        if (null == audioManager) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

please reverse these comparisons to have the constant on the right.

[leozwang1, 2013/03/21 23:10:07]

Done.

+            return;

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

When can this be true?

+        }
+        mIsSpeakerOn = audioManager.isSpeakerphoneOn();

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

mOriginalSpeakerStatus ?

[leozwang1, 2013/03/21 23:10:07]

Done.

+
+        IntentFilter filter = new IntentFilter();
+        filter.addAction(Intent.ACTION_HEADSET_PLUG);
+
+        mReceiver = new BroadcastReceiver() {
+            @Override
+                public void onReceive(Context context, Intent intent) {
+                    if (Intent.ACTION_HEADSET_PLUG.equals(intent.getAction())) {
+                        int state = intent.getIntExtra("state", 0);

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

s/0/kUnplugged/

[leozwang1, 2013/03/21 23:10:07]

Done.

[leozwang1, 2013/03/21 23:10:07]

Done.

+                        AudioManager audioManager = getAudioManager();
+                        if (null == audioManager ||
+                            AudioManager.MODE_IN_COMMUNICATION != audioManager.getMode()) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

How can this fail to be true?

+                            return;
+                        }
+                        int ep = hasEarpiece();
+                        if (kPhoneTypeError == ep) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

why isn't this using a Java enum with a switch statement?

[leozwang1, 2013/03/21 23:10:07]

Done.

+                            return;
+                        }
+                        // state 0 for unplugged, 1 for plugged.
+                        if (kPlugged == state && kPhoneTypePhone == ep) {
+                            audioManager.setSpeakerphoneOn(false);
+                        } else {
+                            audioManager.setSpeakerphoneOn(true);

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

l.72-81 can be summarized as:
audioManager.setSpeakerphoneOn(state != kPlugged || ep != kPhoneTypePhone);

+                        }
+                    }
+                }
+            };
+        if (null == mReceiver) {

[nilesh, 2013/03/21 21:02:48]

Why would mReceiver be null here. You assign it on the line above.
What I meant was to do some kind of checking if mReceiver is not null and
registerHeadsetReceiver is called.

[leozwang1, 2013/03/21 23:10:07]

Done.

+            return;
+        }
+        mContext.registerReceiver(mReceiver, filter);
+    }
+
+    @CalledByNative
+    public void unregisterHeadsetReceiver() {
+        if (mReceiver == null) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

how can this be true?

[leozwang1, 2013/03/21 23:10:07]

Done.

+            return;
+        }
+        mContext.unregisterReceiver(mReceiver);
+        mReceiver = null;
+
+        AudioManager audioManager =
+                (AudioManager)mContext.getSystemService(Context.AUDIO_SERVICE);

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

use getAudioManager()

[leozwang1, 2013/03/21 23:10:07]

Done.

+        if (null == audioManager) {
+            return;
+        }
+        audioManager.setSpeakerphoneOn(mIsSpeakerOn);
+    }
+
+    private int hasEarpiece() {
+        TelephonyManager tm =
+                (TelephonyManager)mContext.getSystemService(Context.TELEPHONY_SERVICE);
+        if (null == tm) {

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

how can this fail?

[leozwang1, 2013/03/21 23:10:07]

Done.

+            return -1;

[qinmin, 2013/03/21 21:19:38]

return kPhoneTypeError

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

kPhoneTypeError?

[leozwang1, 2013/03/21 23:10:07]

Done.

+        }
+        return tm.getPhoneType() !=
+            TelephonyManager.PHONE_TYPE_NONE ? kPhoneTypePhone : kPhoneTypeNone;

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

What does SIP/GSM/CDMA have to do with having an earpiece or not?
Why does "earpiece" enter into it?  Isn't the real concern here whether a
"headset" is plugged in?

Put another way: if ACTION_HEADSET_PLUG is fired with a state of 1, why would
you *not* silence the external speaker?

[leozwang1, 2013/03/21 23:10:07]

This function is wrong, thanks for pointing it out.

+    }
+
+    private AudioManager getAudioManager() {
+        return null == mContext ?

[Ami GONE FROM CHROMIUM, 2013/03/21 21:26:21]

how can this be false?

[leozwang1, 2013/03/21 23:10:07]

Done.

+            null : (AudioManager)mContext.getSystemService(Context.AUDIO_SERVICE);
+    }
+
 }