Use Android callback API to obtain cellular signal strength

net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java

Index: net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java
diff --git a/net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java b/net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java
index 06a0a9f0c114ef7d42584b8cd86803130460ae7a..61732ebc82f793d19f38a9fc8b5f9a317d9f1372 100644
--- a/net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java
+++ b/net/android/java/src/org/chromium/net/AndroidCellularSignalStrength.java
@@ -4,156 +4,102 @@
 
 package org.chromium.net;
 
-import android.Manifest;
 import android.annotation.TargetApi;
 import android.content.Context;
-import android.content.pm.PackageManager;
 import android.os.Build;
-import android.os.Process;
-import android.telephony.CellInfo;
-import android.telephony.CellInfoCdma;
-import android.telephony.CellInfoGsm;
-import android.telephony.CellInfoLte;
-import android.telephony.CellInfoWcdma;
+import android.telephony.PhoneStateListener;
+import android.telephony.SignalStrength;
 import android.telephony.TelephonyManager;
 
 import org.chromium.base.ContextUtils;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.SuppressFBWarnings;
 
-import java.util.Iterator;
-import java.util.List;
+import javax.annotation.concurrent.GuardedBy;
 
 /**
- * This class interacts with the CellInfo API provided by Android. This class is thread safe.
+ * This class provides the cellular signal strength using the APIs provided by Android. This class
+ * is thread safe.
  */
-@JNINamespace("net::android::cellular_signal_strength")
+@JNINamespace("net::android")
 public class AndroidCellularSignalStrength {
-    /**
-     * @return Signal strength (in dbM) for the currently registered cellular network. Returns
-     * {@link CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal strength is
-     * unavailable or if there are multiple cellular radios on the device.
-     */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
-    @CalledByNative
-    public static int getSignalStrengthDbm() {
-        List<CellInfo> cellInfos = getRegisteredCellInfo();
-        return cellInfos == null || cellInfos.size() != 1
-                ? CellularSignalStrengthError.ERROR_NOT_SUPPORTED
-                : getSignalStrengthDbm(cellInfos.get(0));
-    }
+    private final Context mContext;

[pauljensen, 2017/06/08 18:07:33]

Don't keep pointers to Context, just use ContextUtils.getApplicationContext()

[tbansal1, 2017/06/08 23:34:30]

Done.

 
-    /**
-     * @return the signal strength level (between 0 and 4, both inclusive) for the currently
-     * registered cellular network with lower value indicating lower signal strength. Returns
-     * {@link CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal strength level is
-     * unavailable or if there are multiple cellular radios on the device.
-     */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
-    @CalledByNative
-    public static int getSignalStrengthLevel() {
-        List<CellInfo> cellInfos = getRegisteredCellInfo();
-        return cellInfos == null || cellInfos.size() != 1
-                ? CellularSignalStrengthError.ERROR_NOT_SUPPORTED
-                : getSignalStrengthLevel(cellInfos.get(0));
-    }
+    private final Object mLock = new Object();
 
-    /**
-     * Returns true if the API for quering the signal strength is available.
-     * {@link android.telephony#CellInfoWcdma} is only available on API Level
-     * {@link Build.VERSION_CODES#JELLY_BEAN_MR2} and higher. Also verifies that appropriate
-     * permissions are already available. This ensures that on Android M and higher, Chromium will
-     * not request run-time permission from the user when querying for cellular signal strength.
-     * TODO(tbansal): Consider using {@link TelephonyManager#getNeighboringCellInfo}
-     * for earlier versions of Android.
-    */
-    private static boolean isAPIAvailable() {
-        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR2) return false;
-
-        try {
-            return ContextUtils.getApplicationContext().checkPermission(
-                           Manifest.permission.ACCESS_COARSE_LOCATION, Process.myPid(),
-                           Process.myUid())
-                    == PackageManager.PERMISSION_GRANTED;
-        } catch (Exception ignored) {
-            // Work around certain platforms where this method sometimes throws a runtime exception.
-            // See crbug.com/663360.
-        }
-        return false;
-    }
+    @GuardedBy("mLock")
+    private int mSignalLevel = CellularSignalStrengthError.ERROR_NOT_SUPPORTED;

