Skip to content

Commit d0fd0c8

Browse files
themiswangthemiswang
committed
remove some task return
1 parent 53121e2 commit d0fd0c8

File tree

8 files changed

+59
-32
lines changed

8 files changed

+59
-32
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: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
3535
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
3636
import java.util.concurrent.TimeUnit;
37+
import java.util.concurrent.TimeoutException;
3738
import org.json.JSONObject;
3839

3940
public class DefaultSettingsControllerTest extends CrashlyticsTestCase {
@@ -366,13 +367,14 @@ public void testNoAvailableSettingsLoad() throws Exception {
366367
assertNotNull(controller.getSettingsSync());
367368
assertFalse(controller.getSettingsAsync().isComplete());
368369

369-
dataCollectionPermission.trySetResult(null);
370+
dataCollectionPermission.trySetException(new TimeoutException("Timeout"));
371+
crashlyticsWorkers.common.await();
370372
await(loadFinished);
371373

372374
assertNotNull(controller.getSettingsSync());
373-
assertFalse(controller.getSettingsAsync().isComplete());
375+
assertTrue(controller.getSettingsAsync().isComplete());
374376

375-
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
377+
verify(mockSettingsSpiCall, times(0)).invoke(any(SettingsRequest.class), eq(true));
376378
verify(mockCachedSettingsIo, times(2)).readCachedSettings();
377379
verifyNoMoreInteractions(mockSettingsJsonParser);
378380
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: 15 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,11 @@ public Task<Void> then(@Nullable Boolean send) throws Exception {
360362
deleteFiles(listAppExceptionMarkerFiles());
361363
reportingCoordinator.removeAllReports();
362364
unsentReportsHandled.trySetResult(null);
365+
Logger.getLogger()
366+
.i(
367+
"waitForReportAction: SuccessContinuation no unsend report"
368+
+ Thread.currentThread());
369+
363370
return Tasks.forResult(null);
364371
}
365372

@@ -371,7 +378,10 @@ public Task<Void> then(@Nullable Boolean send) throws Exception {
371378
// Signal to the settings fetch and onboarding that we have explicit
372379
// permission.
373380
dataCollectionArbiter.grantDataCollectionPermission(dataCollectionToken);
374-
381+
Logger.getLogger()
382+
.i(
383+
"waitForReportAction: SuccessContinuation before send"
384+
+ Thread.currentThread());
375385
return settingsDataTask.onSuccessTask(
376386
crashlyticsWorkers.common,
377387
new SuccessContinuation<Settings, Void>() {
@@ -393,6 +403,7 @@ public Task<Void> then(@Nullable Settings appSettingsData) throws Exception {
393403
});
394404
}
395405
});
406+
return null;
396407
}
397408

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

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import androidx.annotation.Nullable;
2222
import androidx.annotation.VisibleForTesting;
2323
import com.google.android.gms.tasks.Task;
24-
import com.google.android.gms.tasks.Tasks;
2524
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2625
import com.google.firebase.FirebaseApp;
2726
import com.google.firebase.crashlytics.BuildConfig;
@@ -224,13 +223,20 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider)
224223

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

231237
/** Performs background initialization synchronously on the calling thread. */
232238
@CanIgnoreReturnValue
233-
private Task<Settings> doBackgroundInitialization(SettingsProvider settingsProvider) {
239+
private Void doBackgroundInitialization(SettingsProvider settingsProvider) throws Exception {
234240
CrashlyticsWorkers.checkBackgroundThread();
235241
// create the marker for this run
236242
markInitializationStarted();
@@ -246,8 +252,7 @@ private Task<Settings> doBackgroundInitialization(SettingsProvider settingsProvi
246252
Logger.getLogger().d("Collection of crash reports disabled in Crashlytics settings.");
247253
// TODO: This isn't actually an error condition, so figure out the right way to
248254
// handle this case.
249-
return Tasks.forException(
250-
new RuntimeException("Collection of crash reports disabled in Crashlytics settings."));
255+
throw new RuntimeException("Collection of crash reports disabled in Crashlytics settings.");
251256
}
252257

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

257262
Task<Settings> settings = settingsProvider.getSettingsAsync();
258263
controller.submitAllReports(settings);
259-
return settings;
264+
Logger.getLogger().i("after submitAllReports " + Thread.currentThread());
265+
return null;
260266
} catch (Exception e) {
261267
Logger.getLogger()
262268
.e("Crashlytics encountered a problem during asynchronous initialization.", e);
263-
return Tasks.forException(e);
269+
throw e;
264270
} finally {
265271
// The only thing that compels us to leave the marker and start synchronously next time
266272
// 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: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ public static SettingsController create(
131131
*/
132132
@Override
133133
public Task<Settings> getSettingsAsync() {
134+
Logger.getLogger().i("getSettingsAsync " + Thread.currentThread());
134135
return settingsTask.get().getTask();
135136
}
136137

@@ -149,6 +150,7 @@ public Settings getSettingsSync() {
149150
* @return a task that is resolved when loading is completely finished.
150151
*/
151152
public Task<Void> loadSettingsData(CrashlyticsWorkers crashlyticsWorkers) {
153+
Logger.getLogger().i("loadSettingsData wrapper " + Thread.currentThread());
152154
return loadSettingsData(SettingsCacheBehavior.USE_CACHE, crashlyticsWorkers);
153155
}
154156

@@ -161,7 +163,7 @@ public Task<Void> loadSettingsData(
161163
SettingsCacheBehavior cacheBehavior, CrashlyticsWorkers crashlyticsWorkers) {
162164
// TODO: Refactor this so that it doesn't do the cache lookup twice when settings are
163165
// expired.
164-
166+
Logger.getLogger().i("loadSettingsData " + Thread.currentThread());
165167
// We need to bypass the cache if this is the first time a new build has run so the
166168
// backend will know about it.
167169
if (!buildInstanceIdentifierChanged()) {
@@ -222,7 +224,10 @@ public Task<Void> then(@NonNull Task<Void> task) throws Exception {
222224
return Tasks.forResult(null);
223225
}
224226
}
225-
Logger.getLogger().d("waitForDataCollectionPermission failed, set setting to null");
227+
Logger.getLogger()
228+
.d(
229+
"waitForDataCollectionPermission failed, set setting to null "
230+
+ Thread.currentThread());
226231
settingsTask.get().trySetResult(null);
227232
return Tasks.forResult(null);
228233
}

0 commit comments

Comments
 (0)