Skip to content

Commit 8dac78d

Browse files
committed
remove some task return
1 parent 53121e2 commit 8dac78d

File tree

8 files changed

+56
-31
lines changed

8 files changed

+56
-31
lines changed

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,9 @@ public void testUploadWithNoReports() throws Exception {
410410

411411
final CrashlyticsController controller = createController();
412412

413-
Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());
413+
controller.submitAllReports(testSettingsProvider.getSettingsAsync());
414414

415-
await(task);
415+
crashlyticsWorkers.common.await();
416416

417417
verify(mockSessionReportingCoordinator).hasReportsToSend();
418418
verifyNoMoreInteractions(mockSessionReportingCoordinator);
@@ -426,9 +426,9 @@ public void testUploadWithDataCollectionAlwaysEnabled() throws Exception {
426426

427427
final CrashlyticsController controller = createController();
428428

429-
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());
429+
controller.submitAllReports(testSettingsProvider.getSettingsAsync());
430430

431-
await(task);
431+
crashlyticsWorkers.common.await();
432432

433433
verify(mockSessionReportingCoordinator).hasReportsToSend();
434434
verify(mockDataCollectionArbiter).isAutomaticDataCollectionEnabled();
@@ -452,15 +452,13 @@ public void testUploadDisabledThenOptIn() throws Exception {
452452
final ControllerBuilder builder = builder();
453453
builder.setDataCollectionArbiter(arbiter);
454454
final CrashlyticsController controller = builder.build();
455-
456-
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());
457-
455+
controller.submitAllReports(testSettingsProvider.getSettingsAsync());
458456
verify(arbiter).isAutomaticDataCollectionEnabled();
459457
verify(mockSessionReportingCoordinator).hasReportsToSend();
460458
verify(mockSessionReportingCoordinator, never()).sendReports(any(Executor.class));
461459

462460
await(controller.sendUnsentReports());
463-
await(task);
461+
crashlyticsWorkers.common.await();
464462

465463
verify(mockSessionReportingCoordinator).sendReports(any(Executor.class));
466464
verifyNoMoreInteractions(mockSessionReportingCoordinator);
@@ -481,14 +479,14 @@ public void testUploadDisabledThenOptOut() throws Exception {
481479
builder.setDataCollectionArbiter(arbiter);
482480
final CrashlyticsController controller = builder.build();
483481

484-
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());
482+
controller.submitAllReports(testSettingsProvider.getSettingsAsync());
485483

486484
verify(arbiter).isAutomaticDataCollectionEnabled();
487485
verify(mockSessionReportingCoordinator).hasReportsToSend();
488486
verify(mockSessionReportingCoordinator, never()).removeAllReports();
489487

490488
await(controller.deleteUnsentReports());
491-
await(task);
489+
crashlyticsWorkers.common.await();
492490

493491
crashlyticsWorkers.diskWrite.await();
494492

@@ -525,7 +523,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
525523
builder.setDataCollectionArbiter(arbiter);
526524
final CrashlyticsController controller = builder.build();
527525

528-
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());
526+
controller.submitAllReports(testSettingsProvider.getSettingsAsync());
529527

530528
verify(mockSessionReportingCoordinator).hasReportsToSend();
531529
verify(mockSessionReportingCoordinator, never()).sendReports(any(Executor.class));
@@ -547,7 +545,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
547545
when(app.isDataCollectionDefaultEnabled()).thenReturn(false);
548546
assertFalse(arbiter.isAutomaticDataCollectionEnabled());
549547

550-
await(task);
548+
crashlyticsWorkers.common.await();
551549

552550
verify(mockSessionReportingCoordinator).sendReports(any(Executor.class));
553551
verifyNoMoreInteractions(mockSessionReportingCoordinator);

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,18 @@
2727
import com.google.android.gms.tasks.Tasks;
2828
import com.google.firebase.concurrent.TestOnlyExecutors;
2929
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
30+
import com.google.firebase.crashlytics.internal.Logger;
3031
import com.google.firebase.crashlytics.internal.common.CurrentTimeProvider;
3132
import com.google.firebase.crashlytics.internal.common.DataCollectionArbiter;
3233
import com.google.firebase.crashlytics.internal.common.DeliveryMechanism;
3334
import com.google.firebase.crashlytics.internal.common.InstallIdProvider;
3435
import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
3536
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
37+
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
38+
3639
import java.util.concurrent.TimeUnit;
40+
import java.util.concurrent.TimeoutException;
41+
3742
import org.json.JSONObject;
3843

