From b2837a161584a1665b6e6c010996baec4116bc58 Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Tue, 15 Nov 2022 17:05:24 +0000 Subject: [PATCH] Enable providers to report they are "missing" Previously, providers were only expected to report suggestions, not status / events. This meant that "missing" providers could pretend to be "real" until they were asked for a suggestion at which point they would "fail". However, now the LocationTimeZoneProviderController tracks provider status. It's useful (and clearer) to know if providers are present even if they haven't been asked to do something yet. In order to determine the difference between a provider that hasn't been started yet and a provider that is missing, this commit modifies LocationTimeZoneProvider's initialize() methods to return a boolean that can indicating initalization has failed. It could already throw a RuntimeException for the same result, but returning a boolean is a explicit interface, leaving the RuntimeException path still to handle coding errors and other exceptional failures. There is a new "DisabledLocationTimeZoneProvider" to cover the "there is no provider" case at the LocationTimeZoneProvider level which is hardcoded to return false from initialize. This change means that the NullLocationTimeZoneProviderProxy, which previously handled the "there is no provider" case at a deeper level, can be deleted. Bug: 236624675 Test: atest services/tests/servicestests/src/com/android/server/timezonedetector/ Test: atest services/tests/servicestests/src/com/android/server/timezonedetector/location/ Change-Id: I7841f91e7ced4b59be27592e2e7c0e3d9c3acf7a --- .../BinderLocationTimeZoneProvider.java | 3 +- .../DisabledLocationTimeZoneProvider.java | 86 +++++++++++++++++++ .../LocationTimeZoneManagerService.java | 28 +++--- .../location/LocationTimeZoneProvider.java | 19 +++- .../NullLocationTimeZoneProviderProxy.java | 74 ---------------- ...ocationTimeZoneProviderControllerTest.java | 6 +- .../LocationTimeZoneProviderTest.java | 3 +- 7 files changed, 119 insertions(+), 100 deletions(-) create mode 100644 services/core/java/com/android/server/timezonedetector/location/DisabledLocationTimeZoneProvider.java delete mode 100644 services/core/java/com/android/server/timezonedetector/location/NullLocationTimeZoneProviderProxy.java diff --git a/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java b/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java index a1de2941808e3..71aa10d8614a7 100644 --- a/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/timezonedetector/location/BinderLocationTimeZoneProvider.java @@ -53,7 +53,7 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider { } @Override - void onInitialize() { + boolean onInitialize() { mProxy.initialize(new LocationTimeZoneProviderProxy.Listener() { @Override public void onReportTimeZoneProviderEvent( @@ -71,6 +71,7 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider { handleTemporaryFailure("onProviderUnbound()"); } }); + return true; } @Override diff --git a/services/core/java/com/android/server/timezonedetector/location/DisabledLocationTimeZoneProvider.java b/services/core/java/com/android/server/timezonedetector/location/DisabledLocationTimeZoneProvider.java new file mode 100644 index 0000000000000..5d6184ec66d64 --- /dev/null +++ b/services/core/java/com/android/server/timezonedetector/location/DisabledLocationTimeZoneProvider.java @@ -0,0 +1,86 @@ +/* + * 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.timezonedetector.location; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.util.IndentingPrintWriter; + +import java.time.Duration; + +/** + * A {@link LocationTimeZoneProvider} that provides minimal responses needed to operate correctly + * when there is no "real" provider configured / enabled. This is used when the platform supports + * more providers than are needed for an Android deployment. + * + *

That is, the {@link LocationTimeZoneProviderController} supports a primary and a secondary + * {@link LocationTimeZoneProvider}, but if only a primary is configured, the secondary provider + * config will marked as "disabled" and the {@link LocationTimeZoneProvider} implementation will use + * {@link DisabledLocationTimeZoneProvider}. The {@link DisabledLocationTimeZoneProvider} fails + * initialization and immediately moves to a "permanent failure" state, which ensures the {@link + * LocationTimeZoneProviderController} correctly categorizes it and won't attempt to use it. + */ +class DisabledLocationTimeZoneProvider extends LocationTimeZoneProvider { + + DisabledLocationTimeZoneProvider( + @NonNull ProviderMetricsLogger providerMetricsLogger, + @NonNull ThreadingDomain threadingDomain, + @NonNull String providerName, + boolean recordStateChanges) { + super(providerMetricsLogger, threadingDomain, providerName, x -> x, recordStateChanges); + } + + @Override + boolean onInitialize() { + // Fail initialization, preventing further use. + return false; + } + + @Override + void onDestroy() { + } + + @Override + void onStartUpdates(@NonNull Duration initializationTimeout, + @NonNull Duration eventFilteringAgeThreshold) { + throw new UnsupportedOperationException("Provider is disabled"); + } + + @Override + void onStopUpdates() { + throw new UnsupportedOperationException("Provider is disabled"); + } + + @Override + public void dump(@NonNull IndentingPrintWriter ipw, @Nullable String[] args) { + synchronized (mSharedLock) { + ipw.println("{DisabledLocationTimeZoneProvider}"); + ipw.println("mProviderName=" + mProviderName); + ipw.println("mCurrentState=" + mCurrentState); + } + } + + @Override + public String toString() { + synchronized (mSharedLock) { + return "DisabledLocationTimeZoneProvider{" + + "mProviderName=" + mProviderName + + ", mCurrentState=" + mCurrentState + + '}'; + } + } +} diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java index 36ab111dfccb7..8d9854436e29e 100644 --- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java +++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneManagerService.java @@ -447,11 +447,18 @@ public class LocationTimeZoneManagerService extends Binder { @NonNull LocationTimeZoneProvider createProvider() { - LocationTimeZoneProviderProxy proxy = createProxy(); ProviderMetricsLogger providerMetricsLogger = new RealProviderMetricsLogger(mIndex); - return new BinderLocationTimeZoneProvider( - providerMetricsLogger, mThreadingDomain, mName, proxy, - mServiceConfigAccessor.getRecordStateChangesForTests()); + + String mode = getMode(); + if (Objects.equals(mode, PROVIDER_MODE_DISABLED)) { + return new DisabledLocationTimeZoneProvider(providerMetricsLogger, mThreadingDomain, + mName, mServiceConfigAccessor.getRecordStateChangesForTests()); + } else { + LocationTimeZoneProviderProxy proxy = createBinderProxy(); + return new BinderLocationTimeZoneProvider( + providerMetricsLogger, mThreadingDomain, mName, proxy, + mServiceConfigAccessor.getRecordStateChangesForTests()); + } } @Override @@ -460,17 +467,6 @@ public class LocationTimeZoneManagerService extends Binder { ipw.printf("getPackageName()=%s\n", getPackageName()); } - @NonNull - private LocationTimeZoneProviderProxy createProxy() { - String mode = getMode(); - if (Objects.equals(mode, PROVIDER_MODE_DISABLED)) { - return new NullLocationTimeZoneProviderProxy(mContext, mThreadingDomain); - } else { - // mode == PROVIDER_MODE_OVERRIDE_ENABLED (or unknown). - return createRealProxy(); - } - } - /** Returns the mode of the provider (enabled/disabled). */ @NonNull private String getMode() { @@ -482,7 +478,7 @@ public class LocationTimeZoneManagerService extends Binder { } @NonNull - private RealLocationTimeZoneProviderProxy createRealProxy() { + private RealLocationTimeZoneProviderProxy createBinderProxy() { String providerServiceAction = mServiceAction; boolean isTestProvider = isTestProvider(); String providerPackageName = getPackageName(); diff --git a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java index 15b57b1fbdfb6..ba7c328f1e80c 100644 --- a/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java +++ b/services/core/java/com/android/server/timezonedetector/location/LocationTimeZoneProvider.java @@ -448,13 +448,21 @@ abstract class LocationTimeZoneProvider implements Dumpable { currentState = currentState.newState(PROVIDER_STATE_STOPPED, null, null, "initialize"); setCurrentState(currentState, false); + boolean initializationSuccess; + String initializationFailureReason; // Guard against uncaught exceptions due to initialization problems. try { - onInitialize(); + initializationSuccess = onInitialize(); + initializationFailureReason = "onInitialize() returned false"; } catch (RuntimeException e) { - warnLog("Unable to initialize the provider", e); + warnLog("Unable to initialize the provider due to exception", e); + initializationSuccess = false; + initializationFailureReason = "onInitialize() threw exception:" + e.getMessage(); + } + + if (!initializationSuccess) { currentState = currentState.newState(PROVIDER_STATE_PERM_FAILED, null, null, - "Failed to initialize: " + e.getMessage()); + "Failed to initialize: " + initializationFailureReason); setCurrentState(currentState, true); } } @@ -462,9 +470,12 @@ abstract class LocationTimeZoneProvider implements Dumpable { /** * Implemented by subclasses to do work during {@link #initialize}. + * + * @return returns {@code true} on success, {@code false} if the provider should be considered + * "permanently failed" / disabled */ @GuardedBy("mSharedLock") - abstract void onInitialize(); + abstract boolean onInitialize(); /** * Destroys the provider. Called after the provider is stopped. This instance will not be called diff --git a/services/core/java/com/android/server/timezonedetector/location/NullLocationTimeZoneProviderProxy.java b/services/core/java/com/android/server/timezonedetector/location/NullLocationTimeZoneProviderProxy.java deleted file mode 100644 index 9cb1813df6db0..0000000000000 --- a/services/core/java/com/android/server/timezonedetector/location/NullLocationTimeZoneProviderProxy.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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.server.timezonedetector.location; - -import android.annotation.NonNull; -import android.annotation.Nullable; -import android.content.Context; -import android.os.SystemClock; -import android.service.timezone.TimeZoneProviderEvent; -import android.util.IndentingPrintWriter; - -/** - * A {@link LocationTimeZoneProviderProxy} that provides minimal responses needed for the {@link - * BinderLocationTimeZoneProvider} to operate correctly when there is no "real" provider - * configured / enabled. This can be used during development / testing, or in a production build - * when the platform supports more providers than are needed for an Android deployment. - * - *

For example, if the {@link LocationTimeZoneProviderController} supports a primary - * and a secondary {@link LocationTimeZoneProvider}, but only a primary is configured, the secondary - * config will be left null and the {@link LocationTimeZoneProviderProxy} implementation will be - * defaulted to a {@link NullLocationTimeZoneProviderProxy}. The {@link - * NullLocationTimeZoneProviderProxy} sends a "permanent failure" event immediately after being - * started for the first time, which ensures the {@link LocationTimeZoneProviderController} won't - * expect any further {@link TimeZoneProviderEvent}s to come from it, and won't attempt to use it - * again. - */ -class NullLocationTimeZoneProviderProxy extends LocationTimeZoneProviderProxy { - - /** Creates the instance. */ - NullLocationTimeZoneProviderProxy( - @NonNull Context context, @NonNull ThreadingDomain threadingDomain) { - super(context, threadingDomain); - } - - @Override - void onInitialize() { - // No-op - } - - @Override - void onDestroy() { - // No-op - } - - @Override - void setRequest(@NonNull TimeZoneProviderRequest request) { - if (request.sendUpdates()) { - TimeZoneProviderEvent event = TimeZoneProviderEvent.createPermanentFailureEvent( - SystemClock.elapsedRealtime(), "Provider is disabled"); - handleTimeZoneProviderEvent(event); - } - } - - @Override - public void dump(@NonNull IndentingPrintWriter ipw, @Nullable String[] args) { - synchronized (mSharedLock) { - ipw.println("{NullLocationTimeZoneProviderProxy}"); - } - } -} diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerTest.java index b08705be2eacb..7b1db953ef548 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderControllerTest.java @@ -1672,11 +1672,9 @@ public class LocationTimeZoneProviderControllerTest { } @Override - void onInitialize() { + boolean onInitialize() { mInitialized = true; - if (mFailDuringInitialization) { - throw new RuntimeException("Simulated initialization failure"); - } + return !mFailDuringInitialization; } @Override diff --git a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java index 2bee7e66c43f9..1ae74c679b53d 100644 --- a/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java +++ b/services/tests/servicestests/src/com/android/server/timezonedetector/location/LocationTimeZoneProviderTest.java @@ -330,8 +330,9 @@ public class LocationTimeZoneProviderTest { } @Override - void onInitialize() { + boolean onInitialize() { mOnInitializeCalled = true; + return true; } @Override