Merge changes I1476a938,I111189d1 am: b4eec7fc1f

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2217305

Change-Id: I7b1e5c31625bba9dc1a5dd8c4752d3c0b1320859
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/timedetector/GnssTimeUpdateService.java

@@ -18,21 +18,26 @@ package com.android.server.timedetector;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
+import android.annotation.RequiresPermission;
 import android.app.AlarmManager;
 import android.app.timedetector.GnssTimeSuggestion;
 import android.app.timedetector.TimeDetector;
 import android.content.Context;
-import android.location.Location;
 import android.location.LocationListener;
 import android.location.LocationManager;
 import android.location.LocationManagerInternal;
 import android.location.LocationRequest;
 import android.location.LocationTime;
 import android.os.Binder;
+import android.os.Handler;
+import android.os.ResultReceiver;
+import android.os.ShellCallback;
 import android.os.SystemClock;
 import android.os.TimestampedValue;
+import android.util.LocalLog;
 import android.util.Log;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.DumpUtils;
 import com.android.server.FgThread;
@@ -43,6 +48,7 @@ import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.time.Duration;
 import java.util.Objects;
+import java.util.concurrent.Executor;
 
 /**
  * Monitors the GNSS time.
@@ -88,7 +94,7 @@ public final class GnssTimeUpdateService extends Binder {
                 // Instead of polling GNSS time periodically, passive location updates are enabled.
                 // Once an update is received, the gnss time will be queried and suggested to
                 // TimeDetectorService.
-                mService.requestGnssTimeUpdates();
+                mService.startGnssListeningInternal();
             }
         }
     }
@@ -96,15 +102,28 @@ public final class GnssTimeUpdateService extends Binder {
     private static final Duration GNSS_TIME_UPDATE_ALARM_INTERVAL = Duration.ofHours(4);
     private static final String ATTRIBUTION_TAG = "GnssTimeUpdateService";
 
+    /**
+     * A log that records the decisions to fetch a GNSS time update.
+     * This is logged in bug reports to assist with debugging issues with GNSS time suggestions.
+     */
+    private final LocalLog mLocalLog = new LocalLog(10, false /* useLocalTimestamps */);
+    /** The executor used for async operations */
+    private final Executor mExecutor = FgThread.getExecutor();
+    /** The handler used for async operations */
+    private final Handler mHandler = FgThread.getHandler();
+
     private final Context mContext;
     private final TimeDetector mTimeDetector;
     private final AlarmManager mAlarmManager;
     private final LocationManager mLocationManager;
     private final LocationManagerInternal mLocationManagerInternal;
 
