Skip to content

Show dialogs on UI thread #3464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 17, 2022
Merged

Show dialogs on UI thread #3464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf750da
Show dialogs on UI thread
lfkellogg Feb 17, 2022
62d414b
Fix test names
lfkellogg Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 71 additions & 60 deletions ...stribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,34 +215,40 @@ private Task<Void> showSignInConfirmationDialog(Activity hostActivity) {
showSignInDialogTask = new TaskCompletionSource<>();
}

signInConfirmationDialog = new AlertDialog.Builder(hostActivity).create();
dialogHostActivity = hostActivity;

Context context = firebaseApp.getApplicationContext();
signInConfirmationDialog.setTitle(context.getString(R.string.signin_dialog_title));
signInConfirmationDialog.setMessage(context.getString(R.string.singin_dialog_message));

signInConfirmationDialog.setButton(
AlertDialog.BUTTON_POSITIVE,
context.getString(R.string.singin_yes_button),
(dialogInterface, i) -> showSignInDialogTask.setResult(null));

signInConfirmationDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.singin_no_button),
(dialogInterface, i) ->
showSignInDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));

signInConfirmationDialog.setOnCancelListener(
dialogInterface ->
showSignInDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));

signInConfirmationDialog.show();

// We may not be on the main (UI) thread in some cases, specifically if the developer calls
// the basic config from the background. If we are already on the main thread, this will
// execute immediately.
hostActivity.runOnUiThread(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do the work of creating the dialog not on the UI thread? Can we just have the showing of the dialog on the UI thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question - we actually can't. new AlertDialog.Builder(hostActivity).create() has to be called on the UI thread. I at first just wrapped the .show() call and then had to figure out why it was still broken :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bummer! Great investigation!

() -> {
signInConfirmationDialog = new AlertDialog.Builder(hostActivity).create();

Context context = firebaseApp.getApplicationContext();
signInConfirmationDialog.setTitle(context.getString(R.string.signin_dialog_title));
signInConfirmationDialog.setMessage(context.getString(R.string.singin_dialog_message));

signInConfirmationDialog.setButton(
AlertDialog.BUTTON_POSITIVE,
context.getString(R.string.singin_yes_button),
(dialogInterface, i) -> showSignInDialogTask.setResult(null));

signInConfirmationDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.singin_no_button),
(dialogInterface, i) ->
showSignInDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));

signInConfirmationDialog.setOnCancelListener(
dialogInterface ->
showSignInDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));

signInConfirmationDialog.show();
});
return showSignInDialogTask.getTask();
}

Expand Down Expand Up @@ -420,42 +426,47 @@ private Task<Void> showUpdateConfirmationDialog(
}

Context context = firebaseApp.getApplicationContext();

updateConfirmationDialog = new AlertDialog.Builder(hostActivity).create();
dialogHostActivity = hostActivity;
updateConfirmationDialog.setTitle(context.getString(R.string.update_dialog_title));

StringBuilder message =
new StringBuilder(
String.format(
"Version %s (%s) is available.",
newRelease.getDisplayVersion(), newRelease.getVersionCode()));

if (newRelease.getReleaseNotes() != null && !newRelease.getReleaseNotes().isEmpty()) {
message.append(String.format("\n\nRelease notes: %s", newRelease.getReleaseNotes()));
}
updateConfirmationDialog.setMessage(message);

updateConfirmationDialog.setButton(
AlertDialog.BUTTON_POSITIVE,
context.getString(R.string.update_yes_button),
(dialogInterface, i) -> showUpdateDialogTask.setResult(null));

updateConfirmationDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.update_no_button),
(dialogInterface, i) ->
showUpdateDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));

updateConfirmationDialog.setOnCancelListener(
dialogInterface ->
showUpdateDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));

