Skip to content

Commit fdda906

Browse files
schmidt-sebastianschmidt-sebastian
committed
Merge branch 'master' into mrschmidt/offset
2 parents a5fcb80 + ab6bab4 commit fdda906

File tree

18 files changed

+262
-228
lines changed

18 files changed

+262
-228
lines changed

‎firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/CheckForNewReleaseClient.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import java.util.concurrent.Executors;
3838

3939
class CheckForNewReleaseClient {
40-
private static final int NEW_RELEASE_THREAD_POOL_SIZE = 4;
4140
private static final String TAG = "CheckForNewReleaseClient:";
4241

4342
private final FirebaseApp firebaseApp;
@@ -56,8 +55,7 @@ class CheckForNewReleaseClient {
5655
this.firebaseApp = firebaseApp;
5756
this.firebaseAppDistributionTesterApiClient = firebaseAppDistributionTesterApiClient;
5857
this.firebaseInstallationsApi = firebaseInstallationsApi;
59-
// TODO: verify if this is best way to use executorservice here
60-
this.checkForNewReleaseExecutor = Executors.newFixedThreadPool(NEW_RELEASE_THREAD_POOL_SIZE);
58+
this.checkForNewReleaseExecutor = Executors.newSingleThreadExecutor();
6159
this.releaseIdentifierStorage =
6260
new ReleaseIdentifierStorage(firebaseApp.getApplicationContext());
6361
}

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

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import android.content.Context;
2424
import androidx.annotation.GuardedBy;
2525
import androidx.annotation.NonNull;
26+
import androidx.annotation.Nullable;
2627
import androidx.annotation.VisibleForTesting;
2728
import com.google.android.gms.common.internal.Preconditions;
2829
import com.google.android.gms.tasks.Task;
@@ -35,24 +36,28 @@
3536
import com.google.firebase.installations.FirebaseInstallationsApi;
3637

3738
public class FirebaseAppDistribution {
39+
private static final int UNKNOWN_RELEASE_FILE_SIZE = -1;
3840

3941
private final FirebaseApp firebaseApp;
4042
private final TesterSignInClient testerSignInClient;
4143
private final CheckForNewReleaseClient checkForNewReleaseClient;
4244
private final UpdateAppClient updateAppClient;
4345
private final FirebaseAppDistributionLifecycleNotifier lifecycleNotifier;
44-
private static final int UNKNOWN_RELEASE_FILE_SIZE = -1;
46+
private final SignInStorage signInStorage;
4547

46-
@GuardedBy("updateTaskLock")
48+
private final Object updateIfNewReleaseTaskLock = new Object();
49+
50+
@GuardedBy("updateIfNewReleaseTaskLock")
4751
private UpdateTaskImpl cachedUpdateIfNewReleaseTask;
4852

49-
private final Object updateTaskLock = new Object();
50-
private Task<AppDistributionRelease> cachedCheckForNewReleaseTask;
53+
private final Object cachedNewReleaseLock = new Object();
5154

55+
@GuardedBy("cachedNewReleaseLock")
5256
private AppDistributionReleaseInternal cachedNewRelease;
57+
58+
private Task<AppDistributionRelease> cachedCheckForNewReleaseTask;
5359
private AlertDialog updateDialog;
5460
private boolean updateDialogShown;
55-
private final SignInStorage signInStorage;
5661

5762
/** Constructor for FirebaseAppDistribution */
5863
@VisibleForTesting
@@ -126,8 +131,8 @@ public static FirebaseAppDistribution getInstance(@NonNull FirebaseApp app) {
126131
* to complete the download and installation.
127132
*/
128133
@NonNull
129-
public synchronized UpdateTask updateIfNewReleaseAvailable() {
130-
synchronized (updateTaskLock) {
134+
public UpdateTask updateIfNewReleaseAvailable() {
135+
synchronized (updateIfNewReleaseTaskLock) {
131136
if (cachedUpdateIfNewReleaseTask != null && !cachedUpdateIfNewReleaseTask.isComplete()) {
132137
return cachedUpdateIfNewReleaseTask;
133138
}
@@ -163,7 +168,8 @@ public synchronized UpdateTask updateIfNewReleaseAvailable() {
163168
Constants.ErrorMessages.NETWORK_ERROR,
164169
FirebaseAppDistributionException.Status.NETWORK_FAILURE));
165170
});
166-
synchronized (updateTaskLock) {
171+
172+
synchronized (updateIfNewReleaseTaskLock) {
167173
return cachedUpdateIfNewReleaseTask;
168174
}
169175
}
@@ -218,15 +224,15 @@ public synchronized Task<AppDistributionRelease> checkForNewRelease() {
218224
* new release is cached from checkForNewRelease
219225
*/
220226
@NonNull
221-
public synchronized UpdateTask updateApp() {
227+
public UpdateTask updateApp() {
222228
return updateApp(false);
223229
}
224230

225231
/**
226232
* Overloaded updateApp with boolean input showDownloadInNotificationsManager. Set to true for
227233
* basic configuration and false for advanced configuration.
228234
*/
229-
private synchronized UpdateTask updateApp(boolean showDownloadInNotificationManager) {
235+
private UpdateTask updateApp(boolean showDownloadInNotificationManager) {
230236
if (!isTesterSignedIn()) {
231237
UpdateTaskImpl updateTask = new UpdateTaskImpl();
232238
updateTask.setException(
@@ -235,7 +241,9 @@ private synchronized UpdateTask updateApp(boolean showDownloadInNotificationMana
235241
return updateTask;
236242
}
237243

238-
return this.updateAppClient.updateApp(cachedNewRelease, showDownloadInNotificationManager);
244+
synchronized (cachedNewReleaseLock) {
245+
return this.updateAppClient.updateApp(cachedNewRelease, showDownloadInNotificationManager);
246+
}
239247
}
240248

241249
/** Returns true if the App Distribution tester is signed in */
@@ -245,7 +253,7 @@ public boolean isTesterSignedIn() {
245253

246254
/** Signs out the App Distribution tester */
247255
public void signOutTester() {
248-
this.cachedNewRelease = null;
256+
setCachedNewRelease(null);
249257
this.signInStorage.setSignInStatus(false);
250258
}
251259

@@ -263,13 +271,17 @@ void onActivityDestroyed(@NonNull Activity activity) {
263271
}
264272

265273
@VisibleForTesting
266-
void setCachedNewRelease(AppDistributionReleaseInternal newRelease) {
267-
this.cachedNewRelease = newRelease;
274+
void setCachedNewRelease(@Nullable AppDistributionReleaseInternal newRelease) {
275+
synchronized (cachedNewReleaseLock) {
276+
this.cachedNewRelease = newRelease;
277+
}
268278
}
269279

270280
@VisibleForTesting
271281
AppDistributionReleaseInternal getCachedNewRelease() {
272-
return this.cachedNewRelease;
282+
synchronized (cachedNewReleaseLock) {
283+
return this.cachedNewRelease;
284+
}
273285
}
274286

275287
private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease) {
@@ -302,7 +314,7 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
302314
AlertDialog.BUTTON_POSITIVE,
303315
context.getString(R.string.update_yes_button),
304316
(dialogInterface, i) -> {
305-
synchronized (updateTaskLock) {
317+
synchronized (updateIfNewReleaseTaskLock) {
306318
// show download progress in notification manager
307319
updateApp(true)
308320
.addOnProgressListener(this::postProgressToCachedUpdateIfNewReleaseTask)
@@ -316,7 +328,7 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
316328
context.getString(R.string.update_no_button),
317329
(dialogInterface, i) -> {
318330
dialogInterface.dismiss();
319-
synchronized (updateTaskLock) {
331+
synchronized (updateIfNewReleaseTaskLock) {
320332
postProgressToCachedUpdateIfNewReleaseTask(
321333
UpdateProgress.builder()
322334
.setApkFileTotalBytes(UNKNOWN_RELEASE_FILE_SIZE)
@@ -331,16 +343,16 @@ private UpdateTaskImpl showUpdateAlertDialog(AppDistributionRelease newRelease)
331343

332344
updateDialog.show();
333345
updateDialogShown = true;
334-
synchronized (updateTaskLock) {
346+
synchronized (updateIfNewReleaseTaskLock) {
335347
return cachedUpdateIfNewReleaseTask;
336348
}
337349
}
338350

339351
private void setCachedUpdateIfNewReleaseCompletionError(FirebaseAppDistributionException e) {
340-
synchronized (updateTaskLock) {
352+
synchronized (updateIfNewReleaseTaskLock) {
341353
safeSetTaskException(cachedUpdateIfNewReleaseTask, e);
342-
dismissUpdateDialog();
343354
}
355+
dismissUpdateDialog();
344356
}
345357

346358
private void setCachedUpdateIfNewReleaseCompletionError(
@@ -353,16 +365,16 @@ private void setCachedUpdateIfNewReleaseCompletionError(
353365
}
354366

355367
private void postProgressToCachedUpdateIfNewReleaseTask(UpdateProgress progress) {
356-
synchronized (updateTaskLock) {
368+
synchronized (updateIfNewReleaseTaskLock) {
357369
cachedUpdateIfNewReleaseTask.updateProgress(progress);
358370
}
359371
}
360372

361373
private void setCachedUpdateIfNewReleaseResult() {
362-
synchronized (updateTaskLock) {
374+
synchronized (updateIfNewReleaseTaskLock) {
363375
safeSetTaskResult(cachedUpdateIfNewReleaseTask);
364-
dismissUpdateDialog();
365376
}
377+
dismissUpdateDialog();
366378
}
367379

368380
private void dismissUpdateDialog() {

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

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import android.content.Intent;
2727
import android.content.pm.ResolveInfo;
2828
import android.net.Uri;
29+
import androidx.annotation.GuardedBy;
2930
import androidx.annotation.NonNull;
3031
import androidx.annotation.VisibleForTesting;
3132
import androidx.browser.customtabs.CustomTabsIntent;
@@ -42,16 +43,19 @@
4243

4344
class TesterSignInClient {
4445
private static final String TAG = "TesterSignIn:";
45-
46-
private TaskCompletionSource<Void> signInTaskCompletionSource = null;
47-
48-
private final String SIGNIN_REDIRECT_URL =
46+
private static final String SIGNIN_REDIRECT_URL =
4947
"https://appdistribution.firebase.google.com/pub/testerapps/%s/installations/%s/buildalerts?appName=%s&packageName=%s";
48+
5049
private final FirebaseApp firebaseApp;
5150
private final FirebaseInstallationsApi firebaseInstallationsApi;
5251
private final SignInStorage signInStorage;
5352
private final FirebaseAppDistributionLifecycleNotifier lifecycleNotifier;
5453

54+
private final Object signInTaskLock = new Object();
55+
56+
@GuardedBy("signInTaskLock")
57+
private TaskCompletionSource<Void> signInTaskCompletionSource = null;
58+
5559
private AlertDialog alertDialog;
5660

5761
TesterSignInClient(
@@ -113,36 +117,42 @@ private void onActivityDestroyed(Activity activity) {
113117
}
114118

115119
@NonNull
116-
public synchronized Task<Void> signInTester() {
120+
public Task<Void> signInTester() {
117121

118122
if (signInStorage.getSignInStatus()) {
119123
LogWrapper.getInstance().v(TAG + "Tester is already signed in.");
120124
return Tasks.forResult(null);
121125
}
122126

123-
if (this.isCurrentlySigningIn()) {
124-
LogWrapper.getInstance()
125-
.v(TAG + "Detected In-Progress sign in task. Returning the same task.");
126-
return signInTaskCompletionSource.getTask();
127-
}
128-
Activity currentActivity = lifecycleNotifier.getCurrentActivity();
129-
if (currentActivity == null) {
130-
LogWrapper.getInstance().e(TAG + "No foreground activity found.");
131-
return Tasks.forException(
132-
new FirebaseAppDistributionException(
133-
ErrorMessages.APP_BACKGROUNDED,
134-
FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE));
135-
}
136-
signInTaskCompletionSource = new TaskCompletionSource<>();
127+
synchronized (signInTaskLock) {
128+
if (this.isCurrentlySigningIn()) {
129+
LogWrapper.getInstance()
130+
.v(TAG + "Detected In-Progress sign in task. Returning the same task.");
131+
return signInTaskCompletionSource.getTask();
132+
}
133+
Activity currentActivity = lifecycleNotifier.getCurrentActivity();
134+
if (currentActivity == null) {
135+
LogWrapper.getInstance().e(TAG + "No foreground activity found.");
136+
return Tasks.forException(
137+
new FirebaseAppDistributionException(
138+
ErrorMessages.APP_BACKGROUNDED,
139+
FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE));
140+
}
137141

138-
alertDialog = getSignInAlertDialog(currentActivity);
139-
alertDialog.show();
142+
signInTaskCompletionSource = new TaskCompletionSource<>();
140143

141-
return signInTaskCompletionSource.getTask();
144+
alertDialog = getSignInAlertDialog(currentActivity);
145+
alertDialog.show();
146+
147+
return signInTaskCompletionSource.getTask();
148+
}
142149
}
143150

144151
private boolean isCurrentlySigningIn() {
145-
return signInTaskCompletionSource != null && !signInTaskCompletionSource.getTask().isComplete();
152+
synchronized (signInTaskLock) {
153+
return signInTaskCompletionSource != null
154+
&& !signInTaskCompletionSource.getTask().isComplete();
155+
}
146156
}
147157

148158
private AlertDialog getSignInAlertDialog(Activity currentActivity) {
@@ -190,7 +200,9 @@ public void onClick(DialogInterface dialogInterface, int i) {
190200
}
191201

192202
private void setSignInTaskCompletionError(FirebaseAppDistributionException e) {
193-
safeSetTaskException(signInTaskCompletionSource, e);
203+
synchronized (signInTaskLock) {
204+
safeSetTaskException(signInTaskCompletionSource, e);
205+
}
194206
dismissAlertDialog();
195207
}
196208

@@ -201,7 +213,9 @@ private void setCanceledAuthenticationError() {
201213
}
202214

203215
private void setSuccessfulSignInResult() {
204-
safeSetTaskResult(signInTaskCompletionSource, null);
216+
synchronized (signInTaskLock) {
217+
safeSetTaskResult(signInTaskCompletionSource, null);
218+
}
205219
dismissAlertDialog();
206220
}
207221

‎firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/UpdateApkClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public UpdateApkClient(
7474
this.installApkClient = installApkClient;
7575
}
7676

77-
public synchronized UpdateTaskImpl updateApk(
77+
public UpdateTaskImpl updateApk(
7878
@NonNull AppDistributionReleaseInternal newRelease, boolean showDownloadNotificationManager) {
7979
synchronized (updateTaskLock) {
8080
if (cachedUpdateTask != null && !cachedUpdateTask.isComplete()) {

‎firebase-crashlytics-ndk/src/third_party/crashpad

Submodule crashpad updated from e5e47bc to 281ba70

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ public void tearDown() {
4949
}
5050

5151
@Test
52-
public void testCanConfigureIndices() throws ExecutionException, InterruptedException {
52+
public void testCanConfigureIndexes() throws ExecutionException, InterruptedException {
5353
FirebaseFirestore db = testFirestore();
5454
Task<Void> indexTask =
55-
db.configureIndices(
55+
db.setIndexConfiguration(
5656
"{\n"
5757
+ " \"indexes\": [\n"
5858
+ " {\n"
@@ -89,7 +89,7 @@ public void testCanConfigureIndices() throws ExecutionException, InterruptedExce
8989
public void testBadJsonDoesNotCrashClient() {
9090
FirebaseFirestore db = testFirestore();
9191

92-
Assert.assertThrows(IllegalArgumentException.class, () -> db.configureIndices("{,"));
92+
Assert.assertThrows(IllegalArgumentException.class, () -> db.setIndexConfiguration("{,"));
9393
}
9494

9595
@Test
@@ -98,7 +98,7 @@ public void testBadIndexDoesNotCrashClient() {
9898
Assert.assertThrows(
9999
IllegalArgumentException.class,
100100
() ->
101-
db.configureIndices(
101+
db.setIndexConfiguration(
102102
"{\n"
103103
+ " \"indexes\": [\n"
104104
+ " {\n"

‎firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,14 @@ public FirebaseApp getApp() {
297297
* automatically start using the index once the index entries have been written.
298298
*
299299
* <p>The method accepts the JSON format exported by the Firebase CLI (`firebase
300-
* firestore:indexes`). If the JSON format is invalid, this method rejects the returned task.
300+
* firestore:indexes`). If the JSON format is invalid, this method throws an exception.
301301
*
302302
* @param json The JSON format exported by the Firebase CLI.
303303
* @return A task that resolves once all indices are successfully configured.
304+
* @throws IllegalArgumentException if the JSON format is invalid
304305
*/
305306
@VisibleForTesting
306-
Task<Void> configureIndices(String json) {
307+
Task<Void> setIndexConfiguration(String json) {
307308
ensureClientConfigured();
308309

309310
// Preconditions.checkState(BuildConfig.ENABLE_INDEXING, "Indexing support is not yet

0 commit comments

Comments
 (0)