[pauljensen, 2017/06/08 18:07:33]

I don't think this needs to be guarded by a lock.  Can you just make it
volatile?  "int"s are atomic in Java.

[tbansal1, 2017/06/08 23:34:30]

Done.

+
+    @SuppressFBWarnings("URF_UNREAD_FIELD")
+    private CellStateListener mCellStateListener;
 
     /**
-     * Returns all observed cell information from all radios on the device including the primary
-     * and neighboring cells. Returns only the information of cells that are registered to a
-     * mobile network. May return {@code null}.
+     * This class listens to the changes in the cellular signal strength level and updates {@link
+     * mSignalLevel}.
      */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
-    private static List<CellInfo> getRegisteredCellInfo() {
-        if (!isAPIAvailable()) {
-            return null;
+    private class CellStateListener extends PhoneStateListener {
+        CellStateListener() {
+            ThreadUtils.assertOnUiThread();

[pauljensen, 2017/06/08 18:07:33]

Why does this need to be done on the UI thread?

[tbansal1, 2017/06/08 23:34:30]

06-08 15:55:20.685 11863 11930 W System.err: java.lang.NullPointerException:
Attempt to read from field 'android.os.MessageQueue android.os.Looper.mQueue' on
a null object reference
06-08 15:55:20.685 11863 11930 W System.err: 	at
android.os.Handler.<init>(Handler.java:229)
06-08 15:55:20.685 11863 11930 W System.err: 	at
android.os.Handler.<init>(Handler.java:137)
06-08 15:55:20.685 11863 11930 W System.err: 	at
android.telephony.PhoneStateListener$1.<init>(PhoneStateListener.java:275)
06-08 15:55:20.685 11863 11930 W System.err: 	at
android.telephony.PhoneStateListener.<init>(PhoneStateListener.java:275)
06-08 15:55:20.686 11863 11930 W System.err: 	at
android.telephony.PhoneStateListener.<init>(PhoneStateListener.java:245)
06-08 15:55:20.686 11863 11930 W System.err: 	at
org.chromium.net.AndroidCellularSignalStrength$CellStateListener.<init>(AndroidCellularSignalStrength.java:47)
06-08 15:55:20.686 11863 11930 W System.err: 	at
org.chromium.net.AndroidCellularSignalStrength.<init>(AndroidCellularSignalStrength.java:103)
06-08 15:55:20.686 11863 11930 W System.err: 	at
org.chromium.net.AndroidCellularSignalStrength.create(AndroidCellularSignalStrength.java:89)

[pauljensen, 2017/06/13 11:53:43]

I don't think it needs to be done on the UI thread.  The UI thread is probably
the most overbooked thread and we've gone great lengths to move as much as work
as possible off the UI thread.   It appears to need to be done on a thread that
has a Looper, which the IO thread does not apparently.  I have suspicions that
this kind of operation could take a while; Context.registerListener() takes a
while.  Can you measure how long this operation takes on an older device?  We
may want to move it to another thread if it takes a while.  I wonder if there is
a thread-pool containing Java threads.

[tbansal1, 2017/06/20 15:50:53]

It takes around 10 msec or so. I have moved it to a different thread based on
suggestion here: http://shortn/_jJ1xZ2wPwR

+
+            TelephonyManager telephonyManager =
+                    (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE);
+
+            if (telephonyManager.getSimState() != TelephonyManager.SIM_STATE_READY) return;
+
+            telephonyManager.listen(this, PhoneStateListener.LISTEN_SIGNAL_STRENGTHS);
         }
 
-        TelephonyManager telephonyManager =
-                (TelephonyManager) ContextUtils.getApplicationContext().getSystemService(
-                        Context.TELEPHONY_SERVICE);
-        if (telephonyManager == null) {
-            return null;
+        @Override
+        @TargetApi(Build.VERSION_CODES.M)
+        public void onSignalStrengthsChanged(SignalStrength signalStrength) {
+            synchronized (mLock) {
+                mSignalLevel = signalStrength.getLevel();
+            }
         }
+    }
+
+    @CalledByNative
+    private static AndroidCellularSignalStrength create() {
+        return new AndroidCellularSignalStrength();
+    }
 
-        List<CellInfo> cellInfos = telephonyManager.getAllCellInfo();
-        if (cellInfos == null) {
-            return null;
+    private AndroidCellularSignalStrength() {
+        mContext = ContextUtils.getApplicationContext();
+
+        if (!isAPIAvailable(mContext)) {
+            return;
         }
 
-        Iterator<CellInfo> iter = cellInfos.iterator();
-        while (iter.hasNext()) {
-            if (!iter.next().isRegistered()) {
-                iter.remove();
+        ThreadUtils.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mCellStateListener = new CellStateListener();
             }
-        }
-        return cellInfos;
+        });
     }
 
     /**
-     * @return Signal strength (in dbM) from {@link cellInfo}. Returns {@link
-     * CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal strength is unavailable.
+     * @return the signal strength level (between 0 and 4, both inclusive) for the currently
+     * registered cellular network with lower value indicating lower signal strength. Returns
+     * {@link CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal strength level is
+     * unavailable.
      */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
-    private static int getSignalStrengthDbm(CellInfo cellInfo) {
-        if (cellInfo instanceof CellInfoCdma) {
-            return ((CellInfoCdma) cellInfo).getCellSignalStrength().getDbm();
-        }
-        if (cellInfo instanceof CellInfoGsm) {
-            return ((CellInfoGsm) cellInfo).getCellSignalStrength().getDbm();
-        }
-        if (cellInfo instanceof CellInfoLte) {
-            return ((CellInfoLte) cellInfo).getCellSignalStrength().getDbm();
-        }
-        if (cellInfo instanceof CellInfoWcdma) {
-            return ((CellInfoWcdma) cellInfo).getCellSignalStrength().getDbm();
+    @TargetApi(Build.VERSION_CODES.M)
+    @CalledByNative
+    public int getSignalStrengthLevel() {
+        synchronized (mLock) {
+            return mSignalLevel;
         }
-        return CellularSignalStrengthError.ERROR_NOT_SUPPORTED;
     }
 
     /**
-     * @return the signal level from {@link cellInfo}. Returns {@link
-     * CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal
-     * level is unavailable with lower value indicating lower signal strength.
+     * Returns true if the API for quering the signal strength is available.
+     * {@link android.telephony.SignalStrength#getLevel} is only available on API Level
+     * {@link Build.VERSION_CODES#M} and higher.
      */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
-    private static int getSignalStrengthLevel(CellInfo cellInfo) {
-        if (cellInfo instanceof CellInfoCdma) {
-            return ((CellInfoCdma) cellInfo).getCellSignalStrength().getLevel();
-        }
-        if (cellInfo instanceof CellInfoGsm) {
-            return ((CellInfoGsm) cellInfo).getCellSignalStrength().getLevel();
-        }
-        if (cellInfo instanceof CellInfoLte) {
-            return ((CellInfoLte) cellInfo).getCellSignalStrength().getLevel();
-        }
-        if (cellInfo instanceof CellInfoWcdma) {
-            return ((CellInfoWcdma) cellInfo).getCellSignalStrength().getLevel();
-        }
-        return CellularSignalStrengthError.ERROR_NOT_SUPPORTED;
+    private static boolean isAPIAvailable(Context context) {
+        return Build.VERSION.SDK_INT >= Build.VERSION_CODES.M;

[pauljensen, 2017/06/08 18:07:33]

Are you aware that M+ is only ~40% of users:
https://d.android.com/about/dashboards

[tbansal1, 2017/06/08 23:34:30]

Yes. I think this is probably the best option for now.

     }
 }