updateConfirmationDialog.show();
// We should already be on the main (UI) thread here, but be explicit just to be safe. If we are
// already on the main thread, this will execute immediately.
hostActivity.runOnUiThread(
() -> {
updateConfirmationDialog = new AlertDialog.Builder(hostActivity).create();
updateConfirmationDialog.setTitle(context.getString(R.string.update_dialog_title));

StringBuilder message =
new StringBuilder(
String.format(
"Version %s (%s) is available.",
newRelease.getDisplayVersion(), newRelease.getVersionCode()));

if (newRelease.getReleaseNotes() != null && !newRelease.getReleaseNotes().isEmpty()) {
message.append(String.format("\n\nRelease notes: %s", newRelease.getReleaseNotes()));
}
updateConfirmationDialog.setMessage(message);

updateConfirmationDialog.setButton(
AlertDialog.BUTTON_POSITIVE,
context.getString(R.string.update_yes_button),
(dialogInterface, i) -> showUpdateDialogTask.setResult(null));

updateConfirmationDialog.setButton(
AlertDialog.BUTTON_NEGATIVE,
context.getString(R.string.update_no_button),
(dialogInterface, i) ->
showUpdateDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));

updateConfirmationDialog.setOnCancelListener(
dialogInterface ->
showUpdateDialogTask.setException(
new FirebaseAppDistributionException(
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));

updateConfirmationDialog.show();
});

return showUpdateDialogTask.getTask();
}
Expand Down
8 changes: 8 additions & 0 deletions ...in/java/com/google/firebase/appdistribution/FirebaseAppDistributionLifecycleNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ interface OnActivityDestroyedListener {
/**
* Apply a function to a foreground activity, when one is available, returning a {@link Task} that
* will complete immediately after the function is applied.
*
* <p>The consumer function will be called immediately once the activity is available. This may be
* on the main thread or the calling thread, depending on whether or not there is already a
* foreground activity available when this method is called.
*/
Task<Void> applyToForegroundActivity(ActivityConsumer consumer) {
return getForegroundActivity()
Expand All @@ -117,6 +121,10 @@ Task<Void> applyToForegroundActivity(ActivityConsumer consumer) {
/**
* Apply a function to a foreground activity, when one is available, returning a {@link Task} that
* will complete with the result of the Task returned by that function.
*
* <p>The continuation function will be called immediately once the activity is available. This
* may be on the main thread or the calling thread, depending on whether or not there is already a
* foreground activity available when this method is called.
*/
<T> Task<T> applyToForegroundActivityTask(SuccessContinuation<Activity, T> continuation) {
return getForegroundActivity()
Expand Down
55 changes: 43 additions & 12 deletions ...bution/src/test/java/com/google/firebase/appdistribution/FirebaseAppDistributionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@
import com.google.firebase.installations.InstallationTokenResult;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -231,7 +235,7 @@ public void updateApp_whenNotSignedIn_throwsError() {
}

@Test
public void updateToNewRelease_whenNewAabReleaseAvailable_showsUpdateDialog() {
public void updateIfNewReleaseAvailable_whenNewAabReleaseAvailable_showsUpdateDialog() {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build())));

Expand All @@ -248,7 +252,20 @@ public void updateToNewRelease_whenNewAabReleaseAvailable_showsUpdateDialog() {
}

@Test
public void updateToNewRelease_whenReleaseNotesEmpty_doesNotShowReleaseNotes() {
public void updateIfNewReleaseAvailable_fromABackgroundThread_showsUpdateDialog()
throws InterruptedException {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build())));

ExecutorService executorService = Executors.newSingleThreadExecutor();
executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable());
TestUtils.awaitAsyncOperations(executorService);

assertAlertDialogShown();
}

@Test
public void updateIfNewReleaseAvailable_whenReleaseNotesEmpty_doesNotShowReleaseNotes() {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build())));

Expand All @@ -263,7 +280,7 @@ public void updateToNewRelease_whenReleaseNotesEmpty_doesNotShowReleaseNotes() {
}

@Test
public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
public void updateIfNewReleaseAvailable_whenNoReleaseAvailable_updateDialogNotShown() {
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null));

UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
Expand All @@ -277,7 +294,7 @@ public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
}

