Cherry-picking Id45abeba and Ia065dec6 for MR1

-------------------------------------------------------
MCC detection fixes for CountryDetector

- Don't get and cache phone tpe at the initialization time.  At this point
TelephonyManager is probably not ready yet.

- Refresh MCC whenever we get the service state changed callback, even when
the state hasn't actually changed, in order to make sure we get refresh
country properly when MCC changes.

- Also remove the initialization of mPhoneStateListener, which prevented us from
registering phone state listener properly.

- Also fix tests which were already failing.

Bug 5670680

-------------------------------------------------------
Add logging to country detector logic

This is for debugging purposes to verify the effects of
change Id45abeba1b1e843053ac2c946861b439ca568de4.

Bug: 5670680
Change-Id: I238d953484e2c8135f7dac70fce8662c8300a286

3 files changed, 161 insertions, 38 deletions

diff --git a/services/java/com/android/server/CountryDetectorService.java b/services/java/com/android/server/CountryDetectorService.java
index 3081ebe..ab61a3c 100644
--- a/services/java/com/android/server/CountryDetectorService.java
+++ b/services/java/com/android/server/CountryDetectorService.java
@@ -16,6 +16,8 @@
 
 package com.android.server;
 
+import java.io.FileDescriptor;
+import java.io.PrintWriter;
 import java.util.HashMap;
 
 import com.android.server.location.ComprehensiveCountryDetector;
@@ -30,6 +32,8 @@ import android.os.IBinder;
 import android.os.Looper;
 import android.os.Process;
 import android.os.RemoteException;
