Skip to content

Commit c7c34e0

Browse files
lfkellogglfkellogg
committed
Allow AAB install to be started from a background thread
1 parent b7c3081 commit c7c34e0

File tree

6 files changed

+93
-61
lines changed

6 files changed

+93
-61
lines changed

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

Lines changed: 55 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616

1717
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE;
1818
import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.NETWORK_FAILURE;
19+
import static com.google.firebase.appdistribution.TaskUtils.combineWithResultOf;
20+
import static com.google.firebase.appdistribution.TaskUtils.runAsyncInTask;
1921
import static com.google.firebase.appdistribution.TaskUtils.safeSetTaskException;
2022

2123
import android.app.Activity;
22-
import android.content.Context;
2324
import android.content.Intent;
2425
import android.net.Uri;
2526
import androidx.annotation.GuardedBy;
@@ -42,29 +43,28 @@ class AabUpdater {
4243
private final HttpsUrlConnectionFactory httpsUrlConnectionFactory;
4344
private final Executor executor;
4445

46+
private final Object updateAabLock = new Object();
47+
4548
@GuardedBy("updateAabLock")
4649
private UpdateTaskImpl cachedUpdateTask;
4750

4851
@GuardedBy("updateAabLock")
4952
private AppDistributionReleaseInternal aabReleaseInProgress;
5053

51-
private final Object updateAabLock = new Object();
52-
private final Context context;
54+
@GuardedBy("updateAabLock")
55+
private boolean hasBeenSentToPlayForCurrentTask = false;
5356

54-
AabUpdater(@NonNull Context context) {
57+
AabUpdater() {
5558
this(
56-
context,
5759
FirebaseAppDistributionLifecycleNotifier.getInstance(),
5860
new HttpsUrlConnectionFactory(),
5961
Executors.newSingleThreadExecutor());
6062
}
6163

6264
AabUpdater(
63-
@NonNull Context context,
6465
@NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
6566
@NonNull HttpsUrlConnectionFactory httpsUrlConnectionFactory,
6667
@NonNull Executor executor) {
67-
this.context = context;
6868
this.lifecycleNotifier = lifecycleNotifier;
6969
this.httpsUrlConnectionFactory = httpsUrlConnectionFactory;
7070
lifecycleNotifier.addOnActivityStartedListener(this::onActivityStarted);
@@ -76,9 +76,13 @@ void onActivityStarted(Activity activity) {
7676
if (activity instanceof SignInResultActivity || activity instanceof InstallActivity) {
7777
return;
7878
}
79-
// If app resumes and aab update task is in progress, assume that installation didn't happen so
80-
// cancel the task
81-
this.tryCancelAabUpdateTask();
79+
// If app resumes and update is in progress, assume that installation didn't happen and cancel
80+
// the task
81+
synchronized (updateAabLock) {
82+
if (awaitingUpdateFromPlay()) {
83+
this.tryCancelAabUpdateTask();
84+
}
85+
}
8286
}
8387

8488
UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
@@ -89,7 +93,18 @@ UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
8993

9094
cachedUpdateTask = new UpdateTaskImpl();
9195
aabReleaseInProgress = newRelease;
92-
redirectToPlayForAabUpdate(newRelease.getDownloadUrl());
96+
hasBeenSentToPlayForCurrentTask = false;
97+
98+
// On a background thread, fetch the redirect URL and open it in the Play app
99+
runAsyncInTask(executor, () -> fetchDownloadRedirectUrl(newRelease.getDownloadUrl()))
100+
.onSuccessTask(
101+
executor,
102+
combineWithResultOf(executor, () -> lifecycleNotifier.getForegroundActivity()))
103+
.addOnSuccessListener(
104+
executor,
105+
urlAndActivity ->
106+
openRedirectUrlInPlay(urlAndActivity.first(), urlAndActivity.second()))
107+
.addOnFailureListener(executor, this::setUpdateTaskCompletionError);
93108

94109
return cachedUpdateTask;
95110
}
@@ -138,38 +153,30 @@ private static boolean isRedirectResponse(int responseCode) {
138153
return responseCode >= 300 && responseCode < 400;
139154
}
140155

141-
private void redirectToPlayForAabUpdate(String downloadUrl) {
142-
// The 302 redirect is obtained here to open the play store directly and avoid opening chrome
143-
executor.execute( // Execute the network calls on a background thread
144-
() -> {
145-
String redirectUrl;
146-
try {
147-
redirectUrl = fetchDownloadRedirectUrl(downloadUrl);
148-
} catch (FirebaseAppDistributionException e) {
149-
setUpdateTaskCompletionError(e);
150-
return;
151-
}
152-
153-
Intent updateIntent = new Intent(Intent.ACTION_VIEW);
154-
updateIntent.setData(Uri.parse(redirectUrl));
155-
updateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
156-
LogWrapper.getInstance().v(TAG + "Redirecting to play");
157-
158-
synchronized (updateAabLock) {
159-
context.startActivity(updateIntent);
160-
cachedUpdateTask.updateProgress(
161-
UpdateProgress.builder()
162-
.setApkBytesDownloaded(-1)
163-
.setApkFileTotalBytes(-1)
164-
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
165-
.build());
166-
}
167-
});
156+
private void openRedirectUrlInPlay(String redirectUrl, Activity hostActivity) {
157+
Intent updateIntent = new Intent(Intent.ACTION_VIEW);
158+
updateIntent.setData(Uri.parse(redirectUrl));
159+
updateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
160+
LogWrapper.getInstance().v(TAG + "Redirecting to play");
161+
162+
// Launch the intent outside of the synchronized block because we don't want to risk the
163+
// activity leaving the foreground while we wait for the lock.
164+
hostActivity.startActivity(updateIntent);
165+
166+
synchronized (updateAabLock) {
167+
cachedUpdateTask.updateProgress(
168+
UpdateProgress.builder()
169+
.setApkBytesDownloaded(-1)
170+
.setApkFileTotalBytes(-1)
171+
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
172+
.build());
173+
hasBeenSentToPlayForCurrentTask = true;
174+
}
168175
}
169176

170-
private void setUpdateTaskCompletionError(FirebaseAppDistributionException e) {
177+
private void setUpdateTaskCompletionError(Exception e) {
171178
synchronized (updateAabLock) {
172-
safeSetTaskException(cachedUpdateTask, e);
179+
safeSetTaskException(cachedUpdateTask, FirebaseAppDistributionException.wrap(e));
173180
}
174181
}
175182

@@ -183,4 +190,12 @@ void tryCancelAabUpdateTask() {
183190
ReleaseUtils.convertToAppDistributionRelease(aabReleaseInProgress)));
184191
}
185192
}
193+
194+
private boolean awaitingUpdateFromPlay() {
195+
synchronized (updateAabLock) {
196+
return cachedUpdateTask != null
197+
&& !cachedUpdateTask.isComplete()
198+
&& hasBeenSentToPlayForCurrentTask;
199+
}
200+
}
186201
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public class FirebaseAppDistribution {
102102
new FirebaseAppDistributionTesterApiClient(),
103103
firebaseInstallationsApiProvider),
104104
new ApkUpdater(firebaseApp, new ApkInstaller()),
105-
new AabUpdater(firebaseApp.getApplicationContext()),
105+
new AabUpdater(),
106106
signInStorage,
107107
lifecycleNotifier);
108108
}

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,18 +122,35 @@ static <T1, T2> CombinedTaskResults<T1, T2> create(T1 first, T2 second) {
122122
*
123123
* <pre>{@code
124124
* runFirstAsyncTask()
125-
* .onSuccessTask(combineWithResultOf(() -> startSecondAsyncTask())
125+
* .onSuccessTask(executor, combineWithResultOf(executor, () -> startSecondAsyncTask())
126126
* .addOnSuccessListener(
127+
* executor,
127128
* results ->
128129
* doSomethingWithBothResults(results.result1(), results.result2()));
129130
* }</pre>
130131
*
132+
* @param executor The {@link Executor} to use to call the listener that combines the results
131133
* @param secondTaskSource A {@link TaskSource} providing the next task to run
132134
* @param <T1> The result type of the first task
133135
* @param <T2> The result type of the second task
134136
* @return A {@link SuccessContinuation} that will return a new task with result type {@link
135137
* CombinedTaskResults}, combining the results of both tasks
136138
*/
139+
static <T1, T2> SuccessContinuation<T1, CombinedTaskResults<T1, T2>> combineWithResultOf(
140+
Executor executor, TaskSource<T2> secondTaskSource) {
141+
return firstResult ->
142+
secondTaskSource
143+
.get()
144+
.onSuccessTask(
145+
executor,
146+
secondResult ->
147+
Tasks.forResult(CombinedTaskResults.create(firstResult, secondResult)));
148+
}
149+
150+
/**
151+
* A version of #combineWithResultsOf that runs the listener that combines the results on the main
152+
* thread.
153+
*/
137154
static <T1, T2> SuccessContinuation<T1, CombinedTaskResults<T1, T2>> combineWithResultOf(
138155
TaskSource<T2> secondTaskSource) {
139156
return firstResult ->

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/AabUpdaterTest.java

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,14 @@
1616
import static com.google.common.truth.Truth.assertThat;
1717
import static com.google.firebase.appdistribution.TestUtils.assertTaskFailure;
1818
import static org.junit.Assert.assertEquals;
19-
import static org.junit.Assert.assertThrows;
2019
import static org.mockito.Mockito.when;
2120
import static org.robolectric.Shadows.shadowOf;
2221

2322
import android.app.Activity;
2423
import android.net.Uri;
25-
import androidx.test.core.app.ApplicationProvider;
2624
import com.google.android.gms.tasks.Tasks;
2725
import com.google.firebase.FirebaseApp;
26+
import com.google.firebase.appdistribution.Constants.ErrorMessages;
2827
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
2928
import java.io.ByteArrayInputStream;
3029
import java.io.IOException;
@@ -85,11 +84,7 @@ public void setup() throws IOException, FirebaseAppDistributionException {
8584

8685
aabUpdater =
8786
Mockito.spy(
88-
new AabUpdater(
89-
ApplicationProvider.getApplicationContext(),
90-
mockLifecycleNotifier,
91-
mockHttpsUrlConnectionFactory,
92-
testExecutor));
87+
new AabUpdater(mockLifecycleNotifier, mockHttpsUrlConnectionFactory, testExecutor));
9388
when(mockLifecycleNotifier.getForegroundActivity()).thenReturn(Tasks.forResult(activity));
9489
}
9590

@@ -100,7 +95,7 @@ public void updateAppTask_whenOpenConnectionFails_setsNetworkFailure()
10095
when(mockHttpsUrlConnectionFactory.openConnection(TEST_URL)).thenThrow(caughtException);
10196

10297
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
103-
testExecutor.awaitTermination(1, TimeUnit.SECONDS);
98+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
10499

105100
assertTaskFailure(
106101
updateTask, Status.NETWORK_FAILURE, "Failed to open connection", caughtException);
@@ -112,7 +107,7 @@ public void updateAppTask_isNotRedirectResponse_setsDownloadFailure()
112107
when(mockHttpsUrlConnection.getResponseCode()).thenReturn(200);
113108

114109
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
115-
testExecutor.awaitTermination(1, TimeUnit.SECONDS);
110+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
116111

117112
assertTaskFailure(updateTask, Status.DOWNLOAD_FAILURE, "Expected redirect");
118113
}
@@ -123,7 +118,7 @@ public void updateAppTask_missingLocationHeader_setsDownloadFailure()
123118
when(mockHttpsUrlConnection.getHeaderField("Location")).thenReturn(null);
124119

125120
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
126-
testExecutor.awaitTermination(1, TimeUnit.SECONDS);
121+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
127122

128123
assertTaskFailure(updateTask, Status.DOWNLOAD_FAILURE, "No Location header");
129124
}
@@ -133,18 +128,18 @@ public void updateAppTask_emptyLocationHeader_setsDownloadFailure() throws Inter
133128
when(mockHttpsUrlConnection.getHeaderField("Location")).thenReturn("");
134129

135130
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
136-
testExecutor.awaitTermination(1, TimeUnit.SECONDS);
131+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
137132

138133
assertTaskFailure(updateTask, Status.DOWNLOAD_FAILURE, "Empty Location header");
139134
}
140135