-    @Nullable private AlarmManager.OnAlarmListener mAlarmListener;
-    @Nullable private LocationListener mLocationListener;
-    @Nullable private TimestampedValue<Long> mLastSuggestedGnssTime;
+
+    private final Object mLock = new Object();
+    @GuardedBy("mLock") @Nullable private AlarmManager.OnAlarmListener mAlarmListener;
+    @GuardedBy("mLock") @Nullable private LocationListener mLocationListener;
+
+    @Nullable private volatile TimestampedValue<Long> mLastSuggestedGnssTime;
 
     @VisibleForTesting
     GnssTimeUpdateService(@NonNull Context context, @NonNull AlarmManager alarmManager,
@@ -119,87 +138,133 @@ public final class GnssTimeUpdateService extends Binder {
     }
 
     /**
-     * Request passive location updates. Such a request will not trigger any active locations or
-     * power usage itself.
+     * Used by {@link com.android.server.timedetector.GnssTimeUpdateServiceShellCommand} to force
+     * the service into GNSS listening mode.
+     */
+    @RequiresPermission(android.Manifest.permission.SET_TIME)
+    boolean startGnssListening() {
+        mContext.enforceCallingPermission(
+                android.Manifest.permission.SET_TIME, "Start GNSS listening");
+        mLocalLog.log("startGnssListening() called");
+
+        final long token = Binder.clearCallingIdentity();
+        try {
+            return startGnssListeningInternal();
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
+    /**
+     * Starts listening for passive location updates. Such a request will not trigger any active
+     * locations or power usage itself. Returns {@code true} if the service is listening after the
+     * method returns and {@code false} otherwise. At present this method only returns {@code false}
+     * if there is no GPS provider on the device.
+     *
+     * <p>If the service is already listening for locations this is a no-op. If the device is in a
+     * "sleeping" state between listening periods then it will return to listening.
      */
     @VisibleForTesting
-    void requestGnssTimeUpdates() {
-        if (D) {
-            Log.d(TAG, "requestGnssTimeUpdates()");
+    boolean startGnssListeningInternal() {
+        if (!mLocationManager.hasProvider(LocationManager.GPS_PROVIDER)) {
+            logError("GPS provider does not exist on this device");
+            return false;
         }
 
-        if (!mLocationManager.hasProvider(LocationManager.GPS_PROVIDER)) {
-            Log.e(TAG, "GPS provider does not exist on this device");
-            return;
+        synchronized (mLock) {
+            if (mLocationListener != null) {
+                logDebug("Already listening for GNSS updates");
+                return true;
+            }
+
+            // If startGnssListening() is called during manual tests to jump back into location
+            // listening then there will usually be an alarm set.
+            if (mAlarmListener != null) {
+                mAlarmManager.cancel(mAlarmListener);
+                mAlarmListener = null;
+            }
+
+            startGnssListeningLocked();
+            return true;
         }
+    }
+
+    @GuardedBy("mLock")
+    private void startGnssListeningLocked() {
+        logDebug("startGnssListeningLocked()");
 
         // Location Listener triggers onLocationChanged() when GNSS data is available, so
         // that the getGnssTimeMillis() function doesn't need to be continuously polled.
-        mLocationListener = new LocationListener() {
-            @Override
-            public void onLocationChanged(Location location) {
-                if (D) {
-                    Log.d(TAG, "onLocationChanged()");
-                }
-
-                // getGnssTimeMillis() can return null when the Master Location Switch for the
-                // foreground user is disabled.
-                LocationTime locationTime = mLocationManagerInternal.getGnssTimeMillis();
-                if (locationTime != null) {
-                    suggestGnssTime(locationTime);
-                } else {
-                    if (D) {
-                        Log.d(TAG, "getGnssTimeMillis() returned null");
-                    }
-                }
-
-                mLocationManager.removeUpdates(mLocationListener);
-                mLocationListener = null;
-
-                mAlarmListener = new AlarmManager.OnAlarmListener() {
-                    @Override
-                    public void onAlarm() {
-                        if (D) {
-                            Log.d(TAG, "onAlarm()");
-                        }
-                        mAlarmListener = null;
-                        requestGnssTimeUpdates();
-                    }
-                };
-
-                // Set next alarm to re-enable location updates.
-                long next = SystemClock.elapsedRealtime()
-                        + GNSS_TIME_UPDATE_ALARM_INTERVAL.toMillis();
-                mAlarmManager.set(
-                        AlarmManager.ELAPSED_REALTIME_WAKEUP,
-                        next,
-                        TAG,
-                        mAlarmListener,
-                        FgThread.getHandler());
-            }
-        };
-
+        mLocationListener = location -> handleLocationAvailable();
         mLocationManager.requestLocationUpdates(
                 LocationManager.GPS_PROVIDER,
                 new LocationRequest.Builder(LocationRequest.PASSIVE_INTERVAL)
                         .setMinUpdateIntervalMillis(0)
                         .build(),
-                FgThread.getExecutor(),
+                mExecutor,
                 mLocationListener);
     }
 
+    private void handleLocationAvailable() {
+        logDebug("handleLocationAvailable()");
+
+        // getGnssTimeMillis() can return null when the Master Location Switch for the
+        // foreground user is disabled.
+        LocationTime locationTime = mLocationManagerInternal.getGnssTimeMillis();
+        if (locationTime != null) {
+            String msg = "Passive location time received: " + locationTime;
+            logDebug(msg);
+            mLocalLog.log(msg);
+            suggestGnssTime(locationTime);
+        } else {
+            logDebug("getGnssTimeMillis() returned null");
+        }
+
+        synchronized (mLock) {
+            if (mLocationListener == null) {
+                logWarning("mLocationListener unexpectedly null");
+            } else {
+                mLocationManager.removeUpdates(mLocationListener);
+                mLocationListener = null;
+            }
+
+            if (mAlarmListener != null) {
+                logWarning("mAlarmListener was unexpectedly non-null");
+                mAlarmManager.cancel(mAlarmListener);
+            }
+
+            // Set next alarm to re-enable location updates.
+            long next = SystemClock.elapsedRealtime()
+                    + GNSS_TIME_UPDATE_ALARM_INTERVAL.toMillis();
+            mAlarmListener = this::handleAlarmFired;
+            mAlarmManager.set(
+                    AlarmManager.ELAPSED_REALTIME_WAKEUP,
+                    next,
+                    TAG,
+                    mAlarmListener,
+                    mHandler);
+        }
+    }
+
+    private void handleAlarmFired() {
+        logDebug("handleAlarmFired()");
+
+        synchronized (mLock) {
+            mAlarmListener = null;
+            startGnssListeningLocked();
+        }
+    }
+
     /**
      * Convert LocationTime to TimestampedValue. Then suggest TimestampedValue to Time Detector.
      */
     private void suggestGnssTime(LocationTime locationTime) {
-        if (D) {
-            Log.d(TAG, "suggestGnssTime()");
-        }
+        logDebug("suggestGnssTime()");
+
         long gnssTime = locationTime.getTime();
         long elapsedRealtimeMs = locationTime.getElapsedRealtimeNanos() / 1_000_000L;
 
-        TimestampedValue<Long> timeSignal = new TimestampedValue<>(
-                elapsedRealtimeMs, gnssTime);
+        TimestampedValue<Long> timeSignal = new TimestampedValue<>(elapsedRealtimeMs, gnssTime);
         mLastSuggestedGnssTime = timeSignal;
 
         GnssTimeSuggestion timeSuggestion = new GnssTimeSuggestion(timeSignal);
@@ -210,11 +275,38 @@ public final class GnssTimeUpdateService extends Binder {
     protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
         if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return;
         pw.println("mLastSuggestedGnssTime: " + mLastSuggestedGnssTime);
-        pw.print("state: ");
-        if (mLocationListener != null) {
-            pw.println("time updates enabled");
-        } else {
-            pw.println("alarm enabled");
+        synchronized (mLock) {
+            pw.print("state: ");
+            if (mLocationListener != null) {
+                pw.println("time updates enabled");
+            } else {
+                pw.println("alarm enabled");
+            }
+        }
+        pw.println("Log:");
+        mLocalLog.dump(pw);
+    }
+
+    @Override
+    public void onShellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err,
+            String[] args, ShellCallback callback, ResultReceiver resultReceiver) {
+        new GnssTimeUpdateServiceShellCommand(this).exec(
+                this, in, out, err, args, callback, resultReceiver);
+    }
+
+    private void logError(String msg) {
+        Log.e(TAG, msg);
+        mLocalLog.log(msg);
+    }
+
+    private void logWarning(String msg) {
+        Log.w(TAG, msg);
+        mLocalLog.log(msg);
+    }
+
+    private void logDebug(String msg) {
+        if (D) {
+            Log.d(TAG, msg);
         }
     }
 }

services/core/java/com/android/server/timedetector/GnssTimeUpdateServiceShellCommand.java

@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.timedetector;
+
+import android.annotation.NonNull;
+import android.os.ShellCommand;
+
+import java.io.PrintWriter;
+import java.util.Objects;
+
+/** Implements the shell command interface for {@link GnssTimeUpdateService}. */
+class GnssTimeUpdateServiceShellCommand extends ShellCommand {
+
+    /**
+     * The name of the service.
+     */
+    private static final String SHELL_COMMAND_SERVICE_NAME = "gnss_time_update_service";
+
+    /**
+     * A shell command that forces the service in to GNSS listening mode if it isn't already.
+     */
+    private static final String SHELL_COMMAND_START_GNSS_LISTENING = "start_gnss_listening";
+
+    @NonNull
+    private final GnssTimeUpdateService mGnssTimeUpdateService;
+
+    GnssTimeUpdateServiceShellCommand(GnssTimeUpdateService gnssTimeUpdateService) {
+        mGnssTimeUpdateService = Objects.requireNonNull(gnssTimeUpdateService);
+    }
+
+    @Override
+    public int onCommand(String cmd) {
+        if (cmd == null) {
+            return handleDefaultCommands(cmd);
+        }
+
+        switch (cmd) {
+            case SHELL_COMMAND_START_GNSS_LISTENING:
+                return runStartGnssListening();
+            default: {
+                return handleDefaultCommands(cmd);
+            }
+        }
+    }
+
+    private int runStartGnssListening() {
+        boolean success = mGnssTimeUpdateService.startGnssListening();
+        getOutPrintWriter().println(success);
+        return 0;
+    }
+
+    @Override
+    public void onHelp() {
+        final PrintWriter pw = getOutPrintWriter();
+        pw.printf("Network Time Update Service (%s) commands:\n", SHELL_COMMAND_SERVICE_NAME);
+        pw.printf("  help\n");
+        pw.printf("    Print this help text.\n");
+        pw.printf("  %s\n", SHELL_COMMAND_START_GNSS_LISTENING);
+        pw.printf("    Forces the service in to GNSS listening mode (if it isn't already).\n");
+        pw.printf("    Prints true if the service is listening after this command.\n");
+        pw.println();
+    }
+}

services/tests/servicestests/src/com/android/server/timedetector/GnssTimeUpdateServiceTest.java

@@ -16,15 +16,18 @@
 
 package com.android.server.timedetector;
 
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyLong;
 import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 import android.app.AlarmManager;
+import android.app.AlarmManager.OnAlarmListener;
 import android.app.timedetector.GnssTimeSuggestion;
 import android.app.timedetector.TimeDetector;
 import android.content.Context;
@@ -38,9 +41,6 @@ import android.os.TimestampedValue;
 
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.server.LocalServices;
-
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -66,19 +66,13 @@ public final class GnssTimeUpdateServiceTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
-        when(mMockLocationManager.hasProvider(LocationManager.GPS_PROVIDER))
-            .thenReturn(true);
+        installGpsProviderInMockLocationManager();
 
         mGnssTimeUpdateService = new GnssTimeUpdateService(
                 mMockContext, mMockAlarmManager, mMockLocationManager, mMockLocationManagerInternal,
                 mMockTimeDetector);
     }
 
-    @After
-    public void tearDown() {
-        LocalServices.removeServiceForTest(LocationManagerInternal.class);
-    }
-
     @Test
     public void testLocationListenerOnLocationChanged_validLocationTime_suggestsGnssTime() {
         TimestampedValue<Long> timeSignal = new TimestampedValue<>(
@@ -87,9 +81,9 @@ public final class GnssTimeUpdateServiceTest {
         LocationTime locationTime = new LocationTime(GNSS_TIME, ELAPSED_REALTIME_NS);
         doReturn(locationTime).when(mMockLocationManagerInternal).getGnssTimeMillis();
 
-        mGnssTimeUpdateService.requestGnssTimeUpdates();
+        assertTrue(mGnssTimeUpdateService.startGnssListeningInternal());
 
-        ArgumentCaptor<LocationListener> argumentCaptor =
+        ArgumentCaptor<LocationListener> locationListenerCaptor =
                 ArgumentCaptor.forClass(LocationListener.class);
         verify(mMockLocationManager).requestLocationUpdates(
                 eq(LocationManager.GPS_PROVIDER),
@@ -97,8 +91,8 @@ public final class GnssTimeUpdateServiceTest {
                     .setMinUpdateIntervalMillis(0)
                     .build()),
                 any(),
-                argumentCaptor.capture());
-        LocationListener locationListener = argumentCaptor.getValue();
+                locationListenerCaptor.capture());
+        LocationListener locationListener = locationListenerCaptor.getValue();
         Location location = new Location(LocationManager.GPS_PROVIDER);
 
         locationListener.onLocationChanged(location);
@@ -117,9 +111,9 @@ public final class GnssTimeUpdateServiceTest {
     public void testLocationListenerOnLocationChanged_nullLocationTime_doesNotSuggestGnssTime() {
         doReturn(null).when(mMockLocationManagerInternal).getGnssTimeMillis();
 
-        mGnssTimeUpdateService.requestGnssTimeUpdates();
+        assertTrue(mGnssTimeUpdateService.startGnssListeningInternal());
 
-        ArgumentCaptor<LocationListener> argumentCaptor =
+        ArgumentCaptor<LocationListener> locationListenerCaptor =
                 ArgumentCaptor.forClass(LocationListener.class);
         verify(mMockLocationManager).requestLocationUpdates(
                 eq(LocationManager.GPS_PROVIDER),
@@ -127,14 +121,14 @@ public final class GnssTimeUpdateServiceTest {
                     .setMinUpdateIntervalMillis(0)
                     .build()),
                 any(),
-                argumentCaptor.capture());
-        LocationListener locationListener = argumentCaptor.getValue();
+                locationListenerCaptor.capture());
+        LocationListener locationListener = locationListenerCaptor.getValue();
         Location location = new Location(LocationManager.GPS_PROVIDER);
 
         locationListener.onLocationChanged(location);
 
         verify(mMockLocationManager).removeUpdates(locationListener);
-        verify(mMockTimeDetector, never()).suggestGnssTime(any());
+        verifyZeroInteractions(mMockTimeDetector);
         verify(mMockAlarmManager).set(
                 eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
                 anyLong(),
@@ -142,4 +136,90 @@ public final class GnssTimeUpdateServiceTest {
                 any(),
                 any());
     }
+
+    @Test
+    public void testLocationListeningRestartsAfterSleep() {
+        ArgumentCaptor<LocationListener> locationListenerCaptor =
+                ArgumentCaptor.forClass(LocationListener.class);
+        ArgumentCaptor<OnAlarmListener> alarmListenerCaptor =
+                ArgumentCaptor.forClass(OnAlarmListener.class);
+
+        advanceServiceToSleepingState(locationListenerCaptor, alarmListenerCaptor);
+
+        // Simulate the alarm manager's wake-up call.
+        OnAlarmListener wakeUpListener = alarmListenerCaptor.getValue();
+        wakeUpListener.onAlarm();
+
+        // Verify the service returned to location listening.
+        verify(mMockLocationManager).requestLocationUpdates(any(), any(), any(), any());
+        verifyZeroInteractions(mMockAlarmManager, mMockTimeDetector);
+    }
+
+    // Tests what happens when a call is made to startGnssListeningInternal() when service is
+    // sleeping. This can happen when the start_gnss_listening shell command is used.
+    @Test
+    public void testStartGnssListeningInternalCalledWhenSleeping() {
+        ArgumentCaptor<LocationListener> locationListenerCaptor =
+                ArgumentCaptor.forClass(LocationListener.class);
+        ArgumentCaptor<OnAlarmListener> alarmListenerCaptor =
+                ArgumentCaptor.forClass(OnAlarmListener.class);
+
+        advanceServiceToSleepingState(locationListenerCaptor, alarmListenerCaptor);
+
+        // Call startGnssListeningInternal(), as can happen if the start_gnss_listening shell
+        // command is used.
+        assertTrue(mGnssTimeUpdateService.startGnssListeningInternal());
+
+        // Verify the alarm manager is told to stopped sleeping and the location manager is
+        // listening again.
+        verify(mMockAlarmManager).cancel(alarmListenerCaptor.getValue());
+        verify(mMockLocationManager).requestLocationUpdates(any(), any(), any(), any());
+        verifyZeroInteractions(mMockTimeDetector);
+    }
+
+    private void advanceServiceToSleepingState(
+            ArgumentCaptor<LocationListener> locationListenerCaptor,
+            ArgumentCaptor<OnAlarmListener> alarmListenerCaptor) {
+        TimestampedValue<Long> timeSignal = new TimestampedValue<>(
+                ELAPSED_REALTIME_MS, GNSS_TIME);
+        GnssTimeSuggestion timeSuggestion = new GnssTimeSuggestion(timeSignal);
+        LocationTime locationTime = new LocationTime(GNSS_TIME, ELAPSED_REALTIME_NS);
+        doReturn(locationTime).when(mMockLocationManagerInternal).getGnssTimeMillis();
+
+        assertTrue(mGnssTimeUpdateService.startGnssListeningInternal());
+
+        verify(mMockLocationManager).requestLocationUpdates(
+                any(), any(), any(), locationListenerCaptor.capture());
+        LocationListener locationListener = locationListenerCaptor.getValue();
+        Location location = new Location(LocationManager.GPS_PROVIDER);
+        verifyZeroInteractions(mMockAlarmManager, mMockTimeDetector);
+
+        locationListener.onLocationChanged(location);
+
+        verify(mMockLocationManager).removeUpdates(locationListener);
+        verify(mMockTimeDetector).suggestGnssTime(timeSuggestion);
+
+        // Verify the service is now "sleeping", i.e. waiting for a period before listening for
+        // GNSS locations again.
+        verify(mMockAlarmManager).set(
+                eq(AlarmManager.ELAPSED_REALTIME_WAKEUP),
+                anyLong(),
+                any(),
+                alarmListenerCaptor.capture(),
+                any());
+
+        // Reset mocks making it easier to verify the calls that follow.
+        reset(mMockAlarmManager, mMockTimeDetector, mMockLocationManager,
+                mMockLocationManagerInternal);
+        installGpsProviderInMockLocationManager();
+    }
+
+    /**
+     * Configures the mock response to ensure {@code
+     * locationManager.hasProvider(LocationManager.GPS_PROVIDER) == true }
+     */
+    private void installGpsProviderInMockLocationManager() {
+        when(mMockLocationManager.hasProvider(LocationManager.GPS_PROVIDER))
+                .thenReturn(true);
+    }
 }