Use Android callback API to obtain cellular signal strength

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 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 {
+    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.

+
+    private final Object mLock = new Object();
+
+    @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;
+
     /**
-     * @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.
+     * This class listens to the changes in the cellular signal strength level and updates {@link
+     * mSignalLevel}.
      */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
+    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);
+        }
+
+        @Override
+        @TargetApi(Build.VERSION_CODES.M)
+        public void onSignalStrengthsChanged(SignalStrength signalStrength) {
+            synchronized (mLock) {
+                mSignalLevel = signalStrength.getLevel();
+            }
+        }
+    }
+
     @CalledByNative
-    public static int getSignalStrengthDbm() {
-        List<CellInfo> cellInfos = getRegisteredCellInfo();
-        return cellInfos == null || cellInfos.size() != 1
-                ? CellularSignalStrengthError.ERROR_NOT_SUPPORTED
-                : getSignalStrengthDbm(cellInfos.get(0));
+    private static AndroidCellularSignalStrength create() {
+        return new AndroidCellularSignalStrength();
+    }
+
+    private AndroidCellularSignalStrength() {
+        mContext = ContextUtils.getApplicationContext();
+
+        if (!isAPIAvailable(mContext)) {
+            return;
+        }
+
+        ThreadUtils.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mCellStateListener = new CellStateListener();
+            }
+        });
     }
 
     /**
      * @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.
+     * unavailable.
      */
-    @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
+    @TargetApi(Build.VERSION_CODES.M)
     @CalledByNative
-    public static int getSignalStrengthLevel() {
-        List<CellInfo> cellInfos = getRegisteredCellInfo();
-        return cellInfos == null || cellInfos.size() != 1
-                ? CellularSignalStrengthError.ERROR_NOT_SUPPORTED
-                : getSignalStrengthLevel(cellInfos.get(0));
+    public int getSignalStrengthLevel() {
+        synchronized (mLock) {
+            return mSignalLevel;
+        }
     }
 
     /**
      * 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;
-    }
-
-    /**
-     * 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}.
+     * {@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 List<CellInfo> getRegisteredCellInfo() {
-        if (!isAPIAvailable()) {
-            return null;
-        }
-
-        TelephonyManager telephonyManager =
-                (TelephonyManager) ContextUtils.getApplicationContext().getSystemService(
-                        Context.TELEPHONY_SERVICE);
-        if (telephonyManager == null) {
-            return null;
-        }
-
-        List<CellInfo> cellInfos = telephonyManager.getAllCellInfo();
-        if (cellInfos == null) {
-            return null;
-        }
-
-        Iterator<CellInfo> iter = cellInfos.iterator();
-        while (iter.hasNext()) {
-            if (!iter.next().isRegistered()) {
-                iter.remove();
-            }
-        }
-        return cellInfos;
-    }
-
-    /**
-     * @return Signal strength (in dbM) from {@link cellInfo}. Returns {@link
-     * CellularSignalStrengthError#ERROR_NOT_SUPPORTED} if the signal strength 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();
-        }
-        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.
-     */
-    @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.

     }
 }