+import android.util.PrintWriterPrinter;
+import android.util.Printer;
 import android.util.Slog;
 
 /**
@@ -75,7 +79,7 @@ public class CountryDetectorService extends ICountryDetector.Stub implements Run
         }
     }
 
-    private final static String TAG = "CountryDetectorService";
+    private final static String TAG = "CountryDetector";
 
     private final HashMap<IBinder, Receiver> mReceivers;
     private final Context mContext;
@@ -201,4 +205,20 @@ public class CountryDetectorService extends ICountryDetector.Stub implements Run
     boolean isSystemReady() {
         return mSystemReady;
     }
+
+    @Override
+    protected void dump(FileDescriptor fd, PrintWriter fout, String[] args) {
+        try {
+            final Printer p = new PrintWriterPrinter(fout);
+            p.println("CountryDetectorService state:");
+            p.println("  Number of listeners=" + mReceivers.keySet().size());
+            if (mCountryDetector == null) {
+                p.println("  ComprehensiveCountryDetector not initialized");
+            } else {
+                p.println("  " + mCountryDetector.toString());
+            }
+        } catch (Exception e) {
+            Slog.e(TAG, "Failed to dump CountryDetectorService: ", e);
+        }
+    }
 }
diff --git a/services/java/com/android/server/location/ComprehensiveCountryDetector.java b/services/java/com/android/server/location/ComprehensiveCountryDetector.java
index bb9e60f..2d6a148 100755
--- a/services/java/com/android/server/location/ComprehensiveCountryDetector.java
+++ b/services/java/com/android/server/location/ComprehensiveCountryDetector.java
@@ -20,16 +20,19 @@ import android.content.Context;
 import android.location.Country;
 import android.location.CountryListener;
 import android.location.Geocoder;
+import android.os.SystemClock;
 import android.provider.Settings;
 import android.telephony.PhoneStateListener;
 import android.telephony.ServiceState;
 import android.telephony.TelephonyManager;
 import android.text.TextUtils;
+import android.util.Log;
 import android.util.Slog;
 
 import java.util.Locale;
 import java.util.Timer;
 import java.util.TimerTask;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 /**
  * This class is used to detect the country where the user is. The sources of
@@ -55,10 +58,15 @@ import java.util.TimerTask;
  */
 public class ComprehensiveCountryDetector extends CountryDetectorBase {
 
-    private final static String TAG = "ComprehensiveCountryDetector";
+    private final static String TAG = "CountryDetector";
     /* package */ static final boolean DEBUG = false;
 
     /**
+     * Max length of logs to maintain for debugging.
+     */
+    private static final int MAX_LENGTH_DEBUG_LOGS = 20;
+
+    /**
      * The refresh interval when the location based country was used
      */
     private final static long LOCATION_REFRESH_INTERVAL = 1000 * 60 * 60 * 24; // 1 day
@@ -66,26 +74,58 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
     protected CountryDetectorBase mLocationBasedCountryDetector;
     protected Timer mLocationRefreshTimer;
 
-    private final int mPhoneType;
     private Country mCountry;
-    private TelephonyManager mTelephonyManager;
+    private final TelephonyManager mTelephonyManager;
     private Country mCountryFromLocation;
     private boolean mStopped = false;
-    private ServiceState mLastState;
 
-    private PhoneStateListener mPhoneStateListener = new PhoneStateListener() {
-        @Override
-        public void onServiceStateChanged(ServiceState serviceState) {
-            // TODO: Find out how often we will be notified, if this method is called too
-            // many times, let's consider querying the network.
-            Slog.d(TAG, "onServiceStateChanged");
-            // We only care the state change
-            if (mLastState == null || mLastState.getState() != serviceState.getState()) {
-                detectCountry(true, true);
-                mLastState = new ServiceState(serviceState);
-            }
-        }
-    };
+    private PhoneStateListener mPhoneStateListener;
+
+    /**
+     * List of the most recent country state changes for debugging. This should have
+     * a max length of MAX_LENGTH_LOGS.
+     */
+    private final ConcurrentLinkedQueue<Country> mDebugLogs = new ConcurrentLinkedQueue<Country>();
+
+    /**
+     * Most recent {@link Country} result that was added to the debug logs {@link #mDebugLogs}.
+     * We keep track of this value to help prevent adding many of the same {@link Country} objects
+     * to the logs.
+     */
+    private Country mLastCountryAddedToLogs;
+
+    /**
+     * Object used to synchronize access to {@link #mLastCountryAddedToLogs}. Be careful if
+     * using it to synchronize anything else in this file.
+     */
+    private final Object mObject = new Object();
+
+    /**
+     * Start time of the current session for which the detector has been active.
+     */
+    private long mStartTime;
+
+    /**
+     * Stop time of the most recent session for which the detector was active.
+     */
+    private long mStopTime;
+
+    /**
+     * The sum of all the time intervals in which the detector was active.
+     */
+    private long mTotalTime;
+
+    /**
+     * Number of {@link PhoneStateListener#onServiceStateChanged(ServiceState state)} events that
+     * have occurred for the current session for which the detector has been active.
+     */
+    private int mCountServiceStateChanges;
+
+    /**
+     * Total number of {@link PhoneStateListener#onServiceStateChanged(ServiceState state)} events
+     * that have occurred for all time intervals in which the detector has been active.
+     */
+    private int mTotalCountServiceStateChanges;
 
     /**
      * The listener for receiving the notification from LocationBasedCountryDetector.
@@ -104,7 +144,6 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
     public ComprehensiveCountryDetector(Context context) {
         super(context);
         mTelephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
-        mPhoneType = mTelephonyManager.getPhoneType();
     }
 
     @Override
@@ -115,6 +154,7 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
 
     @Override
     public void stop() {
+        // Note: this method in this subclass called only by tests.
         Slog.i(TAG, "Stop the detector.");
         cancelLocationRefresh();
         removePhoneStateListener();
@@ -138,17 +178,50 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
         if (result == null) {
             result = getLocaleCountry();
         }
+        addToLogs(result);
         return result;
     }
 
     /**
+     * Attempt to add this {@link Country} to the debug logs.
+     */
+    private void addToLogs(Country country) {
+        if (country == null) {
+            return;
+        }
+        // If the country (ISO and source) are the same as before, then there is no
+        // need to add this country as another entry in the logs. Synchronize access to this
+        // variable since multiple threads could be calling this method.
+        synchronized (mObject) {
+            if (mLastCountryAddedToLogs != null && mLastCountryAddedToLogs.equals(country)) {
+                return;
+            }
+            mLastCountryAddedToLogs = country;
+        }
+        // Manually maintain a max limit for the list of logs
+        if (mDebugLogs.size() >= MAX_LENGTH_DEBUG_LOGS) {
+            mDebugLogs.poll();
+        }
+        if (Log.isLoggable(TAG, Log.DEBUG)) {
+            Slog.d(TAG, country.toString());
+        }
+        mDebugLogs.add(country);
+    }
+
+    private boolean isNetworkCountryCodeAvailable() {
+        // On CDMA TelephonyManager.getNetworkCountryIso() just returns SIM country.  We don't want
+        // to prioritize it over location based country, so ignore it.
+        final int phoneType = mTelephonyManager.getPhoneType();
+        if (DEBUG) Slog.v(TAG, "    phonetype=" + phoneType);
+        return phoneType == TelephonyManager.PHONE_TYPE_GSM;
+    }
+
+    /**
      * @return the country from the mobile network.
      */
     protected Country getNetworkBasedCountry() {
         String countryIso = null;
-        // TODO: The document says the result may be unreliable on CDMA networks. Shall we use
-        // it on CDMA phone? We may test the Android primarily used countries.
-        if (mPhoneType == TelephonyManager.PHONE_TYPE_GSM) {
+        if (isNetworkCountryCodeAvailable()) {
             countryIso = mTelephonyManager.getNetworkCountryIso();
             if (!TextUtils.isEmpty(countryIso)) {
                 return new Country(countryIso, Country.COUNTRY_SOURCE_NETWORK);
@@ -226,9 +299,14 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
             removePhoneStateListener();
             stopLocationBasedDetector();
             cancelLocationRefresh();
+            mStopTime = SystemClock.elapsedRealtime();
+            mTotalTime += mStopTime;
         } else if (prevListener == null) {
             addPhoneStateListener();
             detectCountry(false, true);
+            mStartTime = SystemClock.elapsedRealtime();
+            mStopTime = 0;
+            mCountServiceStateChanges = 0;
         }
     }
 
@@ -316,9 +394,9 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
     private void notifyIfCountryChanged(final Country country, final Country detectedCountry) {
         if (detectedCountry != null && mListener != null
                 && (country == null || !country.equals(detectedCountry))) {
-            Slog.d(TAG,
-                    "The country was changed from " + country != null ? country.getCountryIso() :
-                        country + " to " + detectedCountry.getCountryIso());
+            if (Log.isLoggable(TAG, Log.DEBUG)) {
+                Slog.d(TAG, "" + country + " --> " + detectedCountry);
+            }
             notifyListener(detectedCountry);
         }
     }
@@ -356,20 +434,19 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
     }
 
     protected synchronized void addPhoneStateListener() {
-        if (mPhoneStateListener == null && mPhoneType == TelephonyManager.PHONE_TYPE_GSM) {
-            mLastState = null;
+        if (mPhoneStateListener == null) {
             mPhoneStateListener = new PhoneStateListener() {
                 @Override
                 public void onServiceStateChanged(ServiceState serviceState) {
-                    // TODO: Find out how often we will be notified, if this
-                    // method is called too
-                    // many times, let's consider querying the network.
-                    Slog.d(TAG, "onServiceStateChanged");
-                    // We only care the state change
-                    if (mLastState == null || mLastState.getState() != serviceState.getState()) {
-                        detectCountry(true, true);
-                        mLastState = new ServiceState(serviceState);
+                    mCountServiceStateChanges++;
+                    mTotalCountServiceStateChanges++;
+
+                    if (!isNetworkCountryCodeAvailable()) {
+                        return;
                     }
+                    if (DEBUG) Slog.d(TAG, "onServiceStateChanged: " + serviceState.getState());
+
+                    detectCountry(true, true);
                 }
             };
             mTelephonyManager.listen(mPhoneStateListener, PhoneStateListener.LISTEN_SERVICE_STATE);
@@ -386,4 +463,30 @@ public class ComprehensiveCountryDetector extends CountryDetectorBase {
     protected boolean isGeoCoderImplemented() {
         return Geocoder.isPresent();
     }
+
+    @Override
+    public String toString() {
+        long currentTime = SystemClock.elapsedRealtime();
+        long currentSessionLength = 0;
+        StringBuilder sb = new StringBuilder();
+        sb.append("ComprehensiveCountryDetector{");
+        // The detector hasn't stopped yet --> still running
+        if (mStopTime == 0) {
+            currentSessionLength = currentTime - mStartTime;
+            sb.append("timeRunning=" + currentSessionLength + ", ");
+        } else {
+            // Otherwise, it has already stopped, so take the last session
+            sb.append("lastRunTimeLength=" + (mStopTime - mStartTime) + ", ");
+        }
+        sb.append("totalCountServiceStateChanges=" + mTotalCountServiceStateChanges + ", ");
+        sb.append("currentCountServiceStateChanges=" + mCountServiceStateChanges + ", ");
+        sb.append("totalTime=" + (mTotalTime + currentSessionLength) + ", ");
+        sb.append("currentTime=" + currentTime + ", ");
+        sb.append("countries=");
+        for (Country country : mDebugLogs) {
+            sb.append("\n   " + country.toString());
+        }
+        sb.append("}");
+        return sb.toString();
+    }
 }
diff --git a/services/tests/servicestests/src/com/android/server/location/LocationBasedCountryDetectorTest.java b/services/tests/servicestests/src/com/android/server/location/LocationBasedCountryDetectorTest.java
index 60677df..5f5d668 100755
--- a/services/tests/servicestests/src/com/android/server/location/LocationBasedCountryDetectorTest.java
+++ b/services/tests/servicestests/src/com/android/server/location/LocationBasedCountryDetectorTest.java
@@ -213,7 +213,7 @@ public class LocationBasedCountryDetectorTest extends AndroidTestCase {
         // QueryThread should be set to NULL
         assertNull(detector.getQueryThread());
         assertTrue(countryListener.notified());
-        assertEquals(countryListener.getCountry(), country);
+        assertEquals("us", countryListener.getCountry().toLowerCase());
     }
 
     public void testFindingCountryCancelled() {
@@ -238,7 +238,7 @@ public class LocationBasedCountryDetectorTest extends AndroidTestCase {
         // QueryThread should be set to NULL
         assertNull(detector.getQueryThread());
         assertTrue(countryListener.notified());
-        assertEquals(countryListener.getCountry(), country);
+        assertEquals("us", countryListener.getCountry().toLowerCase());
     }
 
     public void testFindingLocationCancelled() {
@@ -339,7 +339,7 @@ public class LocationBasedCountryDetectorTest extends AndroidTestCase {
         assertNull(detector.getQueryThread());
         // CountryListener should be notified
         assertTrue(countryListener.notified());
-        assertEquals(countryListener.getCountry(), country);
+        assertEquals("us", countryListener.getCountry().toLowerCase());
     }
 
     private void waitForTimerReset(TestCountryDetector detector) {