Clean up CrashlyticsWorkers, and fix some comments

Author: mrober

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

@@ -14,8 +14,6 @@
 
 package com.google.firebase.crashlytics;
 
-import static com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers.StrictLevel.ASSERT;
-
 import com.google.firebase.FirebaseApp;
 import com.google.firebase.analytics.connector.AnalyticsConnector;
 import com.google.firebase.annotations.concurrent.Background;
@@ -69,10 +67,8 @@ public List<Component<?>> getComponents() {
   }
 
   private FirebaseCrashlytics buildCrashlytics(ComponentContainer container) {
-    // TODO(mrober): Make this a build time configuration. Do not release like this.
-    CrashlyticsWorkers.setStrictLevel(ASSERT); // Kill the process on violation for debugging.
+    CrashlyticsWorkers.setEnforcement(BuildConfig.DEBUG);
 
-    // CrashlyticsWorkers.checkMainThread();
     long startTime = System.currentTimeMillis();
 
     FirebaseCrashlytics crashlytics =
@@ -86,8 +82,8 @@ private FirebaseCrashlytics buildCrashlytics(ComponentContainer container) {
             container.get(blockingExecutorService));
 
     long duration = System.currentTimeMillis() - startTime;
-    if (duration > 30) {
-      Logger.getLogger().i("Initializing Crashlytics blocked main for " + duration + " ms");
+    if (duration > 16) {
+      Logger.getLogger().d("Initializing Crashlytics blocked main for " + duration + " ms");
     }
 
     return crashlytics;

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

@@ -33,8 +33,8 @@
 /**
  * Worker for executing tasks sequentially on the given executor service.
  *
- * <p>Work on the queue may block, or it may return a Task, such that the underlying thread may be
- * re-used while the worker queue is still blocked.
+ * <p>Work on the queue may suspend, or it may return a Task, such that the underlying thread may be
+ * re-used while the worker queue is suspended.
  *
  * <p>Work enqueued on this worker will be run serially, regardless of the underlying executor.
  * Therefore, workers on the queue should not add new work to the queue and then block on it, as
@@ -104,7 +104,7 @@ public Task<Void> submit(Runnable runnable) {
   /**
    * Submits a <code>Callable Task</code> for asynchronous execution on the executor.
    *
-   * <p>This is useful for making the worker block on an asynchronous operation, while letting the
+   * <p>This is useful for making the worker suspend on an asynchronous operation, while letting the
    * underlying threads be re-used.
    *
    * <p>Returns a <code>Task</code> which will be resolved upon successful completion of the Task

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

@@ -16,13 +16,7 @@
 
 package com.google.firebase.crashlytics.internal.concurrency
 
-import android.os.Build
-import android.os.Looper
 import com.google.firebase.crashlytics.internal.Logger
-import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers.StrictLevel.ASSERT
-import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers.StrictLevel.NONE
-import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers.StrictLevel.THROW
-import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers.StrictLevel.WARN
 import java.util.concurrent.ExecutorService
 
 /**
@@ -37,19 +31,20 @@ class CrashlyticsWorkers(
 
   /**
    * The common worker is for common background tasks, like background init, user actions, and
-   * processing uncaught exceptions. This is the main worker of the sdk.
+   * processing uncaught exceptions. This is the main worker of the sdk. This worker will never
+   * block on a disk write or network call.
    */
   @JvmField val common = CrashlyticsWorker(backgroundExecutorService)
 
   /**
-   * The disk write worker is for background tasks that persisting data to local disk. Ideally, no
-   * user action should require waiting on this (although still do).
+   * The disk write worker is for background tasks that persisting data to local disk. No user
+   * action should wait on this. Use for fire and forget, safe to ignore exceptions.
    */
   @JvmField val diskWrite = CrashlyticsWorker(backgroundExecutorService)
 
   /**
    * The data collect worker is for any background tasks that send data remotely, like fetching fid,
-   * settings, or uploading crash reports. This worker is blocked until permission is granted.
+   * settings, or uploading crash reports. This worker is suspended until permission is granted.
    */
   @JvmField val dataCollect = CrashlyticsWorker(backgroundExecutorService)
 
@@ -61,13 +56,8 @@ class CrashlyticsWorkers(
     private val threadName
       get() = Thread.currentThread().name
 
-    @JvmStatic var strictLevel: StrictLevel = NONE
-
-    @JvmStatic
-    fun checkMainThread() =
-      checkThread(::isMainThread) {
-        "Must be called on the main thread, was called on $threadName."
-      }
+    /** When enabled, failed preconditions will cause assertion errors for debugging. */
+    @JvmStatic var enforcement: Boolean = false
 
     @JvmStatic
     fun checkBlockingThread() =
@@ -81,34 +71,15 @@ class CrashlyticsWorkers(
         "Must be called on a background thread, was called on $threadName."
       }
 
-    private fun isMainThread() =
-      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-        Looper.getMainLooper().isCurrentThread
-      } else {
-        Looper.getMainLooper() == Looper.myLooper()
-      }
-
     private fun isBlockingThread() = threadName.contains("Firebase Blocking Thread #")
 
     private fun isBackgroundThread() = threadName.contains("Firebase Background Thread #")
 
     private fun checkThread(isCorrectThread: () -> Boolean, failureMessage: () -> String) {
-      if (strictLevel.level >= WARN.level && !isCorrectThread()) {
-        Logger.getLogger().w(failureMessage())
-        assert(strictLevel.level < ASSERT.level, failureMessage)
-        check(strictLevel.level < THROW.level, failureMessage)
+      if (!isCorrectThread()) {
+        Logger.getLogger().d(failureMessage())
+        assert(enforcement, failureMessage)
       }
     }
   }
-
-  enum class StrictLevel(val level: Int) : Comparable<StrictLevel> {
-    /** Do not check for violations. */
-    NONE(0),
-    /** Log violations as warnings. */
-    WARN(1),
-    /** Throw an exception on violation. */
-    THROW(2),
-    /** Kill the process on violation. Useful for debugging. */
-    ASSERT(3),
-  }
 }