From 3299f0913fd1382b755abb9fd07a2e0bdc41f2a5 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Mon, 31 Aug 2020 15:11:34 +0100 Subject: [PATCH] Tidy up the "unbundled" provider API This commit introduces a LocationTimeZoneEventUnbundled, as LocationTimeZoneEvent will not be on another API surface so cannot be reused. (The equivalent from LocationProvider is Location, which is public API so there is no LocationUnbundled.) Add an initialization timeout to LocationTimeZoneProviderRequest so that providers can make intelligent choices about (for example) how long to spend waiting for geolocation to happen passively. Test: atest services/tests/servicestests/src/com/android/internal/location/timezone/ Test: atest services/tests/servicestests/src/com/android/server/location/timezone/ Bug: 152744911 Change-Id: Id3e9e6916e8c3a132d8fc892338578ab9d2ff574 --- .../timezone/LocationTimeZoneEvent.java | 17 +- .../LocationTimeZoneProviderRequest.java | 69 +++++-- location/lib/Android.bp | 5 +- .../LocationTimeZoneEventUnbundled.java | 172 ++++++++++++++++++ .../LocationTimeZoneProviderBase.java | 31 +++- ...ationTimeZoneProviderRequestUnbundled.java | 15 ++ .../BinderLocationTimeZoneProvider.java | 6 +- .../timezone/ControllerEnvironmentImpl.java | 24 +++ .../location/timezone/ControllerImpl.java | 42 +++-- .../timezone/LocationTimeZoneProvider.java | 8 +- .../LocationTimeZoneProviderController.java | 19 ++ .../NullLocationTimeZoneProvider.java | 4 +- .../LocationTimeZoneProviderRequestTest.java | 8 +- .../location/timezone/ControllerImplTest.java | 65 +++++-- .../NullLocationTimeZoneProviderTest.java | 5 +- .../timezone/TestThreadingDomain.java | 4 +- 16 files changed, 414 insertions(+), 80 deletions(-) create mode 100644 location/lib/java/com/android/location/timezone/provider/LocationTimeZoneEventUnbundled.java diff --git a/location/java/android/location/timezone/LocationTimeZoneEvent.java b/location/java/android/location/timezone/LocationTimeZoneEvent.java index 55bc507964e6e..d3fd5c3ac061c 100644 --- a/location/java/android/location/timezone/LocationTimeZoneEvent.java +++ b/location/java/android/location/timezone/LocationTimeZoneEvent.java @@ -23,6 +23,8 @@ import android.os.Parcel; import android.os.Parcelable; import android.os.UserHandle; +import com.android.internal.util.Preconditions; + import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -37,7 +39,7 @@ public final class LocationTimeZoneEvent implements Parcelable { @IntDef({ EVENT_TYPE_UNKNOWN, EVENT_TYPE_PERMANENT_FAILURE, EVENT_TYPE_SUCCESS, EVENT_TYPE_UNCERTAIN }) - @interface EventType {} + public @interface EventType {} /** Uninitialized value for {@link #mEventType} - must not be used for real events. */ private static final int EVENT_TYPE_UNKNOWN = 0; @@ -49,8 +51,8 @@ public final class LocationTimeZoneEvent implements Parcelable { public static final int EVENT_TYPE_PERMANENT_FAILURE = 1; /** - * Indicates a successful geolocation time zone detection event. {@link #mTimeZoneIds} will be - * non-null but can legitimately be empty, e.g. for disputed areas, oceans. + * Indicates a successful geolocation time zone detection event. {@link #getTimeZoneIds()} will + * be non-null but can legitimately be empty, e.g. for disputed areas, oceans. */ public static final int EVENT_TYPE_SUCCESS = 2; @@ -81,10 +83,7 @@ public final class LocationTimeZoneEvent implements Parcelable { mTimeZoneIds = immutableList(timeZoneIds); boolean emptyTimeZoneIdListExpected = eventType != EVENT_TYPE_SUCCESS; - if (emptyTimeZoneIdListExpected && !timeZoneIds.isEmpty()) { - throw new IllegalStateException( - "timeZoneIds must only have values when eventType is success"); - } + Preconditions.checkState(!emptyTimeZoneIdListExpected || timeZoneIds.isEmpty()); mElapsedRealtimeNanos = elapsedRealtimeNanos; } @@ -102,9 +101,7 @@ public final class LocationTimeZoneEvent implements Parcelable { * *

This value can be reliably compared to {@link * android.os.SystemClock#elapsedRealtimeNanos}, to calculate the age of a fix and to compare - * {@link LocationTimeZoneEvent} fixes. This is reliable because elapsed real-time is guaranteed - * monotonic for each system boot and continues to increment even when the system is in deep - * sleep. + * {@link LocationTimeZoneEvent} instances. * * @return elapsed real-time of fix, in nanoseconds since system boot. */ diff --git a/location/java/com/android/internal/location/timezone/LocationTimeZoneProviderRequest.java b/location/java/com/android/internal/location/timezone/LocationTimeZoneProviderRequest.java index 2a37ef882e619..5c9d2908f5dff 100644 --- a/location/java/com/android/internal/location/timezone/LocationTimeZoneProviderRequest.java +++ b/location/java/com/android/internal/location/timezone/LocationTimeZoneProviderRequest.java @@ -30,7 +30,9 @@ import java.util.Objects; public final class LocationTimeZoneProviderRequest implements Parcelable { public static final LocationTimeZoneProviderRequest EMPTY_REQUEST = - new LocationTimeZoneProviderRequest(false); + new LocationTimeZoneProviderRequest( + false /* reportLocationTimeZone */, + 0 /* initializationTimeoutMillis */); public static final Creator CREATOR = new Creator() { @@ -45,17 +47,36 @@ public final class LocationTimeZoneProviderRequest implements Parcelable { } }; - /** Location time zone reporting is requested (true) */ private final boolean mReportLocationTimeZone; - private LocationTimeZoneProviderRequest(boolean reportLocationTimeZone) { + private final long mInitializationTimeoutMillis; + + private LocationTimeZoneProviderRequest( + boolean reportLocationTimeZone, long initializationTimeoutMillis) { mReportLocationTimeZone = reportLocationTimeZone; + mInitializationTimeoutMillis = initializationTimeoutMillis; } + /** + * Returns {@code true} if the provider should report events related to the device's current + * time zone, {@code false} otherwise. + */ public boolean getReportLocationTimeZone() { return mReportLocationTimeZone; } + // TODO(b/152744911) - once there are a couple of implementations, decide whether this needs to + // be passed to the LocationTimeZoneProvider and remove if it is not useful. + /** + * Returns the maximum time that the provider is allowed to initialize before it is expected to + * send an event of any sort. Only valid when {@link #getReportLocationTimeZone()} is {@code + * true}. Failure to send an event in this time (with some fuzz) may be interpreted as if the + * provider is uncertain of the time zone, and/or it could lead to the provider being disabled. + */ + public long getInitializationTimeoutMillis() { + return mInitializationTimeoutMillis; + } + @Override public int describeContents() { return 0; @@ -63,14 +84,15 @@ public final class LocationTimeZoneProviderRequest implements Parcelable { @Override public void writeToParcel(Parcel parcel, int flags) { - parcel.writeInt(mReportLocationTimeZone ? 1 : 0); + parcel.writeBoolean(mReportLocationTimeZone); + parcel.writeLong(mInitializationTimeoutMillis); } static LocationTimeZoneProviderRequest createFromParcel(Parcel in) { - ClassLoader classLoader = LocationTimeZoneProviderRequest.class.getClassLoader(); - return new Builder() - .setReportLocationTimeZone(in.readInt() == 1) - .build(); + boolean reportLocationTimeZone = in.readBoolean(); + long initializationTimeoutMillis = in.readLong(); + return new LocationTimeZoneProviderRequest( + reportLocationTimeZone, initializationTimeoutMillis); } @Override @@ -82,31 +104,28 @@ public final class LocationTimeZoneProviderRequest implements Parcelable { return false; } LocationTimeZoneProviderRequest that = (LocationTimeZoneProviderRequest) o; - return mReportLocationTimeZone == that.mReportLocationTimeZone; + return mReportLocationTimeZone == that.mReportLocationTimeZone + && mInitializationTimeoutMillis == that.mInitializationTimeoutMillis; } @Override public int hashCode() { - return Objects.hash(mReportLocationTimeZone); + return Objects.hash(mReportLocationTimeZone, mInitializationTimeoutMillis); } @Override public String toString() { - StringBuilder s = new StringBuilder(); - s.append("TimeZoneProviderRequest["); - if (mReportLocationTimeZone) { - s.append("ON"); - } else { - s.append("OFF"); - } - s.append(']'); - return s.toString(); + return "LocationTimeZoneProviderRequest{" + + "mReportLocationTimeZone=" + mReportLocationTimeZone + + ", mInitializationTimeoutMillis=" + mInitializationTimeoutMillis + + "}"; } /** @hide */ public static final class Builder { private boolean mReportLocationTimeZone; + private long mInitializationTimeoutMillis; /** * Sets the property that enables / disables the provider. This is set to {@code false} by @@ -117,10 +136,20 @@ public final class LocationTimeZoneProviderRequest implements Parcelable { return this; } + /** + * Sets the initialization timeout. See {@link + * LocationTimeZoneProviderRequest#getInitializationTimeoutMillis()} for details. + */ + public Builder setInitializationTimeoutMillis(long timeoutMillis) { + mInitializationTimeoutMillis = timeoutMillis; + return this; + } + /** Builds the {@link LocationTimeZoneProviderRequest} instance. */ @NonNull public LocationTimeZoneProviderRequest build() { - return new LocationTimeZoneProviderRequest(this.mReportLocationTimeZone); + return new LocationTimeZoneProviderRequest( + mReportLocationTimeZone, mInitializationTimeoutMillis); } } } diff --git a/location/lib/Android.bp b/location/lib/Android.bp index cd45e8e6ffa67..c0188c0df4974 100644 --- a/location/lib/Android.bp +++ b/location/lib/Android.bp @@ -20,5 +20,8 @@ java_sdk_library { libs: [ "androidx.annotation_annotation", ], - api_packages: ["com.android.location.provider"], + api_packages: [ + "com.android.location.provider", + "com.android.location.timezone.provider", + ], } diff --git a/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneEventUnbundled.java b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneEventUnbundled.java new file mode 100644 index 0000000000000..367557442829c --- /dev/null +++ b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneEventUnbundled.java @@ -0,0 +1,172 @@ +/* + * Copyright (C) 2020 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.location.timezone.provider; + +import android.annotation.IntDef; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.app.ActivityManager; +import android.location.timezone.LocationTimeZoneEvent; +import android.os.SystemClock; +import android.os.UserHandle; + +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * An event from a {@link LocationTimeZoneProviderBase} sent while determining a device's time zone + * using its location. + * + * @hide + */ +public final class LocationTimeZoneEventUnbundled { + + @IntDef({ EVENT_TYPE_PERMANENT_FAILURE, EVENT_TYPE_SUCCESS, EVENT_TYPE_UNCERTAIN }) + @interface EventType {} + + /** + * Indicates there was a permanent failure. This is not generally expected, and probably means a + * required backend service has been turned down, or the client is unreasonably old. + */ + public static final int EVENT_TYPE_PERMANENT_FAILURE = + LocationTimeZoneEvent.EVENT_TYPE_PERMANENT_FAILURE; + + /** + * Indicates a successful geolocation time zone detection event. {@link #getTimeZoneIds()} will + * be non-null but can legitimately be empty, e.g. for disputed areas, oceans. + */ + public static final int EVENT_TYPE_SUCCESS = LocationTimeZoneEvent.EVENT_TYPE_SUCCESS; + + /** + * Indicates the time zone is not known because of an expected runtime state or error, e.g. when + * the provider is unable to detect location, or there was a problem when resolving the location + * to a time zone. + */ + public static final int EVENT_TYPE_UNCERTAIN = LocationTimeZoneEvent.EVENT_TYPE_UNCERTAIN; + + @NonNull + private final LocationTimeZoneEvent mDelegate; + + private LocationTimeZoneEventUnbundled(@NonNull LocationTimeZoneEvent delegate) { + mDelegate = Objects.requireNonNull(delegate); + } + + /** + * Returns the event type. + */ + @Nullable + public @EventType int getEventType() { + return mDelegate.getEventType(); + } + + /** + * Gets the time zone IDs of this event. Contains zero or more IDs for a successful lookup. + * The value is undefined for an unsuccessful lookup. See also {@link #getEventType()}. + */ + @NonNull + public List getTimeZoneIds() { + return mDelegate.getTimeZoneIds(); + } + + /** + * Returns the information from this as a {@link LocationTimeZoneEvent}. + * @hide + */ + @NonNull + public LocationTimeZoneEvent getInternalLocationTimeZoneEvent() { + return mDelegate; + } + + @Override + public String toString() { + return "LocationTimeZoneEventUnbundled{" + + "mDelegate=" + mDelegate + + '}'; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LocationTimeZoneEventUnbundled that = (LocationTimeZoneEventUnbundled) o; + return mDelegate.equals(that.mDelegate); + } + + @Override + public int hashCode() { + return mDelegate.hashCode(); + } + + /** + * A builder of {@link LocationTimeZoneEventUnbundled} instances. + * + * @hide + */ + public static final class Builder { + + private @EventType int mEventType; + private @NonNull List mTimeZoneIds = Collections.emptyList(); + + /** + * Set the time zone ID of this event. + */ + @NonNull + public Builder setEventType(@EventType int eventType) { + checkValidEventType(eventType); + mEventType = eventType; + return this; + } + + /** + * Sets the time zone IDs of this event. + */ + @NonNull + public Builder setTimeZoneIds(@NonNull List timeZoneIds) { + mTimeZoneIds = Objects.requireNonNull(timeZoneIds); + return this; + } + + /** + * Builds a {@link LocationTimeZoneEventUnbundled} instance. + */ + @NonNull + public LocationTimeZoneEventUnbundled build() { + final int internalEventType = this.mEventType; + LocationTimeZoneEvent event = new LocationTimeZoneEvent.Builder() + .setUserHandle(UserHandle.of(ActivityManager.getCurrentUser())) + .setEventType(internalEventType) + .setTimeZoneIds(mTimeZoneIds) + .setElapsedRealtimeNanos(SystemClock.elapsedRealtimeNanos()) + .build(); + return new LocationTimeZoneEventUnbundled(event); + } + } + + private static int checkValidEventType(int eventType) { + if (eventType != EVENT_TYPE_SUCCESS + && eventType != EVENT_TYPE_UNCERTAIN + && eventType != EVENT_TYPE_PERMANENT_FAILURE) { + throw new IllegalStateException("eventType=" + eventType); + } + return eventType; + } +} diff --git a/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderBase.java b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderBase.java index c533c20d07e33..9df71664ee1bf 100644 --- a/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderBase.java +++ b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderBase.java @@ -18,7 +18,6 @@ package com.android.location.timezone.provider; import android.annotation.Nullable; import android.content.Context; -import android.location.timezone.LocationTimeZoneEvent; import android.os.IBinder; import android.os.RemoteException; import android.util.Log; @@ -30,12 +29,34 @@ import com.android.internal.location.timezone.LocationTimeZoneProviderRequest; import java.util.Objects; /** - * Base class for location time zone providers implemented as unbundled services. + * A base class for location time zone providers implemented as unbundled services. * - * TODO(b/152744911): Provide details of the expected service actions and threading. + *

Provider implementations are enabled / disabled via a call to {@link + * #onSetRequest(LocationTimeZoneProviderRequestUnbundled)}. + * + *

Once enabled, providers are expected to detect the time zone if possible, and report the + * result via {@link #reportLocationTimeZoneEvent(LocationTimeZoneEventUnbundled)} with a type of + * either {@link LocationTimeZoneEventUnbundled#EVENT_TYPE_UNCERTAIN} or {@link + * LocationTimeZoneEventUnbundled#EVENT_TYPE_SUCCESS}. Providers may also report that they have + * permanently failed by sending an event of type {@link + * LocationTimeZoneEventUnbundled#EVENT_TYPE_PERMANENT_FAILURE}. See the javadocs for each event + * type for details. + * + *

Providers are expected to issue their first event within {@link + * LocationTimeZoneProviderRequest#getInitializationTimeoutMillis()}. + * + *

Once disabled or have failed, providers are required to stop producing events. + * + *

Threading: + * + *

Calls to {@link #reportLocationTimeZoneEvent(LocationTimeZoneEventUnbundled)} can be made on + * on any thread, but may be processed asynchronously by the system server. Similarly, calls to + * {@link #onSetRequest(LocationTimeZoneProviderRequestUnbundled)} may occur on any thread. * *

IMPORTANT: This class is effectively a public API for unbundled applications, and must remain * API stable. + * + * @hide */ public abstract class LocationTimeZoneProviderBase { @@ -64,11 +85,11 @@ public abstract class LocationTimeZoneProviderBase { /** * Reports a new location time zone event from this provider. */ - public void reportLocationTimeZoneEvent(LocationTimeZoneEvent locationTimeZoneEvent) { + protected void reportLocationTimeZoneEvent(LocationTimeZoneEventUnbundled event) { ILocationTimeZoneProviderManager manager = mManager; if (manager != null) { try { - manager.onLocationTimeZoneEvent(locationTimeZoneEvent); + manager.onLocationTimeZoneEvent(event.getInternalLocationTimeZoneEvent()); } catch (RemoteException | RuntimeException e) { Log.w(mTag, e); } diff --git a/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderRequestUnbundled.java b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderRequestUnbundled.java index e898bbf3ecc0c..ab50dc3b00fce 100644 --- a/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderRequestUnbundled.java +++ b/location/lib/java/com/android/location/timezone/provider/LocationTimeZoneProviderRequestUnbundled.java @@ -34,15 +34,30 @@ public final class LocationTimeZoneProviderRequestUnbundled { private final LocationTimeZoneProviderRequest mRequest; + /** @hide */ public LocationTimeZoneProviderRequestUnbundled( @NonNull LocationTimeZoneProviderRequest request) { mRequest = Objects.requireNonNull(request); } + /** + * Returns {@code true} if the provider should report events related to the device's current + * time zone, {@code false} otherwise. + */ public boolean getReportLocationTimeZone() { return mRequest.getReportLocationTimeZone(); } + /** + * Returns the maximum time that the provider is allowed to initialize before it is expected to + * send an event of any sort. Only valid when {@link #getReportLocationTimeZone()} is {@code + * true}. Failure to send an event in this time (with some fuzz) may be interpreted as if the + * provider is uncertain of the time zone, and/or it could lead to the provider being disabled. + */ + public long getInitializationTimeoutMillis() { + return mRequest.getInitializationTimeoutMillis(); + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java index 92dabe337d87c..e28d73e8e0b98 100644 --- a/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/BinderLocationTimeZoneProvider.java @@ -29,6 +29,7 @@ import android.util.Slog; import com.android.internal.location.timezone.LocationTimeZoneProviderRequest; +import java.time.Duration; import java.util.Objects; /** @@ -142,14 +143,13 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider { } @Override - void onEnable() { + void onEnable(@NonNull Duration initializationTimeout) { // Set a request on the proxy - it will be sent immediately if the service is bound, // or will be sent as soon as the service becomes bound. - // TODO(b/152744911): Decide whether to send a timeout so the provider knows how long - // it has to generate the first event before it could be bypassed. LocationTimeZoneProviderRequest request = new LocationTimeZoneProviderRequest.Builder() .setReportLocationTimeZone(true) + .setInitializationTimeoutMillis(initializationTimeout.toMillis()) .build(); mProxy.setRequest(request); } diff --git a/services/core/java/com/android/server/location/timezone/ControllerEnvironmentImpl.java b/services/core/java/com/android/server/location/timezone/ControllerEnvironmentImpl.java index 2e2481c30f222..b1e3306681944 100644 --- a/services/core/java/com/android/server/location/timezone/ControllerEnvironmentImpl.java +++ b/services/core/java/com/android/server/location/timezone/ControllerEnvironmentImpl.java @@ -22,6 +22,7 @@ import com.android.server.LocalServices; import com.android.server.timezonedetector.ConfigurationInternal; import com.android.server.timezonedetector.TimeZoneDetectorInternal; +import java.time.Duration; import java.util.Objects; /** @@ -30,6 +31,10 @@ import java.util.Objects; */ class ControllerEnvironmentImpl extends LocationTimeZoneProviderController.Environment { + private static final Duration PROVIDER_INITIALIZATION_TIMEOUT = Duration.ofMinutes(5); + private static final Duration PROVIDER_INITIALIZATION_TIMEOUT_FUZZ = Duration.ofMinutes(1); + private static final Duration PROVIDER_UNCERTAINTY_DELAY = Duration.ofMinutes(5); + @NonNull private final TimeZoneDetectorInternal mTimeZoneDetectorInternal; @NonNull private final LocationTimeZoneProviderController mController; @@ -45,7 +50,26 @@ class ControllerEnvironmentImpl extends LocationTimeZoneProviderController.Envir } @Override + @NonNull ConfigurationInternal getCurrentUserConfigurationInternal() { return mTimeZoneDetectorInternal.getCurrentUserConfigurationInternal(); } + + @Override + @NonNull + Duration getProviderInitializationTimeout() { + return PROVIDER_INITIALIZATION_TIMEOUT; + } + + @Override + @NonNull + Duration getProviderInitializationTimeoutFuzz() { + return PROVIDER_INITIALIZATION_TIMEOUT_FUZZ; + } + + @Override + @NonNull + Duration getUncertaintyDelay() { + return PROVIDER_UNCERTAINTY_DELAY; + } } diff --git a/services/core/java/com/android/server/location/timezone/ControllerImpl.java b/services/core/java/com/android/server/location/timezone/ControllerImpl.java index e31cfc4e8b408..bedaedacf0e3d 100644 --- a/services/core/java/com/android/server/location/timezone/ControllerImpl.java +++ b/services/core/java/com/android/server/location/timezone/ControllerImpl.java @@ -33,7 +33,6 @@ import android.location.timezone.LocationTimeZoneEvent; import android.util.IndentingPrintWriter; import com.android.internal.annotations.GuardedBy; -import com.android.internal.annotations.VisibleForTesting; import com.android.server.location.timezone.ThreadingDomain.SingleRunnableQueue; import com.android.server.timezonedetector.ConfigurationInternal; import com.android.server.timezonedetector.GeolocationTimeZoneSuggestion; @@ -50,9 +49,6 @@ import java.util.Objects; */ class ControllerImpl extends LocationTimeZoneProviderController { - @VisibleForTesting - static final Duration UNCERTAINTY_DELAY = Duration.ofMinutes(5); - @NonNull private final LocationTimeZoneProvider mProvider; @NonNull private final SingleRunnableQueue mDelayedSuggestionQueue; @@ -140,7 +136,8 @@ class ControllerImpl extends LocationTimeZoneProviderController { switch (providerState.stateEnum) { case PROVIDER_STATE_DISABLED: { debugLog("Enabling " + mProvider); - mProvider.enable(configuration); + mProvider.enable( + configuration, mEnvironment.getProviderInitializationTimeout()); break; } case PROVIDER_STATE_ENABLED: { @@ -183,13 +180,16 @@ class ControllerImpl extends LocationTimeZoneProviderController { if (isProviderEnabled) { if (!providerWasEnabled) { // When a provider has first been enabled, we allow it some time for it to - // initialize. + // initialize before sending its first event. + Duration initializationTimeout = mEnvironment.getProviderInitializationTimeout() + .plus(mEnvironment.getProviderInitializationTimeoutFuzz()); // This sets up an empty suggestion to trigger if no explicit "certain" or - // "uncertain" suggestion preempts it within UNCERTAINTY_DELAY. If, for some reason, - // the provider does provide any events then this scheduled suggestion will ensure - // the controller makes at least an uncertain suggestion. - suggestDelayed(createEmptySuggestion( - "No event received in delay=" + UNCERTAINTY_DELAY), UNCERTAINTY_DELAY); + // "uncertain" suggestion preempts it within initializationTimeout. If, for some + // reason, the provider does not produce any events then this scheduled suggestion + // will ensure the controller makes at least an "uncertain" suggestion. + suggestDelayed(createEmptySuggestion("No event received from provider in" + + " initializationTimeout=" + initializationTimeout), + initializationTimeout); } } else { // Clear any queued suggestions. @@ -199,7 +199,8 @@ class ControllerImpl extends LocationTimeZoneProviderController { // made, then a new "uncertain" suggestion must be made to indicate the provider no // longer has an opinion and will not be sending updates. if (mLastSuggestion != null && mLastSuggestion.getZoneIds() != null) { - suggestImmediate(createEmptySuggestion("")); + suggestImmediate(createEmptySuggestion( + "Provider disabled, clearing previous suggestion")); } } } @@ -309,21 +310,23 @@ class ControllerImpl extends LocationTimeZoneProviderController { * *

Providers are expected to report their uncertainty as soon as they become uncertain, as * this enables the most flexibility for the controller to enable other providers when there are - * multiple ones. The controller is therefore responsible for deciding when to make a + * multiple ones available. The controller is therefore responsible for deciding when to make a * "uncertain" suggestion. * *

This method schedules an "uncertain" suggestion (if one isn't already scheduled) to be * made later if nothing else preempts it. It can be preempted if the provider becomes certain * (or does anything else that calls {@link #suggestImmediate(GeolocationTimeZoneSuggestion)}) - * within UNCERTAINTY_DELAY. Preemption causes the scheduled "uncertain" event to be cancelled. - * If the provider repeatedly sends uncertainty events within UNCERTAINTY_DELAY, those events - * are effectively ignored (i.e. the timer is not reset each time). + * within {@link Environment#getUncertaintyDelay()}. Preemption causes the scheduled + * "uncertain" event to be cancelled. If the provider repeatedly sends uncertainty events within + * the uncertainty delay period, those events are effectively ignored (i.e. the timer is not + * reset each time). */ private void scheduleUncertainSuggestionIfNeeded(@Nullable LocationTimeZoneEvent event) { if (mPendingSuggestion == null || mPendingSuggestion.getZoneIds() != null) { GeolocationTimeZoneSuggestion suggestion = createEmptySuggestion( "provider=" + mProvider + " became uncertain, event=" + event); - suggestDelayed(suggestion, UNCERTAINTY_DELAY); + // Only send the empty suggestion after the uncertainty delay. + suggestDelayed(suggestion, mEnvironment.getUncertaintyDelay()); } } @@ -334,6 +337,11 @@ class ControllerImpl extends LocationTimeZoneProviderController { ipw.increaseIndent(); // level 1 ipw.println("mCurrentUserConfiguration=" + mCurrentUserConfiguration); + ipw.println("providerInitializationTimeout=" + + mEnvironment.getProviderInitializationTimeout()); + ipw.println("providerInitializationTimeoutFuzz=" + + mEnvironment.getProviderInitializationTimeoutFuzz()); + ipw.println("uncertaintyDelay=" + mEnvironment.getUncertaintyDelay()); ipw.println("mPendingSuggestion=" + mPendingSuggestion); ipw.println("mLastSuggestion=" + mLastSuggestion); diff --git a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java index 3743779b20c91..abfa580b5c980 100644 --- a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProvider.java @@ -37,6 +37,7 @@ import com.android.server.timezonedetector.ConfigurationInternal; import com.android.server.timezonedetector.Dumpable; import com.android.server.timezonedetector.ReferenceWithHistory; +import java.time.Duration; import java.util.Objects; /** @@ -368,7 +369,8 @@ abstract class LocationTimeZoneProvider implements Dumpable { * #getCurrentState()} is at {@link ProviderState#PROVIDER_STATE_DISABLED}. This method must be * called using the handler thread from the {@link ThreadingDomain}. */ - final void enable(@NonNull ConfigurationInternal currentUserConfiguration) { + final void enable(@NonNull ConfigurationInternal currentUserConfiguration, + @NonNull Duration initializationTimeout) { mThreadingDomain.assertCurrentThread(); synchronized (mSharedLock) { @@ -378,14 +380,14 @@ abstract class LocationTimeZoneProvider implements Dumpable { ProviderState newState = currentState.newState( PROVIDER_STATE_ENABLED, null, currentUserConfiguration, "enable() called"); setCurrentState(newState, false); - onEnable(); + onEnable(initializationTimeout); } } /** * Implemented by subclasses to do work during {@link #enable}. */ - abstract void onEnable(); + abstract void onEnable(@NonNull Duration initializationTimeout); /** * Disables the provider. It is an error* to call this method except when the {@link diff --git a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderController.java b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderController.java index 2f75c43594df6..88f0f00015c6f 100644 --- a/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderController.java +++ b/services/core/java/com/android/server/location/timezone/LocationTimeZoneProviderController.java @@ -24,6 +24,7 @@ import com.android.server.timezonedetector.ConfigurationInternal; import com.android.server.timezonedetector.Dumpable; import com.android.server.timezonedetector.GeolocationTimeZoneSuggestion; +import java.time.Duration; import java.util.Objects; /** @@ -100,6 +101,24 @@ abstract class LocationTimeZoneProviderController implements Dumpable { /** Returns the {@link ConfigurationInternal} for the current user of the device. */ abstract ConfigurationInternal getCurrentUserConfigurationInternal(); + + /** + * Returns the value passed to LocationTimeZoneProviders informing them of how long they + * have to return their first time zone suggestion. + */ + abstract Duration getProviderInitializationTimeout(); + + /** + * Returns the extra time granted on top of {@link #getProviderInitializationTimeout()} to + * allow for slop like communication delays. + */ + abstract Duration getProviderInitializationTimeoutFuzz(); + + /** + * Returns the delay allowed after receiving uncertainty from a provider before it should be + * passed on. + */ + abstract Duration getUncertaintyDelay(); } /** diff --git a/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java b/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java index 79e2b975089d1..53afaf06f2983 100644 --- a/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/location/timezone/NullLocationTimeZoneProvider.java @@ -23,6 +23,8 @@ import android.annotation.Nullable; import android.util.IndentingPrintWriter; import android.util.Slog; +import java.time.Duration; + /** * A {@link LocationTimeZoneProvider} that provides minimal responses needed for the {@link * LocationTimeZoneProviderController} to operate correctly when there is no "real" provider @@ -55,7 +57,7 @@ class NullLocationTimeZoneProvider extends LocationTimeZoneProvider { } @Override - void onEnable() { + void onEnable(@NonNull Duration initializationTimeout) { // Report a failure (asynchronously using the mThreadingDomain thread to avoid recursion). mThreadingDomain.post(()-> { // Enter the perm-failed state. diff --git a/services/tests/servicestests/src/com/android/internal/location/timezone/LocationTimeZoneProviderRequestTest.java b/services/tests/servicestests/src/com/android/internal/location/timezone/LocationTimeZoneProviderRequestTest.java index 1fa1b8fc06196..75696dac3b44d 100644 --- a/services/tests/servicestests/src/com/android/internal/location/timezone/LocationTimeZoneProviderRequestTest.java +++ b/services/tests/servicestests/src/com/android/internal/location/timezone/LocationTimeZoneProviderRequestTest.java @@ -20,16 +20,20 @@ import static android.location.timezone.ParcelableTestSupport.assertRoundTripPar import org.junit.Test; +import java.time.Duration; + public class LocationTimeZoneProviderRequestTest { @Test public void testParcelable() { LocationTimeZoneProviderRequest.Builder builder = new LocationTimeZoneProviderRequest.Builder() - .setReportLocationTimeZone(true); + .setReportLocationTimeZone(false); assertRoundTripParcelable(builder.build()); - builder.setReportLocationTimeZone(false); + builder.setReportLocationTimeZone(true) + .setInitializationTimeoutMillis(Duration.ofMinutes(5).toMillis()); + assertRoundTripParcelable(builder.build()); } } diff --git a/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java b/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java index dbaad6633e98d..631b8d3cb1ea0 100644 --- a/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java +++ b/services/tests/servicestests/src/com/android/server/location/timezone/ControllerImplTest.java @@ -18,7 +18,6 @@ package com.android.server.location.timezone; import static android.location.timezone.LocationTimeZoneEvent.EVENT_TYPE_SUCCESS; import static android.location.timezone.LocationTimeZoneEvent.EVENT_TYPE_UNCERTAIN; -import static com.android.server.location.timezone.ControllerImpl.UNCERTAINTY_DELAY; import static com.android.server.location.timezone.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_DISABLED; import static com.android.server.location.timezone.LocationTimeZoneProvider.ProviderState.PROVIDER_STATE_ENABLED; import static com.android.server.location.timezone.TestSupport.USER1_CONFIG_GEO_DETECTION_DISABLED; @@ -47,6 +46,7 @@ import com.android.server.timezonedetector.TestState; import org.junit.Before; import org.junit.Test; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -92,7 +92,8 @@ public class ControllerImplTest { mTestLocationTimeZoneProvider.assertInitialized(); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertNextQueueItemIsDelayed(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); } @@ -121,7 +122,8 @@ public class ControllerImplTest { mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); mTestCallback.assertNoSuggestionMade(); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); // Simulate time passing with no event being received. mTestThreadingDomain.executeNext(); @@ -140,7 +142,8 @@ public class ControllerImplTest { controllerImpl.initialize(testEnvironment, mTestCallback); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Simulate a location event being received by the provider. This should cause a suggestion @@ -163,7 +166,8 @@ public class ControllerImplTest { controllerImpl.initialize(testEnvironment, mTestCallback); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Simulate a location event being received by the provider. This should cause a suggestion @@ -203,7 +207,8 @@ public class ControllerImplTest { controllerImpl.initialize(testEnvironment, mTestCallback); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Simulate a location event being received by the provider. This should cause a suggestion @@ -216,15 +221,16 @@ public class ControllerImplTest { mTestCallback.assertSuggestionMadeAndCommit( USER1_SUCCESS_LOCATION_TIME_ZONE_EVENT1.getTimeZoneIds()); - // Uncertainty should cause + // Uncertainty should cause a suggestion to (only) be queued. mTestLocationTimeZoneProvider.simulateLocationTimeZoneEvent( USER1_UNCERTAIN_LOCATION_TIME_ZONE_EVENT); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + mTestThreadingDomain.assertSingleDelayedQueueItem(testEnvironment.getUncertaintyDelay()); mTestCallback.assertNoSuggestionMade(); - // And a third event should cause yet another suggestion. + // And a third event should cause yet another suggestion and for the queued item to be + // removed. mTestLocationTimeZoneProvider.simulateLocationTimeZoneEvent( USER1_SUCCESS_LOCATION_TIME_ZONE_EVENT2); @@ -250,7 +256,8 @@ public class ControllerImplTest { testEnvironment.simulateConfigChange(USER1_CONFIG_GEO_DETECTION_ENABLED); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertNextQueueItemIsDelayed(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Now signal a config change so that geo detection is disabled. @@ -270,7 +277,8 @@ public class ControllerImplTest { controllerImpl.initialize(testEnvironment, mTestCallback); mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Simulate a location event being received by the provider. This should cause a suggestion @@ -304,7 +312,8 @@ public class ControllerImplTest { // There should be a runnable scheduled to suggest uncertainty if no event is received. mTestLocationTimeZoneProvider.assertIsEnabled(USER1_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + Duration expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Have the provider suggest a time zone. @@ -328,7 +337,8 @@ public class ControllerImplTest { int[] expectedStateTransitions = { PROVIDER_STATE_DISABLED, PROVIDER_STATE_ENABLED }; mTestLocationTimeZoneProvider.assertStateChangesAndCommit(expectedStateTransitions); mTestLocationTimeZoneProvider.assertConfig(USER2_CONFIG_GEO_DETECTION_ENABLED); - mTestThreadingDomain.assertSingleDelayedQueueItem(UNCERTAINTY_DELAY); + expectedTimeout = expectedProviderInitializationTimeout(); + mTestThreadingDomain.assertSingleDelayedQueueItem(expectedTimeout); mTestCallback.assertNoSuggestionMade(); // Simulate no event being received, and time passing. @@ -351,8 +361,18 @@ public class ControllerImplTest { return builder.build(); } + + private Duration expectedProviderInitializationTimeout() { + return TestEnvironment.PROVIDER_INITIALIZATION_TIMEOUT + .plus(TestEnvironment.PROVIDER_INITIALIZATION_TIMEOUT_FUZZ); + } + private static class TestEnvironment extends LocationTimeZoneProviderController.Environment { + static final Duration PROVIDER_INITIALIZATION_TIMEOUT = Duration.ofMinutes(5); + static final Duration PROVIDER_INITIALIZATION_TIMEOUT_FUZZ = Duration.ofMinutes(1); + private static final Duration UNCERTAINTY_DELAY = Duration.ofMinutes(3); + private final LocationTimeZoneProviderController mController; private ConfigurationInternal mConfigurationInternal; @@ -369,6 +389,21 @@ public class ControllerImplTest { return mConfigurationInternal; } + @Override + Duration getProviderInitializationTimeout() { + return PROVIDER_INITIALIZATION_TIMEOUT; + } + + @Override + Duration getProviderInitializationTimeoutFuzz() { + return PROVIDER_INITIALIZATION_TIMEOUT_FUZZ; + } + + @Override + Duration getUncertaintyDelay() { + return UNCERTAINTY_DELAY; + } + void simulateConfigChange(ConfigurationInternal newConfig) { ConfigurationInternal oldConfig = mConfigurationInternal; mConfigurationInternal = Objects.requireNonNull(newConfig); @@ -432,7 +467,7 @@ public class ControllerImplTest { } @Override - void onEnable() { + void onEnable(Duration initializationTimeout) { // Nothing needed for tests. } @@ -464,7 +499,7 @@ public class ControllerImplTest { /** * Asserts the provider's config matches the expected, and the current state is set - * accordinly. Commits the latest changes to the state. + * accordingly. Commits the latest changes to the state. */ void assertIsEnabled(@NonNull ConfigurationInternal expectedConfig) { assertConfig(expectedConfig); diff --git a/services/tests/servicestests/src/com/android/server/location/timezone/NullLocationTimeZoneProviderTest.java b/services/tests/servicestests/src/com/android/server/location/timezone/NullLocationTimeZoneProviderTest.java index 7c882fc1f1543..5542db0b5ae3d 100644 --- a/services/tests/servicestests/src/com/android/server/location/timezone/NullLocationTimeZoneProviderTest.java +++ b/services/tests/servicestests/src/com/android/server/location/timezone/NullLocationTimeZoneProviderTest.java @@ -34,6 +34,8 @@ import com.android.server.timezonedetector.TestState; import org.junit.Before; import org.junit.Test; +import java.time.Duration; + /** * Tests for {@link NullLocationTimeZoneProvider} and, indirectly, the class it extends * {@link LocationTimeZoneProvider}. @@ -73,7 +75,8 @@ public class NullLocationTimeZoneProviderTest { provider.initialize(providerState -> mTestController.onProviderStateChange(providerState)); ConfigurationInternal config = USER1_CONFIG_GEO_DETECTION_ENABLED; - provider.enable(config); + Duration arbitraryInitializationTimeout = Duration.ofMinutes(5); + provider.enable(config, arbitraryInitializationTimeout); // The StubbedProvider should enters enabled state, but immediately schedule a runnable to // switch to perm failure. diff --git a/services/tests/servicestests/src/com/android/server/location/timezone/TestThreadingDomain.java b/services/tests/servicestests/src/com/android/server/location/timezone/TestThreadingDomain.java index 70ff22d175136..def919e79731a 100644 --- a/services/tests/servicestests/src/com/android/server/location/timezone/TestThreadingDomain.java +++ b/services/tests/servicestests/src/com/android/server/location/timezone/TestThreadingDomain.java @@ -105,8 +105,8 @@ class TestThreadingDomain extends ThreadingDomain { assertTrue(getNextQueueItemDelayMillis() == 0); } - void assertNextQueueItemIsDelayed(Duration expectedDelay) { - assertTrue(getNextQueueItemDelayMillis() == expectedDelay.toMillis()); + private void assertNextQueueItemIsDelayed(Duration expectedDelay) { + assertEquals(getNextQueueItemDelayMillis(), expectedDelay.toMillis()); } void assertQueueEmpty() {