Skip to content

Commit dd1c336

Browse files
author
rachaprince
authored
Fad cancel task when dialog closed (#3228)
* Add an on cancelled listener to prevent hanging tasks when dialog is canceled * Add tests for dialog cancellation and dismissal * Fix test setup methods
1 parent c971713 commit dd1c336

File tree

5 files changed

+146
-102
lines changed

5 files changed

+146
-102
lines changed

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/FirebaseAppDistribution.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -346,19 +346,11 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
346346
updateDialog.setButton(
347347
AlertDialog.BUTTON_NEGATIVE,
348348
context.getString(R.string.update_no_button),
349-
(dialogInterface, i) -> {
350-
dialogInterface.dismiss();
351-
synchronized (updateIfNewReleaseTaskLock) {
352-
postProgressToCachedUpdateIfNewReleaseTask(
353-
UpdateProgress.builder()
354-
.setApkFileTotalBytes(UNKNOWN_RELEASE_FILE_SIZE)
355-
.setApkBytesDownloaded(UNKNOWN_RELEASE_FILE_SIZE)
356-
.setUpdateStatus(UpdateStatus.UPDATE_CANCELED)
357-
.build());
358-
setCachedUpdateIfNewReleaseCompletionError(
359-
new FirebaseAppDistributionException(
360-
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED));
361-
}
349+
(dialogInterface, i) -> dismissUpdateDialogCallback());
350+
351+
updateDialog.setOnCancelListener(
352+
dialogInterface -> {
353+
dismissUpdateDialogCallback();
362354
});
363355

364356
updateDialog.show();
@@ -368,6 +360,20 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
368360
}
369361
}
370362