3944
public class DefaultSettingsControllerTest extends CrashlyticsTestCase {
@@ -366,13 +371,14 @@ public void testNoAvailableSettingsLoad() throws Exception {
366371
assertNotNull(controller.getSettingsSync());
367372
assertFalse(controller.getSettingsAsync().isComplete());
368373

369-
dataCollectionPermission.trySetResult(null);
374+
dataCollectionPermission.trySetException(new TimeoutException("Timeout"));
375+
crashlyticsWorkers.common.await();
370376
await(loadFinished);
371377

372378
assertNotNull(controller.getSettingsSync());
373-
assertFalse(controller.getSettingsAsync().isComplete());
379+
assertTrue(controller.getSettingsAsync().isComplete());
374380

375-
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
381+
verify(mockSettingsSpiCall, times(0)).invoke(any(SettingsRequest.class), eq(true));
376382
verify(mockCachedSettingsIo, times(2)).readCachedSettings();
377383
verifyNoMoreInteractions(mockSettingsJsonParser);
378384
verify(mockCurrentTimeProvider).getCurrentTimeMillis();

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ public class FirebaseCrashlytics {
168168
core.doBackgroundInitializationAsync(settingsController);
169169
}
170170

171+
Logger.getLogger().i("finish init FirebaseCrashlytics");
172+
171173
return new FirebaseCrashlytics(core);
172174
}
173175

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ public Task<Void> call() throws Exception {
218218
return Tasks.forResult(null);
219219
}
220220

221+
Logger.getLogger().i("handleUncaughtExceptionTask.... " + thread.getName());
222+
221223
return settingsProvider
222224
.getSettingsAsync()
223225
.onSuccessTask(
@@ -339,16 +341,16 @@ Task<Void> deleteUnsentReports() {
339341
return unsentReportsHandled.getTask();
340342
}
341343

342-
Task<Void> submitAllReports(Task<Settings> settingsDataTask) {
344+
Void submitAllReports(Task<Settings> settingsDataTask) {
343345
if (!reportingCoordinator.hasReportsToSend()) {
344346
// Just notify the user that there are no reports and stop.
345347
Logger.getLogger().v("No crash reports are available to be sent.");
346348
unsentReportsAvailable.trySetResult(false);
347-
return Tasks.forResult(null);
349+
return null;
348350
}
349351
Logger.getLogger().v("Crash reports are available to be sent.");
350352

351-
return waitForReportAction()
353+
waitForReportAction()
352354
.onSuccessTask(
353355
crashlyticsWorkers.common,
354356
new SuccessContinuation<Boolean, Void>() {
@@ -360,6 +362,8 @@ public Task<Void> then(@Nullable Boolean send) throws Exception {
360362
deleteFiles(listAppExceptionMarkerFiles());
361363
reportingCoordinator.removeAllReports();
362364
unsentReportsHandled.trySetResult(null);
365+
Logger.getLogger().i("waitForReportAction: SuccessContinuation no unsend report" + Thread.currentThread());
366+
363367
return Tasks.forResult(null);
364368
}
365369

@@ -371,7 +375,7 @@ public Task<Void> then(@Nullable Boolean send) throws Exception {
371375
// Signal to the settings fetch and onboarding that we have explicit
372376
// permission.
373377
dataCollectionArbiter.grantDataCollectionPermission(dataCollectionToken);
374-
378+
Logger.getLogger().i("waitForReportAction: SuccessContinuation before send" + Thread.currentThread());
375379
return settingsDataTask.onSuccessTask(
376380
crashlyticsWorkers.common,
377381
new SuccessContinuation<Settings, Void>() {
@@ -393,6 +397,7 @@ public Task<Void> then(@Nullable Settings appSettingsData) throws Exception {
393397
});
394398
}
395399
});
400+
return null;
396401
}
397402

398403
// region Internal "public" API for data capture

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.google.firebase.crashlytics.internal.stacktrace.StackTraceTrimmingStrategy;
4242
import java.util.Map;
4343
import java.util.concurrent.ExecutionException;
44+
import java.util.concurrent.Executors;
4445
import java.util.concurrent.Future;
4546
import java.util.concurrent.TimeUnit;
4647
import java.util.concurrent.TimeoutException;
@@ -224,13 +225,19 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider)
224225

225226
/** Performs background initialization asynchronously on the common worker. */
226227
@CanIgnoreReturnValue
227-
public Task<Settings> doBackgroundInitializationAsync(SettingsProvider settingsProvider) {
228-
return crashlyticsWorkers.common.submitTask(() -> doBackgroundInitialization(settingsProvider));
228+
public Task<Void> doBackgroundInitializationAsync(SettingsProvider settingsProvider) {
229+
return crashlyticsWorkers.common.submit(() -> {
230+
try{
231+
doBackgroundInitialization(settingsProvider);
232+
} catch (Exception e) {
233+
Logger.getLogger().e(e.toString());
234+
}
235+
});
229236
}
230237

231238
/** Performs background initialization synchronously on the calling thread. */
232239
@CanIgnoreReturnValue
233-
private Task<Settings> doBackgroundInitialization(SettingsProvider settingsProvider) {
240+
private Void doBackgroundInitialization(SettingsProvider settingsProvider) throws Exception {
234241
CrashlyticsWorkers.checkBackgroundThread();
235242
// create the marker for this run
236243
markInitializationStarted();
@@ -246,8 +253,7 @@ private Task<Settings> doBackgroundInitialization(SettingsProvider settingsProvi
246253
Logger.getLogger().d("Collection of crash reports disabled in Crashlytics settings.");
247254
// TODO: This isn't actually an error condition, so figure out the right way to
248255
// handle this case.
249-
return Tasks.forException(
250-
new RuntimeException("Collection of crash reports disabled in Crashlytics settings."));
256+
throw new RuntimeException("Collection of crash reports disabled in Crashlytics settings.");
251257
}
252258

253259
if (!controller.finalizeSessions(settingsProvider)) {
@@ -256,11 +262,12 @@ private Task<Settings> doBackgroundInitialization(SettingsProvider settingsProvi
256262

257263
Task<Settings> settings = settingsProvider.getSettingsAsync();
258264
controller.submitAllReports(settings);
259-
return settings;
265+
Logger.getLogger().i("after submitAllReports " + Thread.currentThread());
266+
return null;
260267
} catch (Exception e) {
261268
Logger.getLogger()
262269
.e("Crashlytics encountered a problem during asynchronous initialization.", e);
263-
return Tasks.forException(e);
270+
throw e;
264271
} finally {
265272
// The only thing that compels us to leave the marker and start synchronously next time
266273
// is not executing all the way through this method. That would indicate that we perhaps

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/DataCollectionArbiter.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ public Task<Void> waitForAutomaticDataCollectionEnabled() {
130130
* enabled, or 2) grantDataCollectionPermission has been called.
131131
*/
132132
public Task<Void> waitForDataCollectionPermission() {
133+
Logger.getLogger().i("waitForDataCollectionPermission " + Thread.currentThread());
133134
return CrashlyticsTasks.race(
134135
dataCollectionExplicitlyApproved.getTask(), waitForAutomaticDataCollectionEnabled());
135136
}

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsTasks.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public static <T> Task<T> race(Task<T> task1, Task<T> task2) {
4646
CancellationTokenSource cancellation = new CancellationTokenSource();
4747
TaskCompletionSource<T> result = new TaskCompletionSource<>(cancellation.getToken());
4848
Timer timer = new Timer();
49-
49+
Logger.getLogger().d("Start race~~~~~ " + Thread.currentThread());
5050
AtomicBoolean otherTaskCancelled = new AtomicBoolean(false);
5151

5252
Continuation<T, Task<Void>> continuation =
@@ -71,8 +71,10 @@ public static <T> Task<T> race(Task<T> task1, Task<T> task2) {
7171
new TimerTask() {
7272
@Override
7373
public void run() {
74-
Logger.getLogger().d("Race gets timed out");
75-
result.trySetException(new TimeoutException("Race result gets timed out"));
74+
if (!result.getTask().isComplete()){
75+
Logger.getLogger().d("Race gets timed out " + Thread.currentThread());
76+
result.trySetException(new TimeoutException("Race result gets timed out"));
77+
}
7678
}
7779
},
7880
10 * 1000);

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/SettingsController.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.google.firebase.crashlytics.internal.network.HttpRequestFactory;
3434
import com.google.firebase.crashlytics.internal.persistence.FileStore;
3535
import java.util.Locale;
36+
import java.util.concurrent.Executor;
37+
import java.util.concurrent.Executors;
3638
import java.util.concurrent.Future;
3739
import java.util.concurrent.atomic.AtomicReference;
3840
import org.json.JSONObject;
@@ -131,6 +133,7 @@ public static SettingsController create(
131133
*/
132134
@Override
133135
public Task<Settings> getSettingsAsync() {
136+
Logger.getLogger().i("getSettingsAsync " + Thread.currentThread());
134137
return settingsTask.get().getTask();
135138
}
136139

@@ -149,6 +152,7 @@ public Settings getSettingsSync() {
149152
* @return a task that is resolved when loading is completely finished.
150153
*/
151154
public Task<Void> loadSettingsData(CrashlyticsWorkers crashlyticsWorkers) {
155+
Logger.getLogger().i("loadSettingsData wrapper " + Thread.currentThread() );
152156
return loadSettingsData(SettingsCacheBehavior.USE_CACHE, crashlyticsWorkers);
153157
}
154158

@@ -161,7 +165,7 @@ public Task<Void> loadSettingsData(
161165
SettingsCacheBehavior cacheBehavior, CrashlyticsWorkers crashlyticsWorkers) {
162166
// TODO: Refactor this so that it doesn't do the cache lookup twice when settings are
163167
// expired.
164-
168+
Logger.getLogger().i("loadSettingsData " + Thread.currentThread() );
165169
// We need to bypass the cache if this is the first time a new build has run so the
166170
// backend will know about it.
167171
if (!buildInstanceIdentifierChanged()) {
@@ -222,7 +226,7 @@ public Task<Void> then(@NonNull Task<Void> task) throws Exception {
222226
return Tasks.forResult(null);
223227
}
224228
}
225-
Logger.getLogger().d("waitForDataCollectionPermission failed, set setting to null");
229+
Logger.getLogger().d("waitForDataCollectionPermission failed, set setting to null " + Thread.currentThread());
226230
settingsTask.get().trySetResult(null);
227231
return Tasks.forResult(null);
228232
}

0 commit comments

Comments
 (0)