141136
@Test
142-
public void updateAppTask_whenAabReleaseAvailable_redirectsToPlay() throws InterruptedException {
137+
public void updateAppTask_whenAabReleaseAvailable_redirectsToPlay() throws Exception {
143138
List<UpdateProgress> progressEvents = new ArrayList<>();
144139

145140
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
146141
updateTask.addOnProgressListener(testExecutor, progressEvents::add);
147-
testExecutor.awaitTermination(1, TimeUnit.SECONDS);
142+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
148143

149144
assertThat(shadowActivity.getNextStartedActivity().getData())
150145
.isEqualTo(Uri.parse(REDIRECT_TO_PLAY));
@@ -159,14 +154,14 @@ public void updateAppTask_whenAabReleaseAvailable_redirectsToPlay() throws Inter
159154
}
160155

161156
@Test
162-
public void updateAppTask_onAppResume_setsUpdateCancelled() {
163-
TestOnCompleteListener<Void> onCompleteListener = new TestOnCompleteListener<>();
157+
public void updateAppTask_onAppResume_setsUpdateCancelled() throws InterruptedException {
164158
UpdateTask updateTask = aabUpdater.updateAab(TEST_RELEASE_NEWER_AAB_INTERNAL);
165-
updateTask.addOnCompleteListener(testExecutor, onCompleteListener);
166-
159+
testExecutor.awaitTermination(100, TimeUnit.MILLISECONDS);
167160
aabUpdater.onActivityStarted(activity);
161+
168162
FirebaseAppDistributionException exception =
169-
assertThrows(FirebaseAppDistributionException.class, onCompleteListener::await);
163+
assertTaskFailure(updateTask, Status.INSTALLATION_CANCELED, ErrorMessages.UPDATE_CANCELED);
164+
170165
assertEquals(
171166
ReleaseUtils.convertToAppDistributionRelease(TEST_RELEASE_NEWER_AAB_INTERNAL),
172167
exception.getRelease());

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/TestOnCompleteListener.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
* Helper listener that works around a limitation of the Tasks API where await() cannot be called on
2727
* the main thread. This listener works around it by running itself on a different thread, thus
2828
* allowing the main thread to be woken up when the Tasks complete.
29+
*
30+
* <p>Note: Calling {@link #await()} from a Robolectric test does block the main thread, since those
31+
* tests are executed on the main thread.
2932
*/
3033
public class TestOnCompleteListener<TResult> implements OnCompleteListener<TResult> {
3134
private static final long TIMEOUT_MS = 5000;

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/TestUtils.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@
2222
final class TestUtils {
2323
private TestUtils() {}
2424

25-
static void assertTaskFailure(Task task, Status status, String messageSubstring) {
25+
static FirebaseAppDistributionException assertTaskFailure(
26+
Task task, Status status, String messageSubstring) {
2627
assertThat(task.isSuccessful()).isFalse();
2728
assertThat(task.getException()).isInstanceOf(FirebaseAppDistributionException.class);
2829
FirebaseAppDistributionException e = (FirebaseAppDistributionException) task.getException();
2930
assertThat(e.getErrorCode()).isEqualTo(status);
3031
assertThat(e).hasMessageThat().contains(messageSubstring);
32+
return e;
3133
}
3234

3335
static void assertTaskFailure(

0 commit comments

Comments
 (0)