363+
private void dismissUpdateDialogCallback() {
364+
synchronized (updateIfNewReleaseTaskLock) {
365+
postProgressToCachedUpdateIfNewReleaseTask(
366+
UpdateProgress.builder()
367+
.setApkFileTotalBytes(UNKNOWN_RELEASE_FILE_SIZE)
368+
.setApkBytesDownloaded(UNKNOWN_RELEASE_FILE_SIZE)
369+
.setUpdateStatus(UpdateStatus.UPDATE_CANCELED)
370+
.build());
371+
setCachedUpdateIfNewReleaseCompletionError(
372+
new FirebaseAppDistributionException(
373+
ErrorMessages.UPDATE_CANCELED, Status.INSTALLATION_CANCELED));
374+
}
375+
}
376+
371377
private void setCachedUpdateIfNewReleaseCompletionError(FirebaseAppDistributionException e) {
372378
synchronized (updateIfNewReleaseTaskLock) {
373379
safeSetTaskException(cachedUpdateIfNewReleaseTask, e);

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/InstallActivity.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.io.File;
2626

2727
/**
28-
* Activity opened during installation in {@link UpdateAppClient} after APK download is finished.
28+
* Activity opened during installation in {@link UpdateApkClient} after APK download is finished.
2929
*/
3030
public class InstallActivity extends AppCompatActivity {
3131
private static final String TAG = "InstallActivity: ";
@@ -92,15 +92,17 @@ private void showUnknownSourcesUi() {
9292
alertDialog.setButton(
9393
AlertDialog.BUTTON_NEGATIVE,
9494
getString(R.string.update_no_button),
95-
(dialogInterface, i) -> {
96-
LogWrapper.getInstance().v(TAG + "Unknown sources dialog cancelled");
97-
dialogInterface.dismiss();
98-
finish();
99-
});
95+
(dialogInterface, i) -> dismissUnknownSourcesDialogCallback());
96+
alertDialog.setOnCancelListener(dialogInterface -> dismissUnknownSourcesDialogCallback());
10097

10198
alertDialog.show();
10299
}
103100

101+
private void dismissUnknownSourcesDialogCallback() {
102+
LogWrapper.getInstance().v(TAG + "Unknown sources dialog cancelled");
103+
finish();
104+
}
105+
104106
private Intent getUnknownSourcesIntent() {
105107
Intent intent;
106108
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/TesterSignInClient.java

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,13 @@
2222
import android.app.Activity;
2323
import android.app.AlertDialog;
2424
import android.content.Context;
25-
import android.content.DialogInterface;
2625
import android.content.Intent;
2726
import android.content.pm.ResolveInfo;
2827
import android.net.Uri;
2928
import androidx.annotation.GuardedBy;
3029
import androidx.annotation.NonNull;
3130
import androidx.annotation.VisibleForTesting;
3231
import androidx.browser.customtabs.CustomTabsIntent;
33-
import com.google.android.gms.tasks.OnFailureListener;
3432
import com.google.android.gms.tasks.OnSuccessListener;
3533
import com.google.android.gms.tasks.Task;
3634
import com.google.android.gms.tasks.TaskCompletionSource;
@@ -162,42 +160,38 @@ private AlertDialog getSignInAlertDialog(Activity currentActivity) {
162160
alertDialog.setButton(
163161
AlertDialog.BUTTON_POSITIVE,
164162
context.getString(R.string.singin_yes_button),
165-
new DialogInterface.OnClickListener() {
166-
@Override
167-
public void onClick(DialogInterface dialogInterface, int i) {
168-
firebaseInstallationsApi
169-
.getId()
170-
.addOnSuccessListener(getFidGenerationOnSuccessListener(currentActivity))
171-
.addOnFailureListener(
172-
new OnFailureListener() {
173-
@Override
174-
public void onFailure(@NonNull Exception e) {
175-
LogWrapper.getInstance().e(TAG + "Fid retrieval failed.", e);
176-
setSignInTaskCompletionError(
177-
new FirebaseAppDistributionException(
178-
Constants.ErrorMessages.AUTHENTICATION_ERROR,
179-
AUTHENTICATION_FAILURE,
180-
e));
181-
}
182-
});
183-
}
163+
(dialogInterface, i) -> {
164+
firebaseInstallationsApi
165+
.getId()
166+
.addOnSuccessListener(getFidGenerationOnSuccessListener(currentActivity))
167+
.addOnFailureListener(
168+
e -> {
169+
LogWrapper.getInstance().e(TAG + "Fid retrieval failed.", e);
170+
setSignInTaskCompletionError(
171+
new FirebaseAppDistributionException(
172+
Constants.ErrorMessages.AUTHENTICATION_ERROR,
173+
AUTHENTICATION_FAILURE,
174+
e));
175+
});
184176
});
177+
185178
alertDialog.setButton(
186179
AlertDialog.BUTTON_NEGATIVE,
187180
context.getString(R.string.singin_no_button),
188-
new DialogInterface.OnClickListener() {
189-
@Override
190-
public void onClick(DialogInterface dialogInterface, int i) {
191-
LogWrapper.getInstance().v("Sign in has been canceled.");
192-
setSignInTaskCompletionError(
193-
new FirebaseAppDistributionException(
194-
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED));
195-
dialogInterface.dismiss();
196-
}
197-
});
181+
(dialogInterface, i) -> dismissSignInDialogCallback());
182+
183+
alertDialog.setOnCancelListener(dialogInterface -> dismissSignInDialogCallback());
184+
198185
return alertDialog;
199186
}
200187

188+
private void dismissSignInDialogCallback() {
189+
LogWrapper.getInstance().v("Sign in has been canceled.");
190+
setSignInTaskCompletionError(
191+
new FirebaseAppDistributionException(
192+
ErrorMessages.AUTHENTICATION_CANCELED, AUTHENTICATION_CANCELED));
193+
}
194+
201195
private void setSignInTaskCompletionError(FirebaseAppDistributionException e) {
202196
synchronized (signInTaskLock) {
203197
safeSetTaskException(signInTaskCompletionSource, e);

firebase-app-distribution/src/test/java/com/google/firebase/app/distribution/FirebaseAppDistributionTest.java

Lines changed: 52 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,11 @@ public void setup() {
165165

166166
activity = Robolectric.buildActivity(TestActivity.class).create().get();
167167
when(mockLifecycleNotifier.getCurrentActivity()).thenReturn(activity);
168+
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
168169
}
169170

170171
@Test
171172
public void checkForNewRelease_whenCheckForNewReleaseFails_throwsError() {
172-
firebaseAppDistribution.setCachedNewRelease(null);
173173
when(mockCheckForNewReleaseClient.checkForNewRelease())
174174
.thenReturn(
175175
Tasks.forException(
@@ -234,55 +234,40 @@ public void updateApp_whenNotSignedIn_throwsError() {
234234

235235
@Test
236236
public void updateToNewRelease_whenNewAabReleaseAvailable_showsUpdateDialog() {
237-
// mockSignInStorage returns false then true to simulate logging in during first signIn check in
238-
// updateIfNewReleaseAvailable
239-
when(mockSignInStorage.getSignInStatus()).thenReturn(false).thenReturn(true);
240-
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
241-
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
242-
firebaseAppDistribution.setCachedNewRelease(newRelease);
243-
doReturn(new UpdateTaskImpl()).when(mockUpdateAabClient).updateAab(newRelease);
237+
when(mockCheckForNewReleaseClient.checkForNewRelease())
238+
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.build())));
244239

245240
firebaseAppDistribution.updateIfNewReleaseAvailable();
246241

247-
// Update flow
248-
verify(mockTesterSignInClient, times(1)).signInTester();
249-
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
250-
AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
242+
AlertDialog dialog = verifyUpdateAlertDialog();
251243
assertEquals(
252244
String.format(
253245
"Version %s (%s) is available.\n\nRelease notes: %s",
254246
TEST_RELEASE_NEWER_AAB.getDisplayVersion(),
255247
TEST_RELEASE_NEWER_AAB.getVersionCode(),
256248
TEST_RELEASE_NEWER_AAB.getReleaseNotes()),
257-
shadowOf(updateDialog).getMessage().toString());
258-
assertTrue(updateDialog.isShowing());
249+
shadowOf(dialog).getMessage().toString());
259250
}
260251

261252
@Test
262253
public void updateToNewRelease_whenReleaseNotesEmpty_doesNotShowReleaseNotes() {
263-
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
264-
AppDistributionReleaseInternal newRelease =
265-
TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build();
266-
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
267-
firebaseAppDistribution.setCachedNewRelease(newRelease);
254+
when(mockCheckForNewReleaseClient.checkForNewRelease())
255+
.thenReturn(Tasks.forResult((TEST_RELEASE_NEWER_AAB_INTERNAL.setReleaseNotes("").build())));
268256

269257
firebaseAppDistribution.updateIfNewReleaseAvailable();
270258

271-
// Update flow
272-
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
273-
AlertDialog updateDialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
259+
AlertDialog dialog = verifyUpdateAlertDialog();
274260
assertEquals(
275261
String.format(
276262
"Version %s (%s) is available.",
277263
TEST_RELEASE_NEWER_AAB.getDisplayVersion(), TEST_RELEASE_NEWER_AAB.getVersionCode()),
278-
shadowOf(updateDialog).getMessage().toString());
264+
shadowOf(dialog).getMessage().toString());
279265
}
280266

281267
@Test
282268
public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
283269
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
284270
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(null));
285-
firebaseAppDistribution.setCachedNewRelease(null);
286271

287272
UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
288273

@@ -298,7 +283,6 @@ public void updateToNewRelease_whenNoReleaseAvailable_updateDialogNotShown() {
298283
public void updateToNewRelease_whenActivityBackgrounded_updateDialogNotShown() {
299284
when(mockSignInStorage.getSignInStatus()).thenReturn(false);
300285
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(null));
301-
firebaseAppDistribution.setCachedNewRelease(null);
302286
when(mockLifecycleNotifier.getCurrentActivity()).thenReturn(null);
303287

304288
UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
@@ -352,6 +336,41 @@ public void updateToNewRelease_whenSignInFailed_checkForUpdateNotCalled() {
352336
assertEquals(AUTHENTICATION_FAILURE, e.getErrorCode());
353337
}
354338

339+
@Test
340+
public void updateToNewRelease_whenDialogDismissed_taskFails() {
341+
when(mockCheckForNewReleaseClient.checkForNewRelease())
342+
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));
343+
344+
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
345+
AlertDialog updateDialog = verifyUpdateAlertDialog();
346+
updateDialog.getButton(AlertDialog.BUTTON_NEGATIVE).performClick(); // dismiss dialog
347+
348+
assertFalse(updateDialog.isShowing());
349+
assertFalse(updateTask.isSuccessful());
350+
Exception e = updateTask.getException();
351+
assertTrue(e instanceof FirebaseAppDistributionException);
352+
assertEquals(INSTALLATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode());
353+
assertEquals(ErrorMessages.UPDATE_CANCELED, e.getMessage());
354+
}
355+
356+
@Test
357+
public void updateToNewRelease_whenDialogCanceled_taskFails() {
358+
when(mockCheckForNewReleaseClient.checkForNewRelease())
359+
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));
360+
361+
UpdateTask updateTask = firebaseAppDistribution.updateIfNewReleaseAvailable();
362+
363+
AlertDialog updateDialog = verifyUpdateAlertDialog();
364+
updateDialog.onBackPressed(); // cancels the dialog
365+
366+
assertFalse(updateDialog.isShowing());
367+
assertFalse(updateTask.isSuccessful());
368+
Exception e = updateTask.getException();
369+
assertTrue(e instanceof FirebaseAppDistributionException);
370+
assertEquals(INSTALLATION_CANCELED, ((FirebaseAppDistributionException) e).getErrorCode());
371+
assertEquals(ErrorMessages.UPDATE_CANCELED, e.getMessage());
372+
}
373+
355374
@Test
356375
public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
357376
when(mockCheckForNewReleaseClient.checkForNewRelease())
@@ -374,14 +393,6 @@ public void updateToNewRelease_whenCheckForUpdateFails_updateAppNotCalled() {
374393
assertEquals(FirebaseAppDistributionException.Status.NETWORK_FAILURE, e.getErrorCode());
375394
}
376395

