Add missing Future cancellation checks

Future.isDone and getDone doesn't imply the Future was successful
and it may have been cancelled or failed.

In case where we handle failure, we should also handle cancellation
to avoid CancellationException to bubble up unchecked.

In demo app code where we use isDone for field initialization, we
want to crash in the failure case (usually security exception where
the connection is disallowed), but we want to gracefully handle
cancellation. Cancellation of these variables usually happens in
Activity.onDestroy/onStop, but methods may be called after this point.

#minor-release

PiperOrigin-RevId: 572178018
This commit is contained in:
tonihei 2023-10-10 02:12:22 -07:00 committed by Copybara-Service
parent dc859eae82
commit fe7c62afe0
5 changed files with 8 additions and 6 deletions

View File

@ -43,7 +43,7 @@ import com.google.common.util.concurrent.ListenableFuture
class PlayableFolderActivity : AppCompatActivity() {
private lateinit var browserFuture: ListenableFuture<MediaBrowser>
private val browser: MediaBrowser?
get() = if (browserFuture.isDone) browserFuture.get() else null
get() = if (browserFuture.isDone && !browserFuture.isCancelled) browserFuture.get() else null
private lateinit var mediaList: ListView
private lateinit var mediaListAdapter: PlayableMediaItemArrayAdapter

View File

@ -45,7 +45,8 @@ import com.google.common.util.concurrent.MoreExecutors
class PlayerActivity : AppCompatActivity() {
private lateinit var controllerFuture: ListenableFuture<MediaController>
private val controller: MediaController?
get() = if (controllerFuture.isDone) controllerFuture.get() else null
get() =
if (controllerFuture.isDone && !controllerFuture.isCancelled) controllerFuture.get() else null
private lateinit var playerView: PlayerView
private lateinit var mediaItemListView: ListView

View File

@ -53,6 +53,7 @@ import com.google.common.util.concurrent.ListenableFuture;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
@ -348,7 +349,7 @@ public class DefaultMediaNotificationProvider implements MediaNotification.Provi
if (bitmapFuture.isDone()) {
try {
builder.setLargeIcon(Futures.getDone(bitmapFuture));
} catch (ExecutionException e) {
} catch (CancellationException | ExecutionException e) {
Log.w(TAG, getBitmapLoadErrorMessage(e));
}
} else {

View File

@ -214,7 +214,7 @@ import java.util.concurrent.TimeoutException;
if (controllerAndListener != null && controllerAndListener.controller.isDone()) {
try {
mediaNotificationController = Futures.getDone(controllerAndListener.controller);
} catch (ExecutionException e) {
} catch (CancellationException | ExecutionException e) {
// Ignore.
}
}
@ -323,7 +323,7 @@ import java.util.concurrent.TimeoutException;
}
try {
return Futures.getDone(controllerAndListener.controller);
} catch (ExecutionException exception) {
} catch (CancellationException | ExecutionException exception) {
// We should never reach this.
throw new IllegalStateException(exception);
}

View File

@ -1375,7 +1375,7 @@ import org.checkerframework.checker.initialization.qual.Initialized;
if (bitmapFuture.isDone()) {
try {
artworkBitmap = Futures.getDone(bitmapFuture);
} catch (ExecutionException e) {
} catch (CancellationException | ExecutionException e) {
Log.w(TAG, getBitmapLoadErrorMessage(e));
}
} else {