Move temporary failure logic up the hierarchy

BinderLocationTimeZoneProvider can temporarily fail if the remote object
dies. This was handled in BinderLocationTimeZoneProvider, but it is
being moved to LocationTimeZoneProvider in this commit.

This makes it easier to fix a bug (also fixed in this commit) where the
initialization timeout was not cancelled if the remote peer died during
initialization. This would later result in a logged warning when the
timeout fired and the LocationTimeZoneProvider was no longer in the
"initializing" state. This has only been witnessed during unit tests
while debugging.

Test: treehugger only
Bug: 188780558
Change-Id: I76a22edaf9bea4287d07c408ad43fc3e5a8e49fc
This commit is contained in:
Neil Fuller
2021-07-09 09:19:22 +01:00
parent 9d5ece2c4b
commit 3c7c2b3b85
2 changed files with 58 additions and 57 deletions

View File

@@ -67,7 +67,7 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider {
@Override
public void onProviderUnbound() {
handleProviderLost("onProviderUnbound()");
handleTemporaryFailure("onProviderUnbound()");
}
});
}
@@ -77,50 +77,6 @@ class BinderLocationTimeZoneProvider extends LocationTimeZoneProvider {
mProxy.destroy();
}
private void handleProviderLost(String reason) {
mThreadingDomain.assertCurrentThread();
synchronized (mSharedLock) {
ProviderState currentState = mCurrentState.get();
switch (currentState.stateEnum) {
case PROVIDER_STATE_STARTED_INITIALIZING:
case PROVIDER_STATE_STARTED_UNCERTAIN:
case PROVIDER_STATE_STARTED_CERTAIN: {
// Losing a remote provider is treated as becoming uncertain.
String msg = "handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState;
debugLog(msg);
// This is an unusual PROVIDER_STATE_STARTED_UNCERTAIN state because
// event == null
ProviderState newState = currentState.newState(
PROVIDER_STATE_STARTED_UNCERTAIN, null,
currentState.currentUserConfiguration, msg);
setCurrentState(newState, true);
break;
}
case PROVIDER_STATE_STOPPED: {
debugLog("handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState
+ ": No state change required, provider is stopped.");
break;
}
case PROVIDER_STATE_PERM_FAILED:
case PROVIDER_STATE_DESTROYED: {
debugLog("handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState
+ ": No state change required, provider is terminated.");
break;
}
default: {
throw new IllegalStateException("Unknown currentState=" + currentState);
}
}
}
}
private void handleOnProviderBound() {
mThreadingDomain.assertCurrentThread();

View File

@@ -592,9 +592,7 @@ abstract class LocationTimeZoneProvider implements Dumpable {
PROVIDER_STATE_STOPPED, null, null, "stopUpdates() called");
setCurrentState(newState, false);
if (mInitializationTimeoutQueue.hasQueued()) {
mInitializationTimeoutQueue.cancel();
}
cancelInitializationTimeoutIfSet();
onStopUpdates();
}
@@ -656,9 +654,7 @@ abstract class LocationTimeZoneProvider implements Dumpable {
ProviderState newState = currentState.newState(
PROVIDER_STATE_PERM_FAILED, null, null, msg);
setCurrentState(newState, true);
if (mInitializationTimeoutQueue.hasQueued()) {
mInitializationTimeoutQueue.cancel();
}
cancelInitializationTimeoutIfSet();
return;
}
case EVENT_TYPE_SUGGESTION:
@@ -691,9 +687,7 @@ abstract class LocationTimeZoneProvider implements Dumpable {
ProviderState newState = currentState.newState(
PROVIDER_STATE_PERM_FAILED, null, null, msg);
setCurrentState(newState, true);
if (mInitializationTimeoutQueue.hasQueued()) {
mInitializationTimeoutQueue.cancel();
}
cancelInitializationTimeoutIfSet();
return;
}
@@ -709,9 +703,7 @@ abstract class LocationTimeZoneProvider implements Dumpable {
timeZoneProviderEvent, currentState.currentUserConfiguration,
"handleTimeZoneProviderEvent() when started");
setCurrentState(newState, true);
if (mInitializationTimeoutQueue.hasQueued()) {
mInitializationTimeoutQueue.cancel();
}
cancelInitializationTimeoutIfSet();
return;
}
default: {
@@ -727,6 +719,52 @@ abstract class LocationTimeZoneProvider implements Dumpable {
}
}
/** For subclasses to invoke when needing to report a temporary failure. */
final void handleTemporaryFailure(String reason) {
mThreadingDomain.assertCurrentThread();
synchronized (mSharedLock) {
ProviderState currentState = mCurrentState.get();
switch (currentState.stateEnum) {
case PROVIDER_STATE_STARTED_INITIALIZING:
case PROVIDER_STATE_STARTED_UNCERTAIN:
case PROVIDER_STATE_STARTED_CERTAIN: {
// A temporary failure is treated as becoming uncertain.
String msg = "handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState;
debugLog(msg);
// This is an unusual PROVIDER_STATE_STARTED_UNCERTAIN state because
// event == null
ProviderState newState = currentState.newState(
PROVIDER_STATE_STARTED_UNCERTAIN, null,
currentState.currentUserConfiguration, msg);
setCurrentState(newState, true);
cancelInitializationTimeoutIfSet();
break;
}
case PROVIDER_STATE_STOPPED: {
debugLog("handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState
+ ": No state change required, provider is stopped.");
break;
}
case PROVIDER_STATE_PERM_FAILED:
case PROVIDER_STATE_DESTROYED: {
debugLog("handleProviderLost reason=" + reason
+ ", mProviderName=" + mProviderName
+ ", currentState=" + currentState
+ ": No state change required, provider is terminated.");
break;
}
default: {
throw new IllegalStateException("Unknown currentState=" + currentState);
}
}
}
}
@GuardedBy("mSharedLock")
private void assertIsStarted() {
ProviderState currentState = mCurrentState.get();
@@ -751,6 +789,13 @@ abstract class LocationTimeZoneProvider implements Dumpable {
}
}
@GuardedBy("mSharedLock")
private void cancelInitializationTimeoutIfSet() {
if (mInitializationTimeoutQueue.hasQueued()) {
mInitializationTimeoutQueue.cancel();
}
}
@VisibleForTesting
Duration getInitializationTimeoutDelay() {
synchronized (mSharedLock) {