377-
@Test
378-
public void updateToNewRelease_callsSignInTester() {
379-
when(mockCheckForNewReleaseClient.checkForNewRelease())
380-
.thenReturn(Tasks.forResult(TEST_RELEASE_NEWER_AAB_INTERNAL.build()));
381-
firebaseAppDistribution.updateIfNewReleaseAvailable();
382-
verify(mockTesterSignInClient, times(1)).signInTester();
383-
}
384-
385396
@Test
386397
public void signOutTester_setsSignInStatusFalse() {
387398
firebaseAppDistribution.signOutTester();
@@ -390,10 +401,8 @@ public void signOutTester_setsSignInStatusFalse() {
390401

391402
@Test
392403
public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {
393-
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
394404
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
395405
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
396-
firebaseAppDistribution.setCachedNewRelease(newRelease);
397406
UpdateTaskImpl mockTask = new UpdateTaskImpl();
398407
when(mockUpdateAabClient.updateAab(newRelease)).thenReturn(mockTask);
399408
mockTask.updateProgress(
@@ -419,10 +428,8 @@ public void updateToNewRelease_receiveProgressUpdateFromUpdateApp() {
419428

420429
@Test
421430
public void taskCancelledOnScreenRotation() {
422-
when(mockSignInStorage.getSignInStatus()).thenReturn(true);
423431
AppDistributionReleaseInternal newRelease = TEST_RELEASE_NEWER_AAB_INTERNAL.build();
424432
when(mockCheckForNewReleaseClient.checkForNewRelease()).thenReturn(Tasks.forResult(newRelease));
425-
firebaseAppDistribution.setCachedNewRelease(newRelease);
426433

427434
UpdateTask task = firebaseAppDistribution.updateIfNewReleaseAvailable();
428435

@@ -475,4 +482,12 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() {
475482

476483
assertEquals(updateTask, updateTaskToReturn);
477484
}
485+
486+
private AlertDialog verifyUpdateAlertDialog() {
487+
assertTrue(ShadowAlertDialog.getLatestDialog() instanceof AlertDialog);
488+
AlertDialog dialog = (AlertDialog) ShadowAlertDialog.getLatestDialog();
489+
assertTrue(dialog.isShowing());
490+
491+
return dialog;
492+
}
478493
}

0 commit comments

Comments
 (0)