@Test
public void updateToNewRelease_whenActivityBackgrounded_updateDialogNotShown() {
public void updateIfNewReleaseAvailable_whenActivityBackgrounded_updateDialogNotShown() {
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(null));

UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
Expand All @@ -291,7 +308,7 @@ public void updateToNewRelease_whenActivityBackgrounded_updateDialogNotShown() {
}

@Test
public void updateToNewRelease_whenSignInCancelled_checkForUpdateNotCalled() {
public void updateIfNewReleaseAvailable_whenSignInCancelled_checkForUpdateNotCalled() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
when(mockTesterSignInManager.signInTester())
.thenReturn(
Expand All @@ -310,7 +327,7 @@ public void updateToNewRelease_whenSignInCancelled_checkForUpdateNotCalled() {
}

@Test
public void updateToNewRelease_whenSignInFailed_checkForUpdateNotCalled() {
public void updateIfNewReleaseAvailable_whenSignInFailed_checkForUpdateNotCalled() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
when(mockTesterSignInManager.signInTester())
.thenReturn(
Expand All @@ -327,7 +344,7 @@ public void updateToNewRelease_whenSignInFailed_checkForUpdateNotCalled() {
}

@Test
public void updateToNewRelease_whenDialogDismissed_taskFails() {
public void updateIfNewReleaseAvailable_whenDialogDismissed_taskFails() {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));

Expand All @@ -341,7 +358,7 @@ public void updateToNewRelease_whenDialogDismissed_taskFails() {
}

@Test
public void updateToNewRelease_whenDialogCanceled_taskFails() {
public void updateIfNewReleaseAvailable_whenDialogCanceled_taskFails() {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));

Expand All @@ -356,7 +373,7 @@ public void updateToNewRelease_whenDialogCanceled_taskFails() {
}

@Test
public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
public void updateIfNewReleaseAvailable_whenCheckForUpdateFails_updateAppNotCalled() {
when(mockNewReleaseFetcher.checkForNewRelease())
.thenReturn(
Tasks.forException(
Expand All @@ -376,7 +393,7 @@ public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
}

@Test
public void updateToNewRelease_whenTesterIsSignedIn_doesNotOpenDialog() {
public void updateIfNewReleaseAvailable_whenTesterIsSignedIn_doesNotOpenDialog() {
when(mockSignInStorage.getSignInStatus()).thenReturn(true);

firebaseAppDistribution.updateIfNewReleaseAvailable();
Expand All @@ -400,7 +417,7 @@ public void signInTester_whenDialogDismissed_taskFails() {
}

@Test
public void updateToNewRelease_whenSignInDialogCanceled_taskFails() {
public void updateIfNewReleaseAvailable_whenSignInDialogCanceled_taskFails() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
Task signInTask = firebaseAppDistribution.updateIfNewReleaseAvailable();

Expand Down Expand Up @@ -429,7 +446,7 @@ public void signOutTester_setsSignInStatusFalse() {
}

@Test
public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {
public void updateIfNewReleaseAvailable_receiveProgressUpdateFromUpdateApp() {
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
when(mockNewReleaseFetcher.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
UpdateTaskImpl mockTask = new UpdateTaskImpl();
Expand All @@ -455,6 +472,20 @@ public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {
assertEquals(UpdateStatus.DOWNLOADING, progressEvents.get(0).getUpdateStatus());
}

@Test
public void updateIfNewReleaseAvailable_fromABackgroundThread_showsSignInDialog()
throws InterruptedException, ExecutionException {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);

ExecutorService executorService = Executors.newSingleThreadExecutor();
Future<UpdateTask> future =
executorService.submit(() -> firebaseAppDistribution.updateIfNewReleaseAvailable());
TestUtils.awaitAsyncOperations(executorService);

assertAlertDialogShown();
assertFalse(((UpdateTask) future.get()).isComplete());
}

@Test
public void updateIfNewReleaseAvailable_whenScreenRotates_signInConfirmationDialogReappears() {
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
Expand Down