Skip to content

Commit 2864495

Browse files
rachaprinceManny Jimenezmanny-jimenez
authored and
jeremyjiang-dev
committed
Host activity interrupted error & dialog dismissal (#3341)
* WIP host activity interrupted error & dialog dismissal * Adding synchro blocks * Changing onResume to accomodate rotation and reentry * Adding in synch blocks * Moving task completion source to be a class variable * Adding in tests * Adding lock * Responding to comments * Responding to feedback * Responding to feedback * Fixing test Co-authored-by: Manny Jimenez <[email protected]> Co-authored-by: Emmanuel Jimenez <[email protected]>
1 parent 0c60276 commit 2864495

File tree

12 files changed

+248
-83
lines changed

12 files changed

+248
-83
lines changed

‎firebase-appdistribution/api.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ package com.google.firebase.appdistribution {
3434
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status AUTHENTICATION_CANCELED;
3535
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status AUTHENTICATION_FAILURE;
3636
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status DOWNLOAD_FAILURE;
37+
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status HOST_ACTIVITY_INTERRUPTED;
3738
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status INSTALLATION_CANCELED;
3839
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status INSTALLATION_FAILURE;
3940
enum_constant public static final com.google.firebase.appdistribution.FirebaseAppDistributionException.Status INSTALLATION_FAILURE_SIGNATURE_MISMATCH;

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/Constants.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ static class ErrorMessages {
3737

3838
public static final String DOWNLOAD_URL_NOT_FOUND = "Download URL not found";
3939

40+
public static final String HOST_ACTIVITY_INTERRUPTED =
41+
"Host activity interrupted while dialog was showing";
42+
4043
public static final String APK_INSTALLATION_FAILED =
4144
"The APK failed to install or installation was cancelled";
4245
}

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java

Lines changed: 102 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_CANCELED;
1818
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE;
19+
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.HOST_ACTIVITY_INTERRUPTED;
1920
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE;
2021
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskException;
2122
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskResult;
@@ -34,7 +35,6 @@
3435
import com.google.firebase.appdistribution.Constants.ErrorMessages;
3536
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
3637
import com.google.firebase.appdistribution.internal.LogWrapper;
37-
import com.google.firebase.appdistribution.internal.SignInResultActivity;
3838
import com.google.firebase.appdistribution.internal.SignInStorage;
3939
import com.google.firebase.inject.Provider;
4040
import com.google.firebase.installations.FirebaseInstallationsApi;
@@ -62,10 +62,15 @@ public class FirebaseAppDistribution {
6262
private AppDistributionReleaseInternal cachedNewRelease;
6363

6464
private Task<AppDistributionRelease> cachedCheckForNewReleaseTask;
65-
private AlertDialog updateDialog;
65+
private AlertDialog updateConfirmationDialog;
6666
private AlertDialog signInConfirmationDialog;
67-
private boolean remakeUpdateDialog = false;
67+
@Nullable private Activity dialogHostActivity = null;
68+
6869
private boolean remakeSignInConfirmationDialog = false;
70+
private boolean remakeUpdateConfirmationDialog = false;
71+
72+
private TaskCompletionSource<Void> showSignInDialogTask = null;
73+
private TaskCompletionSource<Void> showUpdateDialogTask = null;
6974

7075
/** Constructor for FirebaseAppDistribution */
7176
@VisibleForTesting
@@ -85,7 +90,8 @@ public class FirebaseAppDistribution {
8590
this.signInStorage = signInStorage;
8691
this.lifecycleNotifier = lifecycleNotifier;
8792
lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed);
88-
lifecycleNotifier.addOnActivityCreatedListener(this::onActivityCreated);
93+
lifecycleNotifier.addOnActivityPausedListener(this::onActivityPaused);
94+
lifecycleNotifier.addOnActivityResumedListener(this::onActivityResumed);
8995
}
9096

9197
/** Constructor for FirebaseAppDistribution */
@@ -135,17 +141,20 @@ public static FirebaseAppDistribution getInstance() {
135141
@NonNull
136142
public UpdateTask updateIfNewReleaseAvailable() {
137143
synchronized (updateIfNewReleaseTaskLock) {
138-
if (cachedUpdateIfNewReleaseTask != null && !cachedUpdateIfNewReleaseTask.isComplete()) {
144+
if (updateIfNewReleaseAvailableIsTaskInProgress()) {
139145
return cachedUpdateIfNewReleaseTask;
140146
}
141147
cachedUpdateIfNewReleaseTask = new UpdateTaskImpl();
148+
remakeSignInConfirmationDialog = false;
149+
remakeUpdateConfirmationDialog = false;
150+
dialogHostActivity = null;
142151
}
143152

144153
lifecycleNotifier
145154
.applyToForegroundActivityTask(this::showSignInConfirmationDialog)
146155
// TODO(rachelprince): Revisit this comment once changes to checkForNewRelease are reviewed
147156
// Even though checkForNewRelease() calls signInTester(), we explicitly call signInTester
148-
// here both for code clarifty, and because we plan to remove the signInTester() call
157+
// here for code clarity, and because we plan to remove the signInTester() call
149158
// from checkForNewRelease() in the near future
150159
.onSuccessTask(unused -> signInTester())
151160
.onSuccessTask(unused -> checkForNewRelease())
@@ -159,7 +168,7 @@ public UpdateTask updateIfNewReleaseAvailable() {
159168
.setUpdateStatus(UpdateStatus.NEW_RELEASE_CHECK_FAILED)
160169
.build());
161170
}
162-
// if the task failed, this get() will cause the error to propogate to the handler
171+
// if the task failed, this get() will cause the error to propagate to the handler
163172
// below
164173
AppDistributionRelease release = task.getResult();
165174
if (release == null) {
@@ -173,7 +182,7 @@ public UpdateTask updateIfNewReleaseAvailable() {
173182
return Tasks.forResult(null);
174183
}
175184
return lifecycleNotifier.applyToForegroundActivityTask(
176-
activity -> showUpdateAlertDialog(activity, release));
185+
activity -> showUpdateConfirmationDialog(activity, release));
177186
})
178187
.onSuccessTask(
179188
unused ->
@@ -191,34 +200,39 @@ private Task<Void> showSignInConfirmationDialog(Activity hostActivity) {
191200
return Tasks.forResult(null);
192201
}
193202

194-
TaskCompletionSource<Void> showDialogTask = new TaskCompletionSource<>();
203+
if (showSignInDialogTask == null || showSignInDialogTask.getTask().isComplete()) {
204+
showSignInDialogTask = new TaskCompletionSource<>();
205+
}
195206

196207
signInConfirmationDialog = new AlertDialog.Builder(hostActivity).create();
208+
dialogHostActivity = hostActivity;
209+
197210
Context context = firebaseApp.getApplicationContext();
198211
signInConfirmationDialog.setTitle(context.getString(R.string.signin_dialog_title));
199212
signInConfirmationDialog.setMessage(context.getString(R.string.singin_dialog_message));
200213

201214
signInConfirmationDialog.setButton(
202215
AlertDialog.BUTTON_POSITIVE,
203216
context.getString(R.string.singin_yes_button),
204-
(dialogInterface, i) -> showDialogTask.setResult(null));
217+
(dialogInterface, i) -> showSignInDialogTask.setResult(null));
205218

206219
signInConfirmationDialog.setButton(
207220
AlertDialog.BUTTON_NEGATIVE,
208221
context.getString(R.string.singin_no_button),
209222
(dialogInterface, i) ->
210-
showDialogTask.setException(
223+
showSignInDialogTask.setException(
211224
new FirebaseAppDistributionException(
212225
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));
213226

214227
signInConfirmationDialog.setOnCancelListener(
215228
dialogInterface ->
216-
showDialogTask.setException(
229+
showSignInDialogTask.setException(
217230
new FirebaseAppDistributionException(
218231
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED)));
219232

220233
signInConfirmationDialog.show();
221-
return showDialogTask.getTask();
234+
235+
return showSignInDialogTask.getTask();
222236
}
223237

224238
/** Signs in the App Distribution tester. Presents the tester with a Google sign in UI */
@@ -280,14 +294,14 @@ public UpdateTask updateApp() {
280294
* basic configuration and false for advanced configuration.
281295
*/
282296
private UpdateTask updateApp(boolean showDownloadInNotificationManager) {
283-
if (!isTesterSignedIn()) {
284-
UpdateTaskImpl updateTask = new UpdateTaskImpl();
285-
updateTask.setException(
286-
new FirebaseAppDistributionException(
287-
Constants.ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE));
288-
return updateTask;
289-
}
290297
synchronized (cachedNewReleaseLock) {
298+
if (!isTesterSignedIn()) {
299+
UpdateTaskImpl updateTask = new UpdateTaskImpl();
300+
updateTask.setException(
301+
new FirebaseAppDistributionException(
302+
Constants.ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE));
303+
return updateTask;
304+
}
291305
if (cachedNewRelease == null) {
292306
LogWrapper.getInstance().v("New release not found.");
293307
return getErrorUpdateTask(
@@ -322,42 +336,48 @@ public void signOutTester() {
322336
}
323337

324338
@VisibleForTesting
325-
void onActivityDestroyed(@NonNull Activity activity) {
326-
if (activity instanceof SignInResultActivity) {
327-
// SignInResult is internal to the SDK and is destroyed after creation
328-
return;
329-
}
330-
if (activity.isChangingConfigurations()) {
331-
remakeSignInConfirmationDialog =
332-
signInConfirmationDialog != null && signInConfirmationDialog.isShowing();
333-
remakeUpdateDialog = updateDialog != null && updateDialog.isShowing();
334-
dismissDialogs();
335-
return;
339+
void onActivityResumed(Activity activity) {
340+
if (awaitingSignInDialogConfirmation()) {
341+
if (dialogHostActivity != null && dialogHostActivity != activity) {
342+
showSignInDialogTask.setException(
343+
new FirebaseAppDistributionException(
344+
ErrorMessages.HOST_ACTIVITY_INTERRUPTED, HOST_ACTIVITY_INTERRUPTED));
345+
} else {
346+
showSignInConfirmationDialog(activity);
347+
}
336348
}
337349

338-
if (signInConfirmationDialog != null && signInConfirmationDialog.isShowing()) {
339-
setCachedUpdateIfNewReleaseCompletionError(
340-
new FirebaseAppDistributionException(
341-
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED));
350+
if (awaitingUpdateDialogConfirmation()) {
351+
if (dialogHostActivity != null && dialogHostActivity != activity) {
352+
showUpdateDialogTask.setException(
353+
new FirebaseAppDistributionException(
354+
ErrorMessages.HOST_ACTIVITY_INTERRUPTED, HOST_ACTIVITY_INTERRUPTED));
355+
} else {
356+
synchronized (cachedNewReleaseLock) {
357+
showUpdateConfirmationDialog(
358+
activity, ReleaseUtils.convertToAppDistributionRelease(cachedNewRelease));
359+
}
360+
}
342361
}
362+
}
343363

344-
if (updateDialog != null && updateDialog.isShowing()) {
345-
setCachedUpdateIfNewReleaseCompletionError(
346-
new FirebaseAppDistributionException(
347-
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED));
364+
@VisibleForTesting
365+
void onActivityPaused(Activity activity) {
366+
if (activity == dialogHostActivity) {
367+
remakeSignInConfirmationDialog =
368+
signInConfirmationDialog != null && signInConfirmationDialog.isShowing();
369+
remakeUpdateConfirmationDialog =
370+
updateConfirmationDialog != null && updateConfirmationDialog.isShowing();
371+
dismissDialogs();
348372
}
349373
}
350374

351-
void onActivityCreated(Activity activity) {
352-
if (remakeSignInConfirmationDialog) {
353-
remakeSignInConfirmationDialog = false;
354-
showSignInConfirmationDialog(activity);
355-
} else if (remakeUpdateDialog) {
356-
remakeUpdateDialog = false;
357-
synchronized (cachedNewReleaseLock) {
358-
showUpdateAlertDialog(
359-
activity, ReleaseUtils.convertToAppDistributionRelease(cachedNewRelease));
360-
}
375+
@VisibleForTesting
376+
void onActivityDestroyed(@NonNull Activity activity) {
377+
// If the dialogHostActivity is being destroyed it is set to null. This is to ensure onResume
378+
// shows the dialog on a configuration change and does not check the activity reference.
379+
if (activity == dialogHostActivity) {
380+
dialogHostActivity = null;
361381
}
362382
}
363383

@@ -375,14 +395,18 @@ AppDistributionReleaseInternal getCachedNewRelease() {
375395
}
376396
}
377397

378-
private Task<Void> showUpdateAlertDialog(
398+
private Task<Void> showUpdateConfirmationDialog(
379399
Activity hostActivity, AppDistributionRelease newRelease) {
380-
TaskCompletionSource<Void> showUpdateDialogTask = new TaskCompletionSource<>();
400+
401+
if (showUpdateDialogTask == null || showUpdateDialogTask.getTask().isComplete()) {
402+
showUpdateDialogTask = new TaskCompletionSource<>();
403+
}
381404

382405
Context context = firebaseApp.getApplicationContext();
383406

384-
updateDialog = new AlertDialog.Builder(hostActivity).create();
385-
updateDialog.setTitle(context.getString(R.string.update_dialog_title));
407+
updateConfirmationDialog = new AlertDialog.Builder(hostActivity).create();
408+
dialogHostActivity = hostActivity;
409+
updateConfirmationDialog.setTitle(context.getString(R.string.update_dialog_title));
386410

387411
StringBuilder message =
388412
new StringBuilder(
@@ -393,28 +417,28 @@ private Task<Void> showUpdateAlertDialog(
393417
if (newRelease.getReleaseNotes() != null && !newRelease.getReleaseNotes().isEmpty()) {
394418
message.append(String.format("\n\nRelease notes: %s", newRelease.getReleaseNotes()));
395419
}
396-
updateDialog.setMessage(message);
420+
updateConfirmationDialog.setMessage(message);
397421

398-
updateDialog.setButton(
422+
updateConfirmationDialog.setButton(
399423
AlertDialog.BUTTON_POSITIVE,
400424
context.getString(R.string.update_yes_button),
401425
(dialogInterface, i) -> showUpdateDialogTask.setResult(null));
402426

403-
updateDialog.setButton(
427+
updateConfirmationDialog.setButton(
404428
AlertDialog.BUTTON_NEGATIVE,
405429
context.getString(R.string.update_no_button),
406430
(dialogInterface, i) ->
407431
showUpdateDialogTask.setException(
408432
new FirebaseAppDistributionException(
409433
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));
410434

411-
updateDialog.setOnCancelListener(
435+
updateConfirmationDialog.setOnCancelListener(
412436
dialogInterface ->
413437
showUpdateDialogTask.setException(
414438
new FirebaseAppDistributionException(
415439
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED)));
416440

417-
updateDialog.show();
441+
updateConfirmationDialog.show();
418442

419443
return showUpdateDialogTask.getTask();
420444
}
@@ -445,8 +469,8 @@ private void dismissDialogs() {
445469
if (signInConfirmationDialog != null && signInConfirmationDialog.isShowing()) {
446470
signInConfirmationDialog.dismiss();
447471
}
448-
if (updateDialog != null && updateDialog.isShowing()) {
449-
updateDialog.dismiss();
472+
if (updateConfirmationDialog != null && updateConfirmationDialog.isShowing()) {
473+
updateConfirmationDialog.dismiss();
450474
}
451475
}
452476

@@ -455,4 +479,22 @@ private UpdateTaskImpl getErrorUpdateTask(Exception e) {
455479
updateTask.setException(e);
456480
return updateTask;
457481
}
482+
483+
private boolean updateIfNewReleaseAvailableIsTaskInProgress() {
484+
synchronized (updateIfNewReleaseTaskLock) {
485+
return cachedUpdateIfNewReleaseTask != null && !cachedUpdateIfNewReleaseTask.isComplete();
486+
}
487+
}
488+
489+
private boolean awaitingSignInDialogConfirmation() {
490+
return (showSignInDialogTask != null
491+
&& !showSignInDialogTask.getTask().isComplete()
492+
&& remakeSignInConfirmationDialog);
493+
}
494+
495+
private boolean awaitingUpdateDialogConfirmation() {
496+
return (showUpdateDialogTask != null
497+
&& !showUpdateDialogTask.getTask().isComplete()
498+
&& remakeUpdateConfirmationDialog);
499+
}
458500
}

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistributionException.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ public enum Status {
5454

5555
/** Download URL for release expired */
5656
RELEASE_URL_EXPIRED,
57+
58+
/** Host activity for confirmation dialog destroyed or pushed to the backstack */
59+
HOST_ACTIVITY_INTERRUPTED,
5760
}
5861

5962
@NonNull private final Status status;

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistributionLifecycleNotifier.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,12 @@ void removeOnActivityResumedListener(@NonNull OnActivityResumedListener listener
182182
}
183183
}
184184

185+
void addOnActivityPausedListener(@NonNull OnActivityPausedListener listener) {
186+
synchronized (lock) {
187+
this.onActivityPausedListeners.add(listener);
188+
}
189+
}
190+
185191
@Override
186192
public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle bundle) {
187193
synchronized (lock) {

0 commit comments

Comments
 (0)