revert timeout change

Author: themiswang

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

@@ -34,7 +34,6 @@
 import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
 import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import org.json.JSONObject;
 
 public class DefaultSettingsControllerTest extends CrashlyticsTestCase {
@@ -367,14 +366,13 @@ public void testNoAvailableSettingsLoad() throws Exception {
     assertNotNull(controller.getSettingsSync());
     assertFalse(controller.getSettingsAsync().isComplete());
 
-    dataCollectionPermission.trySetException(new TimeoutException("Timeout"));
-    crashlyticsWorkers.common.await();
+    dataCollectionPermission.trySetResult(null);
     await(loadFinished);
 
     assertNotNull(controller.getSettingsSync());
-    assertTrue(controller.getSettingsAsync().isComplete());
+    assertFalse(controller.getSettingsAsync().isComplete());
 
-    verify(mockSettingsSpiCall, times(0)).invoke(any(SettingsRequest.class), eq(true));
+    verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
     verify(mockCachedSettingsIo, times(2)).readCachedSettings();
     verifyNoMoreInteractions(mockSettingsJsonParser);
     verify(mockCurrentTimeProvider).getCurrentTimeMillis();

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

@@ -21,11 +21,7 @@
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
 import com.google.android.gms.tasks.Tasks;
-import com.google.firebase.crashlytics.internal.Logger;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.Executor;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -45,8 +41,7 @@ public final class CrashlyticsTasks {
   public static <T> Task<T> race(Task<T> task1, Task<T> task2) {
     CancellationTokenSource cancellation = new CancellationTokenSource();
     TaskCompletionSource<T> result = new TaskCompletionSource<>(cancellation.getToken());
-    Timer timer = new Timer();
-    Logger.getLogger().d("Race starts " + Thread.currentThread());
+
     AtomicBoolean otherTaskCancelled = new AtomicBoolean(false);
 
     Continuation<T, Task<Void>> continuation =
@@ -67,17 +62,6 @@ public static <T> Task<T> race(Task<T> task1, Task<T> task2) {
     task1.continueWithTask(DIRECT, continuation);
     task2.continueWithTask(DIRECT, continuation);
 
-    timer.schedule(
-        new TimerTask() {
-          @Override
-          public void run() {
-            if (!result.getTask().isComplete()) {
-              Logger.getLogger().d("Race gets timed out " + Thread.currentThread());
-              result.trySetException(new TimeoutException("Race result gets timed out"));
-            }
-          }
-        },
-        10 * 1000);
     return result.getTask();
   }
 

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

@@ -18,7 +18,8 @@
 import android.content.Context;
 import android.content.SharedPreferences;
 import androidx.annotation.NonNull;
-import com.google.android.gms.tasks.Continuation;
+import androidx.annotation.Nullable;
+import com.google.android.gms.tasks.SuccessContinuation;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
 import com.google.android.gms.tasks.Tasks;
@@ -190,44 +191,38 @@ public Task<Void> loadSettingsData(
     // TODO(mrober): Refactor to call worker directly, not expose executor.
     return dataCollectionArbiter
         .waitForDataCollectionPermission()
-        .continueWithTask(
+        .onSuccessTask(
             crashlyticsWorkers.common,
-            new Continuation<Void, Task<Void>>() {
+            new SuccessContinuation<Void, Void>() {
+              @NonNull
               @Override
-              public Task<Void> then(@NonNull Task<Void> task) throws Exception {
-                if (task.isSuccessful()) {
-                  final boolean dataCollectionToken = true;
-                  Future<JSONObject> settingsFuture =
-                      crashlyticsWorkers
-                          .network
-                          .getExecutor()
-                          .submit(
-                              () -> settingsSpiCall.invoke(settingsRequest, dataCollectionToken));
-                  // TODO(mrober): Should we add a timeout here, or let the entire init timeout?
-                  JSONObject settingsJson = settingsFuture.get();
-
-                  if (settingsJson != null) {
-                    final Settings fetchedSettings =
-                        settingsJsonParser.parseSettingsJson(settingsJson);
-                    cachedSettingsIo.writeCachedSettings(
-                        fetchedSettings.expiresAtMillis, settingsJson);
-                    logSettings(settingsJson, "Loaded settings: ");
-
-                    setStoredBuildInstanceIdentifier(settingsRequest.instanceId);
-
-                    // Update the regular settings.
-                    settings.set(fetchedSettings);
-
-                    // Signal the Task that we have a new valid settings
-                    settingsTask.get().trySetResult(fetchedSettings);
-                    return Tasks.forResult(null);
-                  }
+              public Task<Void> then(@Nullable Void aVoid) throws Exception {
+                // Waited for data collection permission, so this is safe.
+                final boolean dataCollectionToken = true;
+                Future<JSONObject> settingsFuture =
+                    crashlyticsWorkers
+                        .network
+                        .getExecutor()
+                        .submit(() -> settingsSpiCall.invoke(settingsRequest, dataCollectionToken));
+                // TODO(mrober): Should we add a timeout here, or let the entire init timeout?
+                JSONObject settingsJson = settingsFuture.get();
+
+                if (settingsJson != null) {
+                  final Settings fetchedSettings =
+                      settingsJsonParser.parseSettingsJson(settingsJson);
+                  cachedSettingsIo.writeCachedSettings(
+                      fetchedSettings.expiresAtMillis, settingsJson);
+                  logSettings(settingsJson, "Loaded settings: ");
+
+                  setStoredBuildInstanceIdentifier(settingsRequest.instanceId);
+
+                  // Update the regular settings.
+                  settings.set(fetchedSettings);
+
+                  // Signal the Task that we have a new valid settings
+                  settingsTask.get().trySetResult(fetchedSettings);
                 }
-                Logger.getLogger()
-                    .d(
-                        "waitForDataCollectionPermission failed, set setting to null "
-                            + Thread.currentThread());
-                settingsTask.get().trySetResult(null);
+
                 return Tasks.forResult(null);
               }
             });