diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java index c38dae62c55..7eaacded569 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java @@ -40,7 +40,7 @@ import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider; import com.google.firebase.crashlytics.internal.NativeSessionFileProvider; import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -63,8 +63,8 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase { private static final String GOOGLE_APP_ID = "google:app:id"; private static final String SESSION_ID = "session_id"; - private final CrashlyticsWorker commonWorker = - new CrashlyticsWorker(TestOnlyExecutors.background()); + private final CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); private Context testContext; private IdManager idManager; @@ -74,8 +74,6 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase { private DataCollectionArbiter mockDataCollectionArbiter; private CrashlyticsNativeComponent mockNativeComponent = mock(CrashlyticsNativeComponent.class); - private CrashlyticsWorker diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); - @Override protected void setUp() throws Exception { super.setUp(); @@ -108,7 +106,7 @@ protected void setUp() throws Exception { @Override protected void tearDown() throws Exception { super.tearDown(); - commonWorker.await(); + crashlyticsWorkers.common.await(); } /** A convenience class for building CrashlyticsController instances for testing. */ @@ -177,7 +175,6 @@ public CrashlyticsController build() { final CrashlyticsController controller = new CrashlyticsController( testContext.getApplicationContext(), - commonWorker, idManager, dataCollectionArbiter, testFileStore, @@ -189,7 +186,7 @@ public CrashlyticsController build() { nativeComponent, analyticsEventLogger, mock(CrashlyticsAppQualitySessionsSubscriber.class), - diskWriteWorker); + crashlyticsWorkers); return controller; } } @@ -218,7 +215,7 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal() controller.writeNonFatalException(thread, nonFatal); controller.doCloseSessions(testSettingsProvider); - commonWorker.await(); + crashlyticsWorkers.common.await(); verify(mockSessionReportingCoordinator) .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong()); @@ -257,7 +254,7 @@ public void testOnDemandFatal_callLogFatalException() throws Exception { controller.enableExceptionHandling(SESSION_ID, exceptionHandler, testSettingsProvider); controller.logFatalException(thread, fatal); - commonWorker.await(); + crashlyticsWorkers.common.await(); verify(mockUserMetadata).setNewSession(not(eq(SESSION_ID))); } @@ -336,8 +333,8 @@ public File getOsFile() { final CrashlyticsController controller = builder().setNativeComponent(mockNativeComponent).setLogFileManager(logFileManager).build(); - commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider)); - commonWorker.await(); + crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider)); + crashlyticsWorkers.common.await(); verify(mockSessionReportingCoordinator) .finalizeSessionWithNativeEvent(eq(previousSessionId), any(), any()); @@ -348,8 +345,8 @@ public File getOsFile() { @SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo public void testMissingNativeComponentCausesNoReports() throws Exception { final CrashlyticsController controller = createController(); - commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider)); - commonWorker.await(); + crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider)); + crashlyticsWorkers.common.await(); List sessions = testFileStore.getAllOpenSessionIds(); for (String sessionId : sessions) { @@ -385,8 +382,9 @@ public void testLogStringAfterCrashOk() throws Exception { testSettingsProvider, Thread.currentThread(), new RuntimeException()); // This should not throw. - diskWriteWorker.submit(() -> controller.writeToLog(System.currentTimeMillis(), "Hi")); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.submit( + () -> controller.writeToLog(System.currentTimeMillis(), "Hi")); + crashlyticsWorkers.diskWrite.await(); } /** @@ -401,8 +399,8 @@ public void testFinalizeSessionAfterCrashOk() throws Exception { testSettingsProvider, Thread.currentThread(), new RuntimeException()); // This should not throw. - commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider)); - commonWorker.await(); + crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider)); + crashlyticsWorkers.common.await(); } @SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo @@ -445,7 +443,7 @@ public void testUploadDisabledThenOptIn() throws Exception { final DataCollectionArbiter arbiter = mock(DataCollectionArbiter.class); when(arbiter.isAutomaticDataCollectionEnabled()).thenReturn(false); - when(arbiter.waitForDataCollectionPermission(any(Executor.class))) + when(arbiter.waitForDataCollectionPermission()) .thenReturn(new TaskCompletionSource().getTask()); when(arbiter.waitForAutomaticDataCollectionEnabled()) .thenReturn(new TaskCompletionSource().getTask()); @@ -566,14 +564,14 @@ public void testFatalEvent_sendsAppExceptionEvent() throws Exception { when(mockSessionReportingCoordinator.listSortedOpenSessionIds()) .thenReturn(new TreeSet<>(Collections.singleton(sessionId))); - commonWorker.submit( + crashlyticsWorkers.common.submit( () -> { controller.openSession(SESSION_ID); controller.handleUncaughtException( testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal")); controller.finalizeSessions(testSettingsProvider); }); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertFirebaseAnalyticsCrashEvent(mockFirebaseAnalyticsLogger); } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java index efed21540cd..1bdb3fc183d 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java @@ -35,7 +35,7 @@ import com.google.firebase.crashlytics.internal.RemoteConfigDeferredProxy; import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger; import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.persistence.FileStore; import com.google.firebase.crashlytics.internal.settings.Settings; import com.google.firebase.crashlytics.internal.settings.SettingsController; @@ -91,8 +91,7 @@ private static final class CoreBuilder { private CrashlyticsNativeComponent nativeComponent; private DataCollectionArbiter arbiter; private FileStore fileStore; - private CrashlyticsWorker commonWorker; - private CrashlyticsWorker diskWriteWorker; + private CrashlyticsWorkers crashlyticsWorkers; public CoreBuilder(Context context, FirebaseOptions firebaseOptions) { app = mock(FirebaseApp.class); @@ -121,8 +120,8 @@ public void whenAvailable( arbiter = mock(DataCollectionArbiter.class); when(arbiter.isAutomaticDataCollectionEnabled()).thenReturn(true); - commonWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); - diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); + crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); fileStore = new FileStore(context); } @@ -147,8 +146,7 @@ public CrashlyticsCore build() { fileStore, mock(CrashlyticsAppQualitySessionsSubscriber.class), mock(RemoteConfigDeferredProxy.class), - commonWorker, - diskWriteWorker); + crashlyticsWorkers); } } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java index 3922237df4c..728757d30e8 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java @@ -39,7 +39,7 @@ import com.google.firebase.crashlytics.internal.breadcrumbs.BreadcrumbHandler; import com.google.firebase.crashlytics.internal.breadcrumbs.BreadcrumbSource; import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.persistence.FileStore; import com.google.firebase.crashlytics.internal.settings.Settings; @@ -70,10 +70,8 @@ public void whenAvailable( private CrashlyticsCore crashlyticsCore; private BreadcrumbSource mockBreadcrumbSource; - private static final CrashlyticsWorker commonWorker = - new CrashlyticsWorker(TestOnlyExecutors.background()); - private static final CrashlyticsWorker diskWriteWorker = - new CrashlyticsWorker(TestOnlyExecutors.background()); + private static final CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); @Override protected void setUp() throws Exception { @@ -97,7 +95,7 @@ public void testCustomAttributes() throws Exception { final String id = "id012345"; crashlyticsCore.setUserId(id); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(id, metadata.getUserId()); final StringBuffer idBuffer = new StringBuffer(id); @@ -108,13 +106,13 @@ public void testCustomAttributes() throws Exception { final String superLongId = longId + "more chars"; crashlyticsCore.setUserId(superLongId); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(longId, metadata.getUserId()); final String key1 = "key1"; final String value1 = "value1"; crashlyticsCore.setCustomKey(key1, value1); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(value1, metadata.getCustomKeys().get(key1)); // Adding an existing key with the same value should return false @@ -128,7 +126,7 @@ public void testCustomAttributes() throws Exception { // test truncation of custom keys and attributes crashlyticsCore.setCustomKey(superLongId, superLongValue); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertNull(metadata.getCustomKeys().get(superLongId)); assertEquals(longValue, metadata.getCustomKeys().get(longId)); @@ -137,28 +135,28 @@ public void testCustomAttributes() throws Exception { final String key = "key" + i; final String value = "value" + i; crashlyticsCore.setCustomKey(key, value); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(value, metadata.getCustomKeys().get(key)); } // should be full now, extra key, value pairs will be dropped. final String key = "new key"; crashlyticsCore.setCustomKey(key, "some value"); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertFalse(metadata.getCustomKeys().containsKey(key)); // should be able to update existing keys crashlyticsCore.setCustomKey(key1, longValue); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(longValue, metadata.getCustomKeys().get(key1)); // when we set a key to null, it should still exist with an empty value crashlyticsCore.setCustomKey(key1, null); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals("", metadata.getCustomKeys().get(key1)); // keys and values are trimmed. crashlyticsCore.setCustomKey(" " + key1 + " ", " " + longValue + " "); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertTrue(metadata.getCustomKeys().containsKey(key1)); assertEquals(longValue, metadata.getCustomKeys().get(key1)); } @@ -209,7 +207,7 @@ public void testBulkCustomKeys() throws Exception { keysAndValues.put(intKey, String.valueOf(intValue)); crashlyticsCore.setCustomKeys(keysAndValues); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(stringValue, metadata.getCustomKeys().get(stringKey)); assertEquals(trimmedValue, metadata.getCustomKeys().get(trimmedKey)); @@ -230,7 +228,7 @@ public void testBulkCustomKeys() throws Exception { addlKeysAndValues.put(key, value); } crashlyticsCore.setCustomKeys(addlKeysAndValues); - commonWorker.await(); + crashlyticsWorkers.common.await(); // Ensure all keys have been set assertEquals(UserMetadata.MAX_ATTRIBUTES, metadata.getCustomKeys().size(), DELTA); @@ -248,7 +246,7 @@ public void testBulkCustomKeys() throws Exception { extraKeysAndValues.put(key, value); } crashlyticsCore.setCustomKeys(extraKeysAndValues); - commonWorker.await(); + crashlyticsWorkers.common.await(); // Make sure the extra keys were not added for (int i = UserMetadata.MAX_ATTRIBUTES; i < UserMetadata.MAX_ATTRIBUTES + 10; ++i) { @@ -274,7 +272,7 @@ public void testBulkCustomKeys() throws Exception { updatedKeysAndValues.put(intKey, String.valueOf(updatedIntValue)); crashlyticsCore.setCustomKeys(updatedKeysAndValues); - commonWorker.await(); + crashlyticsWorkers.common.await(); assertEquals(updatedStringValue, metadata.getCustomKeys().get(stringKey)); assertFalse(Boolean.parseBoolean(metadata.getCustomKeys().get(booleanKey))); @@ -447,8 +445,7 @@ CrashlyticsCore build(Context context) { new FileStore(context), mock(CrashlyticsAppQualitySessionsSubscriber.class), mock(RemoteConfigDeferredProxy.class), - commonWorker, - diskWriteWorker); + crashlyticsWorkers); return crashlyticsCore; } } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java index e9f5bac9efc..4c42bdc8069 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java @@ -34,7 +34,7 @@ import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.concurrent.TestOnlyExecutors; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -71,7 +71,8 @@ public class SessionReportingCoordinatorTest { private SessionReportingCoordinator reportingCoordinator; - private CrashlyticsWorker diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); + private CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); @Before public void setUp() { @@ -85,7 +86,7 @@ public void setUp() { logFileManager, reportMetadata, idManager, - diskWriteWorker); + crashlyticsWorkers); } @Test @@ -132,7 +133,7 @@ public void testNonFatalEvent_persistsNormalPriorityEventWithoutAllThreadsForSes reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); final boolean expectedAllThreads = false; final boolean expectedHighPriority = false; @@ -157,7 +158,7 @@ public void testNonFatalEvent_addsLogsToEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockEventBuilder) .setLog(CrashlyticsReport.Session.Event.Log.builder().setContent(testLog).build()); @@ -178,7 +179,7 @@ public void testNonFatalEvent_addsNoLogsToEventWhenNoneAvailable() throws Except reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockEventBuilder, never()).setLog(any(CrashlyticsReport.Session.Event.Log.class)); verify(mockEventBuilder).build(); @@ -255,7 +256,7 @@ public void testNonFatalEvent_addsSortedKeysToEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockEventAppBuilder).setCustomAttributes(expectedCustomAttributes); verify(mockEventAppBuilder).setInternalKeys(expectedCustomAttributes); @@ -280,7 +281,7 @@ public void testNonFatalEvent_addsNoKeysToEventWhenNoneAvailable() throws Except reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockEventAppBuilder, never()).setCustomAttributes(anyList()); verify(mockEventAppBuilder, never()).build(); @@ -303,7 +304,7 @@ public void testNonFatalEvent_addRolloutsEvent() throws Exception { reportingCoordinator.onBeginSession(sessionId, timestamp); reportingCoordinator.persistNonFatalEvent(mockException, mockThread, sessionId, timestamp); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockEventAppBuilder, never()).setCustomAttributes(anyList()); verify(mockEventAppBuilder, never()).build(); diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/ConcurrencyTesting.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/ConcurrencyTesting.java index 236dd19f995..bb086f920cd 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/ConcurrencyTesting.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/ConcurrencyTesting.java @@ -41,11 +41,5 @@ static void sleep(long millis) { } } - /** Helps to de-flake a test. */ - static void deflake() { - // An easy, but ugly, way to fix a flaky test. - sleep(1); - } - private ConcurrencyTesting() {} } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsTasksTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsTasksTest.java index 6e172051630..f789172d39a 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsTasksTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsTasksTest.java @@ -40,14 +40,14 @@ public void raceReturnsFirstResult() throws Exception { new CrashlyticsWorker(TestOnlyExecutors.background()) .submit( () -> { - sleep(200); + sleep(20); return "first"; }); Task task2 = new CrashlyticsWorker(TestOnlyExecutors.background()) .submit( () -> { - sleep(400); + sleep(40); return "slow"; }); @@ -64,14 +64,14 @@ public void raceReturnsFirstException() { new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(200); + sleep(20); return Tasks.forException(new ArithmeticException()); }); Task task2 = new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(400); + sleep(40); return Tasks.forException(new IllegalStateException()); }); @@ -89,14 +89,14 @@ public void raceFirstCancelsReturnsSecondResult() throws Exception { new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(200); + sleep(20); return Tasks.forCanceled(); }); Task task2 = new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(400); + sleep(40); return Tasks.forResult("I am slow but didn't cancel."); }); @@ -113,14 +113,14 @@ public void raceBothCancel() { new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(200); + sleep(20); return Tasks.forCanceled(); }); Task task2 = new CrashlyticsWorker(TestOnlyExecutors.background()) .submitTask( () -> { - sleep(400); + sleep(40); return Tasks.forCanceled(); }); @@ -189,7 +189,7 @@ public void raceTaskOneOnWorkerAnotherOtherThatCompletesFirst() throws Exception // Add a decoy task to the worker to take up some time. worker.submitTask( () -> { - sleep(200); + sleep(20); return Tasks.forResult(null); }); @@ -216,7 +216,7 @@ public void raceNoExecutor() throws Exception { // Set a task result from another thread. new Thread( () -> { - sleep(300); + sleep(30); task1.trySetResult("yes"); }) .start(); diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkerTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkerTest.java index a06cf1b9129..96c78bb4abc 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkerTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkerTest.java @@ -17,21 +17,24 @@ package com.google.firebase.crashlytics.internal.concurrency; import static com.google.common.truth.Truth.assertThat; -import static com.google.firebase.crashlytics.internal.concurrency.ConcurrencyTesting.deflake; import static com.google.firebase.crashlytics.internal.concurrency.ConcurrencyTesting.getThreadName; import static com.google.firebase.crashlytics.internal.concurrency.ConcurrencyTesting.newNamedSingleThreadExecutor; import static com.google.firebase.crashlytics.internal.concurrency.ConcurrencyTesting.sleep; import static org.junit.Assert.assertThrows; +import com.google.android.gms.tasks.CancellationToken; +import com.google.android.gms.tasks.CancellationTokenSource; 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.concurrent.TestOnlyExecutors; import java.io.IOException; -import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; -import java.util.List; +import java.util.Queue; import java.util.Set; import java.util.concurrent.CancellationException; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -59,7 +62,7 @@ public void tearDown() throws Exception { @Test public void executesTasksOnThreadPool() throws Exception { - Set threads = new HashSet<>(); + Set threads = Collections.synchronizedSet(new HashSet<>()); // Find thread names by adding the names we touch to the set. for (int i = 0; i < 100; i++) { @@ -79,7 +82,7 @@ public void executesTasksOnThreadPool() throws Exception { @Test public void executesTasksInOrder() throws Exception { - List list = new ArrayList<>(); + Queue list = new ConcurrentLinkedQueue<>(); // Add sequential numbers to the list to validate tasks execute in order. for (int i = 0; i < 100; i++) { @@ -95,7 +98,7 @@ public void executesTasksInOrder() throws Exception { @Test public void executesTasksSequentially() throws Exception { - List list = new ArrayList<>(); + Queue list = new ConcurrentLinkedQueue<>(); AtomicBoolean reentrant = new AtomicBoolean(false); for (int i = 0; i < 100; i++) { @@ -390,7 +393,7 @@ public void submitTaskFromAnotherWorkerDoesNotUseLocalThreads() throws Exception Task otherTask = crashlyticsWorker.submit( () -> { - sleep(300); + sleep(30); return localExecutor.getActiveCount(); }); @@ -402,14 +405,12 @@ public void submitTaskFromAnotherWorkerDoesNotUseLocalThreads() throws Exception // 0 active local threads when waiting for other task. // Waiting for a task from another worker does not block a local thread. - deflake(); assertThat(Tasks.await(localWorker.submitTask(() -> otherTask))).isEqualTo(0); // 1 active thread when doing a task. assertThat(Tasks.await(localWorker.submit(localExecutor::getActiveCount))).isEqualTo(1); // No active threads after. - deflake(); assertThat(localExecutor.getActiveCount()).isEqualTo(0); } @@ -417,7 +418,7 @@ public void submitTaskFromAnotherWorkerDoesNotUseLocalThreads() throws Exception public void submitTaskWhenThreadPoolFull() { // Fill the underlying executor thread pool. for (int i = 0; i < 10; i++) { - crashlyticsWorker.getExecutor().execute(() -> sleep(40)); + crashlyticsWorker.execute(() -> sleep(40)); } Task task = crashlyticsWorker.submitTask(() -> Tasks.forResult(42)); @@ -486,10 +487,91 @@ public void submitTaskWithContinuationThatCancels() throws Exception { assertThat(Tasks.await(crashlyticsWorker.submit(() -> "jk"))).isEqualTo("jk"); } + @Test + public void submitTaskOnSuccess() throws Exception { + TaskCompletionSource waitingSource = new TaskCompletionSource<>(); + Task waitingTask = waitingSource.getTask(); + + Task task = + crashlyticsWorker.submitTaskOnSuccess( + () -> waitingTask, + integerResult -> { + // This gets called with the result when the waiting task resolves successfully. + return Tasks.forResult(integerResult + " Success!"); + }); + + waitingSource.trySetResult(1337); + + String result = Tasks.await(task); + + assertThat(result).isEqualTo("1337 Success!"); + } + + @Test + public void submitTaskThatReturnsWithSuccessContinuation() throws Exception { + Task task = + crashlyticsWorker.submitTaskOnSuccess( + () -> Tasks.forResult(1337), integer -> Tasks.forResult(Integer.toString(integer))); + + String result = Tasks.await(task); + + assertThat(result).isEqualTo("1337"); + } + + @Test + public void submitTaskThatThrowsWithSuccessContinuation() { + Task task = + crashlyticsWorker.submitTaskOnSuccess( + () -> Tasks.forException(new IndexOutOfBoundsException()), + object -> Tasks.forResult("Still you don't believe.")); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + + assertThat(thrown).hasCauseThat().isInstanceOf(IndexOutOfBoundsException.class); + } + + @Test + public void submitTaskWithSuccessContinuationThatThrows() throws Exception { + Task task = + crashlyticsWorker.submitTaskOnSuccess( + () -> Tasks.forResult(7), integer -> Tasks.forException(new IOException())); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + + assertThat(thrown).hasCauseThat().isInstanceOf(IOException.class); + + // Verify the worker still executes tasks after the success continuation threw. + assertThat(Tasks.await(crashlyticsWorker.submit(() -> 42))).isEqualTo(42); + } + + @Test + public void submitTaskThatCancelsWithSuccessContinuation() throws Exception { + Task task = + crashlyticsWorker.submitTaskOnSuccess( + Tasks::forCanceled, object -> Tasks.forResult("Will set you free")); + + assertThrows(CancellationException.class, () -> Tasks.await(task)); + + // Verify the worker still executes tasks after the task cancelled. + assertThat(Tasks.await(crashlyticsWorker.submit(() -> 42))).isEqualTo(42); + } + + @Test + public void submitTaskWithSuccessContinuationThatCancels() throws Exception { + Task task = + crashlyticsWorker.submitTaskOnSuccess( + () -> Tasks.forResult(7), integer -> Tasks.forCanceled()); + + assertThrows(CancellationException.class, () -> Tasks.await(task)); + + // Verify the worker still executes tasks after the success continuation was cancelled. + assertThat(Tasks.await(crashlyticsWorker.submit(() -> "jk"))).isEqualTo("jk"); + } + @Test public void submitTaskWithContinuationExecutesInOrder() throws Exception { // The integers added to the list represent the order they should be executed in. - List list = new ArrayList<>(); + Queue list = new ConcurrentLinkedQueue<>(); // Start the chain which adds 1, then kicks off tasks to add 6 & 7 later, but adds 2 before // executing the newly added tasks in the continuation. @@ -497,32 +579,46 @@ public void submitTaskWithContinuationExecutesInOrder() throws Exception { () -> { list.add(1); - // Sleep to give time for the tasks 3, 4, 5 to be submitted. - sleep(300); + // Sleep to give time for the tasks 3, 4, 5, 6 to be submitted. + sleep(3); - // We added the 1 and will add 2 in the continuation. And 3, 4, 5 have been submitted. - crashlyticsWorker.submit(() -> list.add(6)); + // We added the 1 and will add 2 in the continuation. And 3, 4, 5, 6 have been submitted. crashlyticsWorker.submit(() -> list.add(7)); + crashlyticsWorker.submit(() -> list.add(8)); return Tasks.forResult(1); }, task -> { - // When the task 1 completes the next number to add is 2. Because all the other tasks are - // just submitted, not executed yet. + // When the task 1 completes the next number to add is 2. Because all the other tasks + // are just submitted, not executed yet. list.add(2); return Tasks.forResult("a"); }); - // Submit tasks to add 3, 4, 5 since we just added 1 and know a continuation will add the 2. + // Submit task to add 3 since we just added 1 and know a continuation will add the 2. crashlyticsWorker.submit(() -> list.add(3)); - crashlyticsWorker.submit(() -> list.add(4)); - crashlyticsWorker.submit(() -> list.add(5)); + + // Submit a waiting task that blocks adding 4 and 5 so we can kick it off later. + TaskCompletionSource waitingSource = new TaskCompletionSource<>(); + Task waitingTask = waitingSource.getTask(); + + crashlyticsWorker.submitTask( + () -> + waitingTask + .continueWith(crashlyticsWorker, task -> list.add(4)) + .continueWith(crashlyticsWorker, task -> list.add(5))); + + // Submit task to add 6 after the waiting continuations to add 4, 5. + crashlyticsWorker.submit(() -> list.add(6)); + + // Kick off the waiting task to add 4, 5 now that 6 is queued up. + waitingSource.trySetResult(null); crashlyticsWorker.await(); // Verify the list is complete and in order. assertThat(list).isInOrder(); - assertThat(list).hasSize(7); + assertThat(list).hasSize(8); } @Test @@ -581,4 +677,242 @@ public void tasksRunOnCorrectThreads() throws Exception { // Await on the worker to force all the tasks to run their assertions. worker.await(); } + + @Test + public void executeContinuationOnWorker() throws Exception { + Task task = + crashlyticsWorker + .submit(() -> "Hello!") + .continueWith(crashlyticsWorker, greetingTask -> getThreadName()); + + String result = Tasks.await(task); + assertThat(result).contains("Firebase Background Thread"); + } + + @Test + public void executeContinuationInsideWorker() throws Exception { + Task task = + crashlyticsWorker.submitTask( + () -> + Tasks.forResult("Hello!") + .continueWith(crashlyticsWorker, greetingTask -> getThreadName())); + + String result = Tasks.await(task); + assertThat(result).contains("Firebase Background Thread"); + } + + @Test + public void executeSuccessContinuationOnWorker() throws Exception { + Task task = + crashlyticsWorker + .submit(() -> "Ahoy-hoy!") + .onSuccessTask(crashlyticsWorker, greeting -> Tasks.forResult(getThreadName())); + + String result = Tasks.await(task); + assertThat(result).contains("Firebase Background Thread"); + } + + @Test + public void executeSuccessContinuationInsideWorker() throws Exception { + Task task = + crashlyticsWorker.submitTask( + () -> + Tasks.forResult("Ahoy-hoy!") + .onSuccessTask( + crashlyticsWorker, greeting -> Tasks.forResult(getThreadName()))); + + String result = Tasks.await(task); + assertThat(result).contains("Firebase Background Thread"); + } + + @Test + public void executeSuccessContinuationOnExceptionOnWorker() throws Exception { + Task task = + crashlyticsWorker + .submitTask(() -> Tasks.forException(new IllegalStateException())) + .onSuccessTask(crashlyticsWorker, greeting -> Tasks.forResult(getThreadName())); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + assertThat(thrown).hasCauseThat().isInstanceOf(IllegalStateException.class); + + // Verify the chain did not break by adding the on success to a failed task. + assertThat(Tasks.await(crashlyticsWorker.submitTask(() -> Tasks.forResult(7)))).isEqualTo(7); + } + + @Test + public void executeSuccessContinuationOnExceptionInsideWorker() throws Exception { + Task task = + crashlyticsWorker.submitTask( + () -> + Tasks.forException(new IllegalStateException()) + .onSuccessTask( + crashlyticsWorker, greeting -> Tasks.forResult(getThreadName()))); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + assertThat(thrown).hasCauseThat().isInstanceOf(IllegalStateException.class); + + // Verify the chain did not break by adding the on success to a failed task. + assertThat(Tasks.await(crashlyticsWorker.submitTask(() -> Tasks.forResult(7)))).isEqualTo(7); + } + + @Test + public void executeContinuationThatThrowsOnWorker() { + Task task = + crashlyticsWorker + .submit(() -> "Aloha") + .continueWithTask( + crashlyticsWorker, greeting -> Tasks.forException(new ArithmeticException())); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + assertThat(thrown).hasCauseThat().isInstanceOf(ArithmeticException.class); + } + + @Test + public void executeContinuationThatThrowsInsideWorker() { + Task task = + crashlyticsWorker.submitTask( + () -> + Tasks.forResult("Aloha") + .continueWithTask( + crashlyticsWorker, + greeting -> Tasks.forException(new ArithmeticException()))); + + ExecutionException thrown = assertThrows(ExecutionException.class, () -> Tasks.await(task)); + assertThat(thrown).hasCauseThat().isInstanceOf(ArithmeticException.class); + } + + @Test + public void executeContinuationThatCancelsOnWorker() throws Exception { + Task task = + crashlyticsWorker + .submit(() -> "Aloha") + .continueWithTask(crashlyticsWorker, greeting -> Tasks.forCanceled()); + + assertThrows(CancellationException.class, () -> Tasks.await(task)); + + // Verify the chain did not break by the continuation cancelling. + assertThat(Tasks.await(crashlyticsWorker.submitTask(() -> Tasks.forResult(7)))).isEqualTo(7); + } + + @Test + public void executeContinuationThatCancelsInsideWorker() throws Exception { + Task task = + crashlyticsWorker.submitTask( + () -> + Tasks.forResult("Aloha") + .continueWithTask(crashlyticsWorker, greeting -> Tasks.forCanceled())); + + assertThrows(CancellationException.class, () -> Tasks.await(task)); + + // Verify the chain did not break by the continuation cancelling. + assertThat(Tasks.await(crashlyticsWorker.submitTask(() -> Tasks.forResult(7)))).isEqualTo(7); + } + + @Test + public void executeContinuationsOnOrInsideWorkerDoesNotDeadlock() throws Exception { + // Create a single thread worker to catch potential deadlocks. + CrashlyticsWorker worker = new CrashlyticsWorker(newNamedSingleThreadExecutor("single")); + + // Create a waiting task so we can queue up stuff before executing it. + TaskCompletionSource waiting = new TaskCompletionSource<>(); + + Task task = + worker + .submitTask( + () -> waiting.getTask().continueWith(worker, greetingTask -> getThreadName())) + .continueWith(worker, insideTask -> insideTask.getResult() + "-" + getThreadName()); + + // Kick off the waiting task. + waiting.trySetResult("Howdy!"); + + String result = Tasks.await(task); + + // Verify both on and inside continuations ran on the worker's underlying executor. + assertThat(result).contains("single-single"); + } + + @Test + public void executeContinuationTasksOnOrInsideWorkerDoesNotDeadlock() throws Exception { + // Create a single thread worker to catch potential deadlocks. + CrashlyticsWorker worker = new CrashlyticsWorker(newNamedSingleThreadExecutor("single")); + + // Create a waiting task so we can queue up stuff before executing it. + TaskCompletionSource waiting = new TaskCompletionSource<>(); + + Task task = + worker + .submitTask( + () -> + waiting + .getTask() + .continueWithTask(worker, greetingTask -> Tasks.forResult(getThreadName()))) + .continueWithTask( + worker, + insideTask -> Tasks.forResult(insideTask.getResult() + "-" + getThreadName())); + + // Kick off the waiting task. + waiting.trySetResult("Howdy!"); + + String result = Tasks.await(task); + + // Verify both on and inside continuations ran on the worker's underlying executor. + assertThat(result).contains("single-single"); + } + + @Test + public void cancelledTaskInMiddleDoesNotBreakChain() throws Exception { + // List to keep track of tasks that successfully executed. + Queue list = new ConcurrentLinkedQueue<>(); + + // Create a waiting task to block execution on the worker until more tasks are queued up. + TaskCompletionSource taskSource = new TaskCompletionSource<>(); + crashlyticsWorker.submitTask(taskSource::getTask); + + // Setup a waiting cancellation, so the task does not know it's cancelled when submitting more. + CancellationTokenSource cancellationSource = new CancellationTokenSource(); + CancellationToken cancellationToken = cancellationSource.getToken(); + + // Submit the first task that will cancel. + crashlyticsWorker.submitTask(() -> new TaskCompletionSource<>(cancellationToken).getTask()); + + // Submit a Runnable + crashlyticsWorker.submit( + () -> { + list.add("runnable"); + }); + + // Submit another task that will cancel. + crashlyticsWorker.submitTask(() -> new TaskCompletionSource<>(cancellationToken).getTask()); + + // Submit a Callable + crashlyticsWorker.submit(() -> list.add("callable")); + + crashlyticsWorker.submitTask(() -> new TaskCompletionSource<>(cancellationToken).getTask()); + + // Submit a Callable + crashlyticsWorker.submitTask( + () -> { + list.add("callable task"); + return Tasks.forResult(null); + }); + + crashlyticsWorker.submitTask(() -> new TaskCompletionSource<>(cancellationToken).getTask()); + + // Submit a Callable with a Continuation. + crashlyticsWorker.submitTask( + () -> Tasks.forResult(null), + task -> { + list.add("continuation"); + return Tasks.forResult(null); + }); + + // Trigger the cancellations. + cancellationSource.cancel(); + taskSource.trySetResult("go!"); + + crashlyticsWorker.await(); + + // Verify that all types of tasks executed. + assertThat(list).containsExactly("runnable", "callable", "callable task", "continuation"); + } } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java index b8a0d655969..ff7f853adff 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java @@ -18,7 +18,7 @@ import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.crashlytics.internal.CrashlyticsTestCase; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.persistence.FileStore; import java.io.File; import java.io.IOException; @@ -59,7 +59,7 @@ public class MetaDataStoreTest extends CrashlyticsTestCase { private FileStore fileStore; - private CrashlyticsWorker diskWriteWorker; + private CrashlyticsWorkers crashlyticsWorkers; private MetaDataStore storeUnderTest; @Override @@ -67,7 +67,8 @@ public void setUp() throws Exception { super.setUp(); fileStore = new FileStore(getContext()); storeUnderTest = new MetaDataStore(fileStore); - diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); + crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); } @Override @@ -80,71 +81,72 @@ private UserMetadata metadataWithUserId(String sessionId) { } private UserMetadata metadataWithUserId(String sessionId, String userId) { - UserMetadata metadata = new UserMetadata(sessionId, fileStore, diskWriteWorker); + UserMetadata metadata = new UserMetadata(sessionId, fileStore, crashlyticsWorkers); metadata.setUserId(userId); return metadata; } @Test public void testWriteUserData_allFields() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData(SESSION_ID_1, metadataWithUserId(SESSION_ID_1).getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); + Thread.sleep(5); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertEquals(USER_ID, userData.getUserId()); } @Test public void testWriteUserData_noFields() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData( SESSION_ID_1, new UserMetadata(SESSION_ID_1, fileStore, null).getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); } @Test public void testWriteUserData_singleField() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData(SESSION_ID_1, metadataWithUserId(SESSION_ID_1).getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertEquals(USER_ID, userData.getUserId()); } @Test public void testWriteUserData_null() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData( SESSION_ID_1, metadataWithUserId(SESSION_ID_1, null).getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); } @Test public void testWriteUserData_emptyString() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData( SESSION_ID_1, metadataWithUserId(SESSION_ID_1, "").getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertEquals("", userData.getUserId()); } @@ -152,22 +154,22 @@ public void testWriteUserData_emptyString() throws Exception { public void testWriteUserData_unicode() throws Exception { storeUnderTest.writeUserData( SESSION_ID_1, metadataWithUserId(SESSION_ID_1, UNICODE).getUserId()); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertEquals(UNICODE, userData.getUserId()); } @Test public void testWriteUserData_escaped() throws Exception { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { storeUnderTest.writeUserData( SESSION_ID_1, metadataWithUserId(SESSION_ID_1, ESCAPED).getUserId()); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertEquals(ESCAPED.trim(), userData.getUserId()); } @@ -175,7 +177,7 @@ public void testWriteUserData_escaped() throws Exception { public void testWriteUserData_readDifferentSession() { storeUnderTest.writeUserData(SESSION_ID_1, metadataWithUserId(SESSION_ID_1).getUserId()); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_2, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_2, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); } @@ -186,7 +188,7 @@ public void testReadUserData_corruptData() throws IOException { printWriter.println("Matt says hi!"); } UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); assertFalse(file.exists()); } @@ -196,7 +198,7 @@ public void testReadUserData_emptyData() throws IOException { File file = storeUnderTest.getUserDataFileForSession(SESSION_ID_1); file.createNewFile(); UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); assertFalse(file.exists()); } @@ -204,42 +206,42 @@ public void testReadUserData_emptyData() throws IOException { @Test public void testReadUserData_noStoredData() { UserMetadata userData = - UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); assertNull(userData.getUserId()); } @Test public void testUpdateSessionId_notPersistUserIdToNewSessionIfNoUserIdSet() throws Exception { - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat( fileStore.getSessionFile(SESSION_ID_2, UserMetadata.USERDATA_FILENAME).exists()) .isFalse(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); } @Test public void testUpdateSessionId_notPersistCustomKeysToNewSessionIfNoCustomKeysSet() throws Exception { - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat(fileStore.getSessionFile(SESSION_ID_2, UserMetadata.KEYDATA_FILENAME).exists()) .isFalse(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); } @Test public void testUpdateSessionId_notPersistRolloutsToNewSessionIfNoRolloutsSet() throws Exception { - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat( fileStore @@ -247,12 +249,12 @@ public void testUpdateSessionId_notPersistRolloutsToNewSessionIfNoRolloutsSet() .exists()) .isFalse(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); } @Test public void testUpdateSessionId_persistCustomKeysToNewSessionIfCustomKeysSet() throws Exception { - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); final Map keys = new HashMap() { { @@ -263,12 +265,12 @@ public void testUpdateSessionId_persistCustomKeysToNewSessionIfCustomKeysSet() t }; userMetadata.setCustomKeys(keys); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat(fileStore.getSessionFile(SESSION_ID_2, UserMetadata.KEYDATA_FILENAME).exists()) .isTrue(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); MetaDataStore metaDataStore = new MetaDataStore(fileStore); assertThat(metaDataStore.readKeyData(SESSION_ID_2)).isEqualTo(keys); @@ -283,35 +285,35 @@ public void testSetSameKeysRaceCondition_preserveLastEntryValue() throws Excepti put(KEY_2, "20000"); } }; - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); for (int index = 0; index <= 10000; index++) { userMetadata.setCustomKey(KEY_1, String.valueOf(index)); userMetadata.setCustomKey(KEY_2, String.valueOf(index * 2)); } - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { final Map readKeys = storeUnderTest.readKeyData(SESSION_ID_1); assertThat(readKeys.get(KEY_1)).isEqualTo("10000"); assertThat(readKeys.get(KEY_2)).isEqualTo("20000"); assertEqualMaps(keys, readKeys); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); } @Test public void testUpdateSessionId_persistUserIdToNewSessionIfUserIdSet() throws Exception { String userId = "ThemisWang"; - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); userMetadata.setUserId(userId); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat( fileStore.getSessionFile(SESSION_ID_2, UserMetadata.USERDATA_FILENAME).exists()) .isTrue(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); MetaDataStore metaDataStore = new MetaDataStore(fileStore); assertThat(metaDataStore.readUserId(SESSION_ID_2)).isEqualTo(userId); @@ -319,10 +321,10 @@ public void testUpdateSessionId_persistUserIdToNewSessionIfUserIdSet() throws Ex @Test public void testUpdateSessionId_persistRolloutsToNewSessionIfRolloutsSet() throws Exception { - UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, diskWriteWorker); + UserMetadata userMetadata = new UserMetadata(SESSION_ID_1, fileStore, crashlyticsWorkers); userMetadata.updateRolloutsState(ROLLOUTS_STATE); userMetadata.setNewSession(SESSION_ID_2); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { assertThat( fileStore @@ -330,7 +332,7 @@ public void testUpdateSessionId_persistRolloutsToNewSessionIfRolloutsSet() throw .exists()) .isTrue(); }); - diskWriteWorker.await(); + crashlyticsWorkers.diskWrite.await(); MetaDataStore metaDataStore = new MetaDataStore(fileStore); assertThat(metaDataStore.readRolloutsState(SESSION_ID_2)).isEqualTo(ROLLOUTS_STATE); diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java index 8331ef90cb8..83f104d9649 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java @@ -32,9 +32,7 @@ import com.google.firebase.crashlytics.internal.common.DeliveryMechanism; import com.google.firebase.crashlytics.internal.common.InstallIdProvider; import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import java.util.concurrent.TimeUnit; import org.json.JSONObject; @@ -59,9 +57,8 @@ public class DefaultSettingsControllerTest extends CrashlyticsTestCase { private SettingsSpiCall mockSettingsSpiCall; private DataCollectionArbiter mockDataCollectionArbiter; - private final CrashlyticsWorker commonWorker = - new CrashlyticsWorker(TestOnlyExecutors.background()); - private final ExecutorService networkExecutor = TestOnlyExecutors.blocking(); + private final CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); public DefaultSettingsControllerTest() {} @@ -125,7 +122,7 @@ public void testCachedSettingsLoad() throws Exception { mockDataCollectionArbiter, false); - await(controller.loadSettingsData(commonWorker, networkExecutor)); + await(controller.loadSettingsData(crashlyticsWorkers)); assertEquals(cachedSettings, controller.getSettingsSync()); verifyNoMoreInteractions(mockSettingsSpiCall); @@ -143,7 +140,7 @@ public void testCachedSettingsLoad_newInstanceIdentifier() throws Exception { when(mockSettingsJsonParser.parseSettingsJson(fetchedJson)).thenReturn(fetchedSettings); TaskCompletionSource dataCollectionPermission = new TaskCompletionSource<>(); - when(mockDataCollectionArbiter.waitForDataCollectionPermission(any(Executor.class))) + when(mockDataCollectionArbiter.waitForDataCollectionPermission()) .thenReturn(dataCollectionPermission.getTask()); SettingsRequest requestData = buildSettingsRequest(); @@ -157,8 +154,7 @@ public void testCachedSettingsLoad_newInstanceIdentifier() throws Exception { mockDataCollectionArbiter, true); - controller.loadSettingsData( - SettingsCacheBehavior.SKIP_CACHE_LOOKUP, commonWorker, networkExecutor); + controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, crashlyticsWorkers); assertNotNull(controller.getSettingsSync()); dataCollectionPermission.trySetResult(null); @@ -188,7 +184,7 @@ public void testExpiredCachedSettingsLoad() throws Exception { when(mockSettingsJsonParser.parseSettingsJson(fetchedJson)).thenReturn(fetchedSettings); TaskCompletionSource dataCollectionPermission = new TaskCompletionSource<>(); - when(mockDataCollectionArbiter.waitForDataCollectionPermission(any(Executor.class))) + when(mockDataCollectionArbiter.waitForDataCollectionPermission()) .thenReturn(dataCollectionPermission.getTask()); SettingsRequest requestData = buildSettingsRequest(); @@ -202,7 +198,7 @@ public void testExpiredCachedSettingsLoad() throws Exception { mockDataCollectionArbiter, false); - Task loadFinished = controller.loadSettingsData(commonWorker, networkExecutor); + Task loadFinished = controller.loadSettingsData(crashlyticsWorkers); assertEquals(cachedSettings, controller.getSettingsSync()); assertEquals(cachedSettings, await(controller.getSettingsAsync())); @@ -239,8 +235,7 @@ public void testIgnoreExpiredCachedSettingsLoad() throws Exception { mockSettingsSpiCall, mockDataCollectionArbiter, false); - controller.loadSettingsData( - SettingsCacheBehavior.IGNORE_CACHE_EXPIRATION, commonWorker, networkExecutor); + controller.loadSettingsData(SettingsCacheBehavior.IGNORE_CACHE_EXPIRATION, crashlyticsWorkers); assertEquals(cachedSettings, controller.getSettingsSync()); verifyNoMoreInteractions(mockSettingsSpiCall); @@ -267,7 +262,7 @@ public void testSkipCachedSettingsLoad() throws Exception { .thenReturn(expiredCachedSettings); TaskCompletionSource dataCollectionPermission = new TaskCompletionSource<>(); - when(mockDataCollectionArbiter.waitForDataCollectionPermission(any(Executor.class))) + when(mockDataCollectionArbiter.waitForDataCollectionPermission()) .thenReturn(dataCollectionPermission.getTask()); SettingsRequest requestData = buildSettingsRequest(); @@ -282,8 +277,7 @@ public void testSkipCachedSettingsLoad() throws Exception { false); Task loadFinished = - controller.loadSettingsData( - SettingsCacheBehavior.SKIP_CACHE_LOOKUP, commonWorker, networkExecutor); + controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, crashlyticsWorkers); assertEquals(expiredCachedSettings, controller.getSettingsSync()); dataCollectionPermission.trySetResult(null); @@ -319,7 +313,7 @@ public void testLastDitchSettingsLoad() throws Exception { .thenReturn(expiredCachedSettings); TaskCompletionSource dataCollectionPermission = new TaskCompletionSource<>(); - when(mockDataCollectionArbiter.waitForDataCollectionPermission(any(Executor.class))) + when(mockDataCollectionArbiter.waitForDataCollectionPermission()) .thenReturn(dataCollectionPermission.getTask()); SettingsRequest requestData = buildSettingsRequest(); @@ -334,8 +328,7 @@ public void testLastDitchSettingsLoad() throws Exception { false); Task loadFinished = - controller.loadSettingsData( - SettingsCacheBehavior.SKIP_CACHE_LOOKUP, commonWorker, networkExecutor); + controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, crashlyticsWorkers); assertEquals(expiredCachedSettings, controller.getSettingsSync()); dataCollectionPermission.trySetResult(null); @@ -355,7 +348,7 @@ public void testNoAvailableSettingsLoad() throws Exception { when(mockCachedSettingsIo.readCachedSettings()).thenReturn(null); TaskCompletionSource dataCollectionPermission = new TaskCompletionSource<>(); - when(mockDataCollectionArbiter.waitForDataCollectionPermission(any(Executor.class))) + when(mockDataCollectionArbiter.waitForDataCollectionPermission()) .thenReturn(dataCollectionPermission.getTask()); SettingsRequest requestData = buildSettingsRequest(); @@ -369,7 +362,7 @@ public void testNoAvailableSettingsLoad() throws Exception { mockDataCollectionArbiter, false); - Task loadFinished = controller.loadSettingsData(commonWorker, networkExecutor); + Task loadFinished = controller.loadSettingsData(crashlyticsWorkers); assertNotNull(controller.getSettingsSync()); assertFalse(controller.getSettingsAsync().isComplete()); diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/CrashlyticsRegistrar.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/CrashlyticsRegistrar.java index 9051d22258f..b792c51784c 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/CrashlyticsRegistrar.java +++ b/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.CrashlyticsPreconditions.StrictLevel.ASSERT; - import com.google.firebase.FirebaseApp; import com.google.firebase.analytics.connector.AnalyticsConnector; import com.google.firebase.annotations.concurrent.Background; @@ -26,8 +24,8 @@ import com.google.firebase.components.Dependency; import com.google.firebase.components.Qualified; import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent; -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions; import com.google.firebase.crashlytics.internal.Logger; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.installations.FirebaseInstallationsApi; import com.google.firebase.platforminfo.LibraryVersionComponent; import com.google.firebase.remoteconfig.interop.FirebaseRemoteConfigInterop; @@ -69,10 +67,8 @@ public List> getComponents() { } private FirebaseCrashlytics buildCrashlytics(ComponentContainer container) { - // TODO(mrober): Make this a build time configuration. Do not release like this. - CrashlyticsPreconditions.setStrictLevel(ASSERT); // Kill the process on violation for debugging. + CrashlyticsWorkers.setEnforcement(BuildConfig.DEBUG); - // CrashlyticsPreconditions.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; diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java index d5efecceb9f..5a4888c0c33 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/FirebaseCrashlytics.java @@ -34,7 +34,7 @@ import com.google.firebase.crashlytics.internal.common.CrashlyticsCore; import com.google.firebase.crashlytics.internal.common.DataCollectionArbiter; import com.google.firebase.crashlytics.internal.common.IdManager; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.network.HttpRequestFactory; import com.google.firebase.crashlytics.internal.persistence.FileStore; import com.google.firebase.crashlytics.internal.settings.SettingsController; @@ -78,6 +78,9 @@ public class FirebaseCrashlytics { + " for " + appIdentifier); + CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(backgroundExecutorService, blockingExecutorService); + FileStore fileStore = new FileStore(context); final DataCollectionArbiter arbiter = new DataCollectionArbiter(app); final IdManager idManager = @@ -96,9 +99,6 @@ public class FirebaseCrashlytics { RemoteConfigDeferredProxy remoteConfigDeferredProxy = new RemoteConfigDeferredProxy(remoteConfigInteropDeferred); - CrashlyticsWorker commonWorker = new CrashlyticsWorker(backgroundExecutorService); - CrashlyticsWorker diskWriteWorker = new CrashlyticsWorker(backgroundExecutorService); - final CrashlyticsCore core = new CrashlyticsCore( app, @@ -110,8 +110,7 @@ public class FirebaseCrashlytics { fileStore, sessionsSubscriber, remoteConfigDeferredProxy, - commonWorker, - diskWriteWorker); + crashlyticsWorkers); final String googleAppId = app.getOptions().getApplicationId(); final String mappingFileId = CommonUtils.getMappingFileId(context); @@ -160,7 +159,7 @@ public class FirebaseCrashlytics { // Kick off actually fetching the settings. settingsController - .loadSettingsData(commonWorker, blockingExecutorService) + .loadSettingsData(crashlyticsWorkers) .addOnFailureListener(ex -> Logger.getLogger().e("Error fetching settings.", ex)); final boolean finishCoreInBackground = core.onPreExecute(appData, settingsController); diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/CrashlyticsPreconditions.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/CrashlyticsPreconditions.kt deleted file mode 100644 index d142ff1b283..00000000000 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/CrashlyticsPreconditions.kt +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Copyright 2024 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.firebase.crashlytics.internal - -import android.os.Build -import android.os.Looper -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions.StrictLevel.ASSERT -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions.StrictLevel.NONE -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions.StrictLevel.THROW -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions.StrictLevel.WARN - -/** - * Convenient preconditions specific for Crashlytics concurrency. - * - * Use GMS Core's [com.google.android.gms.common.internal.Preconditions] for general preconditions. - */ -internal object CrashlyticsPreconditions { - private val threadName - get() = Thread.currentThread().name - - // TODO(mrober): Make this a build time configuration. - @JvmStatic var strictLevel: StrictLevel = NONE - - @JvmStatic - fun checkMainThread() = - checkThread(::isMainThread) { "Must be called on the main thread, was called on $threadName." } - - @JvmStatic - fun checkBlockingThread() = - checkThread(::isBlockingThread) { - "Must be called on a blocking thread, was called on $threadName." - } - - @JvmStatic - fun checkBackgroundThread() = - checkThread(::isBackgroundThread) { - "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) - } - } - - enum class StrictLevel(val level: Int) : Comparable { - /** 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), - } -} diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsBackgroundWorker.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsBackgroundWorker.java deleted file mode 100644 index f5812bb6a4c..00000000000 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsBackgroundWorker.java +++ /dev/null @@ -1,166 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.firebase.crashlytics.internal.common; - -import androidx.annotation.NonNull; -import com.google.android.gms.tasks.Continuation; -import com.google.android.gms.tasks.Task; -import com.google.android.gms.tasks.Tasks; -import java.util.concurrent.Callable; -import java.util.concurrent.Executor; - -/** - * Helper for executing tasks on the Crashlytics background executor service. - * - *

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. - * - *

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 - * that would create a deadlock. In such a case, the worker can return a Task that depends on the - * future work, and run the future work on the executor's thread, but not put it in the queue as its - * own worker. - * - * @deprecated Use the generic CrashlyticsWorker instead. - */ -@Deprecated -public class CrashlyticsBackgroundWorker { - // TODO(mrober): Clean this up after moving everything to the generic CrashlyticsWorker. - private final Executor executor; - - private Task tail = Tasks.forResult(null); - - private final Object tailLock = new Object(); - - // A thread local to keep track of which thread belongs to this executor. - private final ThreadLocal isExecutorThread = new ThreadLocal<>(); - - public CrashlyticsBackgroundWorker(Executor executor) { - this.executor = executor; - // Queue up the first job as one that marks the thread so we can check it later. - executor.execute( - new Runnable() { - @Override - public void run() { - isExecutorThread.set(true); - } - }); - } - - /** Returns the executor used by this background worker. */ - public Executor getExecutor() { - return executor; - } - - /** Returns true if called on the thread owned by this background worker. */ - private boolean isRunningOnThread() { - return Boolean.TRUE.equals(isExecutorThread.get()); - } - - /** - * Throws an exception if called from any thread other than the background worker's. This helps - * guarantee code is being called on the intended thread. - */ - public void checkRunningOnThread() { - if (!isRunningOnThread()) { - throw new IllegalStateException("Not running on background worker thread as intended."); - } - } - - /** - * Submit a Runnable task for asynchronous execution on the Crashlytics background - * executor service. - * - *

If the runnable throws an exception, the task will be rejected with it. - * - * @return a Task which will be resolved with null upon successful completion of the - * runnable, or null if the runnable is rejected from the background executor - * service. - */ - Task submit(final Runnable runnable) { - return submit( - new Callable() { - @Override - public Void call() throws Exception { - runnable.run(); - return null; - } - }); - } - - /** Convenience method that creates a new Continuation that wraps the given callable. */ - private Continuation newContinuation(Callable callable) { - return new Continuation() { - @Override - public T then(@NonNull Task task) throws Exception { - // We can ignore the task passed in, which is just the queue's tail. - return callable.call(); - } - }; - } - - /** Convenience method that tasks a Task and convert it to a Task. */ - private Task ignoreResult(Task task) { - return task.continueWith( - executor, - new Continuation() { - @Override - public Void then(@NonNull Task task) throws Exception { - // Ignore whether the task succeeded or failed. - return null; - } - }); - } - - /** - * Submit a Callable task for asynchronous execution on the Crashlytics background - * executor service. - * - *

If the callable throws an exception, the task will be rejected with it. - * - * @return a Task which will be resolved upon successful completion of the callable. - */ - public Task submit(final Callable callable) { - synchronized (tailLock) { - // Chain the new callable onto the queue's tail. - Task toReturn = tail.continueWith(executor, newContinuation(callable)); - - // Add a new tail that swallows errors from the callable when it finishes. - tail = ignoreResult(toReturn); - return toReturn; - } - } - - /** - * Submit a Callable task for asynchronous execution on the Crashlytics background - * executor service. This method is useful for making the worker block on an asynchronous - * operation, while letting the underlying thread be re-used. - * - *

If the callable throws an exception, the task will be rejected with it. - * - * @return a Task which will be resolved upon successful completion of the Task - * returns by the callable. - */ - public Task submitTask(final Callable> callable) { - synchronized (tailLock) { - // Chain the new callable onto the queue's tail. - Task toReturn = tail.continueWithTask(executor, newContinuation(callable)); - - // Add a new tail that swallows errors from the callable when it finishes. - tail = ignoreResult(toReturn); - return toReturn; - } - } -} diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index 45b1d914632..4978baa61de 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -30,11 +30,11 @@ import com.google.android.gms.tasks.TaskCompletionSource; import com.google.android.gms.tasks.Tasks; import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent; -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions; import com.google.firebase.crashlytics.internal.Logger; import com.google.firebase.crashlytics.internal.NativeSessionFileProvider; import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsTasks; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -53,7 +53,6 @@ import java.util.Map; import java.util.SortedSet; import java.util.concurrent.Callable; -import java.util.concurrent.Executor; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeoutException; @@ -69,8 +68,6 @@ class CrashlyticsController { static final FilenameFilter APP_EXCEPTION_MARKER_FILTER = (directory, filename) -> filename.startsWith(APP_EXCEPTION_MARKER_PREFIX); - static final String NATIVE_SESSION_DIR = "native-sessions"; - static final int FIREBASE_CRASH_TYPE_FATAL = 1; private static final String GENERATOR_FORMAT = "Crashlytics Android SDK/%s"; @@ -84,9 +81,7 @@ class CrashlyticsController { private final CrashlyticsFileMarker crashMarker; private final UserMetadata userMetadata; - private final CrashlyticsWorker backgroundWorker; - - private final CrashlyticsWorker diskWriteWorker; + private final CrashlyticsWorkers crashlyticsWorkers; private final IdManager idManager; private final FileStore fileStore; @@ -120,7 +115,6 @@ class CrashlyticsController { CrashlyticsController( Context context, - CrashlyticsWorker commonWorker, IdManager idManager, DataCollectionArbiter dataCollectionArbiter, FileStore fileStore, @@ -132,9 +126,8 @@ class CrashlyticsController { CrashlyticsNativeComponent nativeComponent, AnalyticsEventLogger analyticsEventLogger, CrashlyticsAppQualitySessionsSubscriber sessionsSubscriber, - CrashlyticsWorker diskWriteWorker) { + CrashlyticsWorkers crashlyticsWorkers) { this.context = context; - this.backgroundWorker = commonWorker; this.idManager = idManager; this.dataCollectionArbiter = dataCollectionArbiter; this.fileStore = fileStore; @@ -146,11 +139,7 @@ class CrashlyticsController { this.analyticsEventLogger = analyticsEventLogger; this.sessionsSubscriber = sessionsSubscriber; this.reportingCoordinator = sessionReportingCoordinator; - this.diskWriteWorker = diskWriteWorker; - } - - private Context getContext() { - return context; + this.crashlyticsWorkers = crashlyticsWorkers; } // region Exception handling @@ -201,7 +190,7 @@ synchronized void handleUncaughtException( final long timestampMillis = System.currentTimeMillis(); final Task handleUncaughtExceptionTask = - backgroundWorker.submitTask( + crashlyticsWorkers.common.submitTask( new Callable>() { @Override public Task call() throws Exception { @@ -230,12 +219,10 @@ public Task call() throws Exception { return Tasks.forResult(null); } - Executor executor = backgroundWorker.getExecutor(); - return settingsProvider .getSettingsAsync() .onSuccessTask( - executor, + crashlyticsWorkers.common, new SuccessContinuation() { @NonNull @Override @@ -250,7 +237,8 @@ public Task then(@Nullable Settings settings) throws Exception { return Tasks.whenAll( logAnalyticsAppExceptionEvents(), reportingCoordinator.sendReports( - executor, isOnDemand ? currentSessionId : null)); + crashlyticsWorkers.common, + isOnDemand ? currentSessionId : null)); } }); } @@ -309,12 +297,12 @@ public Task then(@Nullable Void aVoid) throws Exception { Logger.getLogger().d("Waiting for send/deleteUnsentReports to be called."); // Wait for either the processReports callback to be called, or data collection to be enabled. - return Utils.race(collectionEnabled, reportActionProvided.getTask()); + return CrashlyticsTasks.race(collectionEnabled, reportActionProvided.getTask()); } /** This function must be called before opening the first session * */ boolean didCrashOnPreviousExecution() { - CrashlyticsPreconditions.checkBackgroundThread(); // To not violate strict mode. + CrashlyticsWorkers.checkBackgroundThread(); // To not violate strict mode. if (!crashMarker.isPresent()) { // Before the first session of this execution is opened, the current session ID still refers // to the previous execution's last session, which is what we want. @@ -369,7 +357,7 @@ Task submitAllReports(Task settingsDataTask) { @Override public Task then(@Nullable Boolean send) throws Exception { - return backgroundWorker.submitTask( + return crashlyticsWorkers.common.submitTask( new Callable>() { @Override public Task call() throws Exception { @@ -390,10 +378,8 @@ public Task call() throws Exception { // permission. dataCollectionArbiter.grantDataCollectionPermission(dataCollectionToken); - Executor executor = backgroundWorker.getExecutor(); - return settingsDataTask.onSuccessTask( - executor, + crashlyticsWorkers.common, new SuccessContinuation() { @NonNull @Override @@ -406,7 +392,7 @@ public Task then(@Nullable Settings appSettingsData) return Tasks.forResult(null); } logAnalyticsAppExceptionEvents(); - reportingCoordinator.sendReports(executor); + reportingCoordinator.sendReports(crashlyticsWorkers.common); unsentReportsHandled.trySetResult(null); return Tasks.forResult(null); @@ -490,14 +476,8 @@ void setInternalKey(String key, String value) { /** Open a new session on the single-threaded executor. */ void openSession(String sessionIdentifier) { - backgroundWorker.submit( - new Callable() { - @Override - public Void call() throws Exception { - doOpenSession(sessionIdentifier, /* isOnDemand= */ false); - return null; - } - }); + crashlyticsWorkers.common.submit( + () -> doOpenSession(sessionIdentifier, /* isOnDemand= */ false)); } /** @@ -523,7 +503,7 @@ private String getCurrentSessionId() { * @param settingsProvider */ boolean finalizeSessions(SettingsProvider settingsProvider) { - CrashlyticsPreconditions.checkBackgroundThread(); + CrashlyticsWorkers.checkBackgroundThread(); if (isHandlingException()) { Logger.getLogger().w("Skipping session finalization because a crash has already occurred."); @@ -930,7 +910,7 @@ private void writeApplicationExitInfoEventIfRelevant(String sessionId) { if (applicationExitInfoList.size() != 0) { final LogFileManager relevantSessionLogManager = new LogFileManager(fileStore, sessionId); final UserMetadata relevantUserMetadata = - UserMetadata.loadFromExistingSession(sessionId, fileStore, diskWriteWorker); + UserMetadata.loadFromExistingSession(sessionId, fileStore, crashlyticsWorkers); reportingCoordinator.persistRelevantAppExitInfoEvent( sessionId, applicationExitInfoList, relevantSessionLogManager, relevantUserMetadata); } else { diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java index 16ef34b4f74..ecfc4e17795 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCore.java @@ -26,12 +26,11 @@ import com.google.firebase.FirebaseApp; import com.google.firebase.crashlytics.BuildConfig; import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent; -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions; import com.google.firebase.crashlytics.internal.Logger; import com.google.firebase.crashlytics.internal.RemoteConfigDeferredProxy; import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; import com.google.firebase.crashlytics.internal.breadcrumbs.BreadcrumbSource; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.persistence.FileStore; @@ -99,8 +98,7 @@ public class CrashlyticsCore { private final RemoteConfigDeferredProxy remoteConfigDeferredProxy; - private final CrashlyticsWorker commonWorker; - private final CrashlyticsWorker diskWriteWorker; + private final CrashlyticsWorkers crashlyticsWorkers; // region Constructors @@ -114,8 +112,7 @@ public CrashlyticsCore( FileStore fileStore, CrashlyticsAppQualitySessionsSubscriber sessionsSubscriber, RemoteConfigDeferredProxy remoteConfigDeferredProxy, - CrashlyticsWorker commonWorker, - CrashlyticsWorker diskWriteWorker) { + CrashlyticsWorkers crashlyticsWorkers) { this.app = app; this.dataCollectionArbiter = dataCollectionArbiter; this.context = app.getApplicationContext(); @@ -126,8 +123,7 @@ public CrashlyticsCore( this.fileStore = fileStore; this.sessionsSubscriber = sessionsSubscriber; this.remoteConfigDeferredProxy = remoteConfigDeferredProxy; - this.commonWorker = commonWorker; - this.diskWriteWorker = diskWriteWorker; + this.crashlyticsWorkers = crashlyticsWorkers; startTime = System.currentTimeMillis(); onDemandCounter = new OnDemandCounter(); @@ -156,7 +152,7 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider) initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore); final UserMetadata userMetadata = - new UserMetadata(sessionIdentifier, fileStore, diskWriteWorker); + new UserMetadata(sessionIdentifier, fileStore, crashlyticsWorkers); final LogFileManager logFileManager = new LogFileManager(fileStore); final StackTraceTrimmingStrategy stackTraceTrimmingStrategy = new MiddleOutFallbackStrategy( @@ -176,12 +172,11 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider) settingsProvider, onDemandCounter, sessionsSubscriber, - diskWriteWorker); + crashlyticsWorkers); controller = new CrashlyticsController( context, - commonWorker, idManager, dataCollectionArbiter, fileStore, @@ -193,7 +188,7 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider) nativeComponent, analyticsEventLogger, sessionsSubscriber, - diskWriteWorker); + crashlyticsWorkers); // If the file is present at this point, then the previous run's initialization // did not complete, and we want to perform initialization synchronously this time. @@ -230,13 +225,13 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider) /** Performs background initialization asynchronously on the common worker. */ @CanIgnoreReturnValue public Task doBackgroundInitializationAsync(SettingsProvider settingsProvider) { - return commonWorker.submitTask(() -> doBackgroundInitialization(settingsProvider)); + return crashlyticsWorkers.common.submitTask(() -> doBackgroundInitialization(settingsProvider)); } /** Performs background initialization synchronously on the calling thread. */ @CanIgnoreReturnValue private Task doBackgroundInitialization(SettingsProvider settingsProvider) { - CrashlyticsPreconditions.checkBackgroundThread(); + CrashlyticsWorkers.checkBackgroundThread(); // create the marker for this run markInitializationStarted(); @@ -326,7 +321,8 @@ public static String getVersion() { * safe to invoke this method from the main thread. */ public void logException(@NonNull Throwable throwable) { - commonWorker.submit(() -> controller.writeNonFatalException(Thread.currentThread(), throwable)); + crashlyticsWorkers.common.submit( + () -> controller.writeNonFatalException(Thread.currentThread(), throwable)); } /** @@ -342,14 +338,14 @@ public void logException(@NonNull Throwable throwable) { public void log(final String msg) { final long timestamp = System.currentTimeMillis() - startTime; // queuing up on common worker to maintain the order - commonWorker.submit( + crashlyticsWorkers.common.submit( () -> { - diskWriteWorker.submit(() -> controller.writeToLog(timestamp, msg)); + crashlyticsWorkers.diskWrite.submit(() -> controller.writeToLog(timestamp, msg)); }); } public void setUserId(String identifier) { - commonWorker.submit(() -> controller.setUserId(identifier)); + crashlyticsWorkers.common.submit(() -> controller.setUserId(identifier)); } /** @@ -362,7 +358,7 @@ public void setUserId(String identifier) { * @throws NullPointerException if key is null. */ public void setCustomKey(String key, String value) { - commonWorker.submit(() -> controller.setCustomKey(key, value)); + crashlyticsWorkers.common.submit(() -> controller.setCustomKey(key, value)); } /** @@ -379,7 +375,7 @@ public void setCustomKey(String key, String value) { * @throws NullPointerException if any key in keysAndValues is null. */ public void setCustomKeys(Map keysAndValues) { - commonWorker.submit(() -> controller.setCustomKeys(keysAndValues)); + crashlyticsWorkers.common.submit(() -> controller.setCustomKeys(keysAndValues)); } // endregion @@ -399,7 +395,7 @@ public void setCustomKeys(Map keysAndValues) { * @throws NullPointerException if key is null. */ public void setInternalKey(String key, String value) { - commonWorker.submit(() -> controller.setInternalKey(key, value)); + crashlyticsWorkers.common.submit(() -> controller.setInternalKey(key, value)); } /** Logs a fatal Throwable on the Crashlytics servers on-demand. */ @@ -408,7 +404,7 @@ public void logFatalException(Throwable throwable) { .d("Recorded on-demand fatal events: " + onDemandCounter.getRecordedOnDemandExceptions()); Logger.getLogger() .d("Dropped on-demand fatal events: " + onDemandCounter.getDroppedOnDemandExceptions()); - commonWorker.submit( + crashlyticsWorkers.common.submit( () -> { controller.setInternalKey( ON_DEMAND_RECORDED_KEY, @@ -447,7 +443,7 @@ public void run() { }; // TODO(mrober): Refactor to Tasks. Maybe just re-use async task and awaitEvenIfOnMain? - final Future future = commonWorker.getExecutor().submit(runnable); + final Future future = crashlyticsWorkers.common.getExecutor().submit(runnable); Logger.getLogger() .d( @@ -467,7 +463,7 @@ public void run() { /** Synchronous call to mark start of initialization */ void markInitializationStarted() { - CrashlyticsPreconditions.checkBackgroundThread(); + CrashlyticsWorkers.checkBackgroundThread(); // Create the Crashlytics initialization marker file, which is used to determine // whether the app crashed before initialization could complete. @@ -477,7 +473,7 @@ void markInitializationStarted() { /** Enqueues a job to remove the Crashlytics initialization marker file */ void markInitializationComplete() { - commonWorker.submit( + crashlyticsWorkers.common.submit( () -> { try { boolean removed = initializationMarker.remove(); @@ -500,7 +496,8 @@ boolean didPreviousInitializationFail() { // region Previous crash handling private void checkForPreviousCrash() { - Task task = commonWorker.submit(() -> controller.didCrashOnPreviousExecution()); + Task task = + crashlyticsWorkers.common.submit(() -> controller.didCrashOnPreviousExecution()); Boolean result; try { diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/DataCollectionArbiter.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/DataCollectionArbiter.java index 84493f7b355..157722753fd 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/DataCollectionArbiter.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/DataCollectionArbiter.java @@ -24,7 +24,7 @@ import com.google.android.gms.tasks.TaskCompletionSource; import com.google.firebase.FirebaseApp; import com.google.firebase.crashlytics.internal.Logger; -import java.util.concurrent.Executor; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsTasks; // Determines whether automatic data collection is enabled. public class DataCollectionArbiter { @@ -129,11 +129,9 @@ public Task waitForAutomaticDataCollectionEnabled() { * Returns a task which will be resolved when either: 1) automatic data collection has been * enabled, or 2) grantDataCollectionPermission has been called. */ - public Task waitForDataCollectionPermission(Executor executor) { - return Utils.race( - executor, - dataCollectionExplicitlyApproved.getTask(), - waitForAutomaticDataCollectionEnabled()); + public Task waitForDataCollectionPermission() { + return CrashlyticsTasks.race( + dataCollectionExplicitlyApproved.getTask(), waitForAutomaticDataCollectionEnabled()); } /** diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 27a493c984f..5e35c87ed67 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -24,7 +24,7 @@ import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.firebase.crashlytics.internal.Logger; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -48,8 +48,8 @@ import java.util.concurrent.Executor; /** - * This class coordinates Crashlytics session data capture and - * persistence, as well as sending of reports to Firebase Crashlytics. + * This class coordinates Crashlytics session data capture and persistence, as well as sending of + * reports to Firebase Crashlytics. */ public class SessionReportingCoordinator { @@ -70,7 +70,7 @@ public static SessionReportingCoordinator create( SettingsProvider settingsProvider, OnDemandCounter onDemandCounter, CrashlyticsAppQualitySessionsSubscriber sessionsSubscriber, - CrashlyticsWorker diskWriteWorker) { + CrashlyticsWorkers crashlyticsWorkers) { final CrashlyticsReportDataCapture dataCapture = new CrashlyticsReportDataCapture( context, idManager, appData, stackTraceTrimmingStrategy, settingsProvider); @@ -85,7 +85,7 @@ public static SessionReportingCoordinator create( logFileManager, userMetadata, idManager, - diskWriteWorker); + crashlyticsWorkers); } private final CrashlyticsReportDataCapture dataCapture; @@ -95,7 +95,7 @@ public static SessionReportingCoordinator create( private final UserMetadata reportMetadata; private final IdManager idManager; - private final CrashlyticsWorker diskWriteWorker; + private final CrashlyticsWorkers crashlyticsWorkers; SessionReportingCoordinator( CrashlyticsReportDataCapture dataCapture, @@ -104,14 +104,14 @@ public static SessionReportingCoordinator create( LogFileManager logFileManager, UserMetadata reportMetadata, IdManager idManager, - CrashlyticsWorker diskWriteWorker) { + CrashlyticsWorkers crashlyticsWorkers) { this.dataCapture = dataCapture; this.reportPersistence = reportPersistence; this.reportsSender = reportsSender; this.logFileManager = logFileManager; this.reportMetadata = reportMetadata; this.idManager = idManager; - this.diskWriteWorker = diskWriteWorker; + this.crashlyticsWorkers = crashlyticsWorkers; } public void onBeginSession(@NonNull String sessionId, long timestampSeconds) { @@ -339,7 +339,7 @@ private void persistEvent( // Non-fatal, persistence write task we move to diskWriteWorker if (!isFatal) { - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { reportPersistence.persistEvent(finallizedEvent, sessionId, isHighPriority); }); diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/Utils.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/Utils.java index 79d43a9017c..d224cae38b5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/Utils.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/Utils.java @@ -14,15 +14,11 @@ package com.google.firebase.crashlytics.internal.common; -import android.annotation.SuppressLint; import android.os.Looper; -import com.google.android.gms.tasks.Continuation; import com.google.android.gms.tasks.Task; -import com.google.android.gms.tasks.TaskCompletionSource; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -36,46 +32,6 @@ public final class Utils { /** Timeout in milliseconds for blocking on the main thread. Be careful about ANRs. */ private static final int MAIN_TIMEOUT_MILLIS = 2_750; - /** - * @return A tasks that is resolved when either of the given tasks is resolved. - */ - // TODO(b/261014167): Use an explicit executor in continuations. - @SuppressLint("TaskMainThread") - public static Task race(Task t1, Task t2) { - final TaskCompletionSource result = new TaskCompletionSource<>(); - Continuation continuation = - task -> { - if (task.isSuccessful()) { - result.trySetResult(task.getResult()); - } else if (task.getException() != null) { - result.trySetException(task.getException()); - } - return null; - }; - t1.continueWith(continuation); - t2.continueWith(continuation); - return result.getTask(); - } - - /** - * @return A tasks that is resolved when either of the given tasks is resolved. - */ - public static Task race(Executor executor, Task t1, Task t2) { - final TaskCompletionSource result = new TaskCompletionSource<>(); - Continuation continuation = - task -> { - if (task.isSuccessful()) { - result.trySetResult(task.getResult()); - } else if (task.getException() != null) { - result.trySetException(task.getException()); - } - return null; - }; - t1.continueWith(executor, continuation); - t2.continueWith(executor, continuation); - return result.getTask(); - } - /** * Blocks until the given Task completes, and then returns the value the Task was resolved with, * if successful. If the Task fails, an exception will be thrown, wrapping the Exception of the diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorker.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorker.java index a0b5510efa7..615c52389a1 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorker.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorker.java @@ -16,13 +16,16 @@ package com.google.firebase.crashlytics.internal.concurrency; +import androidx.annotation.Discouraged; import androidx.annotation.VisibleForTesting; import com.google.android.gms.tasks.Continuation; +import com.google.android.gms.tasks.SuccessContinuation; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.Tasks; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -30,8 +33,8 @@ /** * Worker for executing tasks sequentially on the given executor service. * - *

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. + *

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. * *

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 @@ -41,13 +44,13 @@ * * @hide */ -public class CrashlyticsWorker { +public class CrashlyticsWorker implements Executor { private final ExecutorService executor; private final Object tailLock = new Object(); private Task tail = Tasks.forResult(null); - public CrashlyticsWorker(ExecutorService executor) { + CrashlyticsWorker(ExecutorService executor) { this.executor = executor; } @@ -67,12 +70,8 @@ public ExecutorService getExecutor() { @CanIgnoreReturnValue public Task submit(Callable callable) { synchronized (tailLock) { - // Do not propagate a cancellation. - if (tail.isCanceled()) { - tail = tail.continueWithTask(executor, task -> Tasks.forResult(null)); - } // Chain the new callable onto the queue's tail. - Task result = tail.continueWith(executor, task -> callable.call()); + Task result = tail.continueWithTask(executor, task -> Tasks.forResult(callable.call())); tail = result; return result; } @@ -89,17 +88,13 @@ public Task submit(Callable callable) { @CanIgnoreReturnValue public Task submit(Runnable runnable) { synchronized (tailLock) { - // Do not propagate a cancellation. - if (tail.isCanceled()) { - tail = tail.continueWithTask(executor, task -> Tasks.forResult(null)); - } // Chain the new runnable onto the queue's tail. Task result = - tail.continueWith( + tail.continueWithTask( executor, task -> { runnable.run(); - return null; + return Tasks.forResult(null); }); tail = result; return result; @@ -107,9 +102,9 @@ public Task submit(Runnable runnable) { } /** - * Submits a Callable Task for asynchronous execution on the executor. + * Submits a Callable Task for asynchronous execution on the executor. * - *

This is useful for making the worker block on an asynchronous operation, while letting the + *

This is useful for making the worker suspend on an asynchronous operation, while letting the * underlying threads be re-used. * *

Returns a Task which will be resolved upon successful completion of the Task @@ -119,7 +114,7 @@ public Task submit(Runnable runnable) { @CanIgnoreReturnValue public Task submitTask(Callable> callable) { synchronized (tailLock) { - // Chain the new callable task onto the queue's tail, regardless of cancellation. + // Chain the new callable task onto the queue's tail. Task result = tail.continueWithTask(executor, task -> callable.call()); tail = result; return result; @@ -127,8 +122,8 @@ public Task submitTask(Callable> callable) { } /** - * Submits a Callable Task followed by a Continuation for - * asynchronous execution on the executor. + * Submits a Callable Task followed by a Continuation for asynchronous + * execution on the executor. * *

This is useful for submitting a task that must be immediately followed by another task, * regardless of more tasks being submitted in parallel. For example, settings. @@ -152,7 +147,57 @@ public Task submitTask( } /** - * Blocks until all current pending tasks have completed, up to 30 seconds. Useful for testing. + * Submits a Callable Task followed by a SuccessContinuation for + * asynchronous execution on the executor. + * + *

This is useful for submitting a task that must be immediately followed by another task, only + * if it was successful, but regardless of more tasks being submitted in parallel. + * + *

Returns a Task which will be resolved upon successful completion of the Task + * returned by the callable and continued by the continuation, throws an ExecutionException + * if either task throws an exception, or throws a CancellationException if + * the task is cancelled. + */ + @CanIgnoreReturnValue + public Task submitTaskOnSuccess( + Callable> callable, SuccessContinuation successContinuation) { + synchronized (tailLock) { + // Chain the new callable task and success continuation onto the queue's tail. + Task result = + tail.continueWithTask(executor, task -> callable.call()) + .continueWithTask( + executor, + task -> { + if (task.isSuccessful()) { + return successContinuation.then(task.getResult()); + } else if (task.getException() != null) { + return Tasks.forException(task.getException()); + } else { + return Tasks.forCanceled(); + } + }); + tail = result; + return result; + } + } + + /** + * Forwards a Runnable to the underlying executor. + * + *

This is useful for passing the worker as the executor to task continuations. + * + *

This is different than {@link #submit(Runnable)}. This will not submit the runnable to the + * worker to execute in order, this will forward the runnable to the underlying executor. If you + * are calling this directly from your code, you probably want {@link #submit(Runnable)}. + */ + @Override + @Discouraged(message = "This is probably not that you want. Use {@link #submit(Runnable)}.") + public void execute(Runnable runnable) { + executor.execute(runnable); + } + + /** + * Blocks until all current pending tasks have completed, up to 30 seconds. Only for testing. * *

This is not a shutdown, this does not stop new tasks from being submitted to the queue. */ @@ -160,5 +205,8 @@ public Task submitTask( public void await() throws ExecutionException, InterruptedException, TimeoutException { // Submit an empty runnable, and await on it for 30 sec so deadlocked tests fail faster. Tasks.await(submit(() -> {}), 30, TimeUnit.SECONDS); + + // Sleep for a bit here, instead of de-flaking individual test cases. + Thread.sleep(1); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkers.kt b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkers.kt new file mode 100644 index 00000000000..dcb4cb6e7b1 --- /dev/null +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/concurrency/CrashlyticsWorkers.kt @@ -0,0 +1,85 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.crashlytics.internal.concurrency + +import com.google.firebase.crashlytics.internal.Logger +import java.util.concurrent.ExecutorService + +/** + * Container to hold the different Crashlytics workers. + * + * @suppress @hide + */ +class CrashlyticsWorkers( + backgroundExecutorService: ExecutorService, + blockingExecutorService: ExecutorService, +) { + + /** + * 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. 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. 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 suspended until permission is granted. + */ + @JvmField val dataCollect = CrashlyticsWorker(backgroundExecutorService) + + /** The network worker is for making blocking network calls. */ + @JvmField val network = CrashlyticsWorker(blockingExecutorService) + + /** Convenient preconditions specific for Crashlytics worker threads. */ + companion object { + private val threadName + get() = Thread.currentThread().name + + /** When enabled, failed preconditions will cause assertion errors for debugging. */ + @JvmStatic var enforcement: Boolean = false + + @JvmStatic + fun checkBlockingThread() = + checkThread(::isBlockingThread) { + "Must be called on a blocking thread, was called on $threadName." + } + + @JvmStatic + fun checkBackgroundThread() = + checkThread(::isBackgroundThread) { + "Must be called on a background thread, was called on $threadName." + } + + private fun isBlockingThread() = threadName.contains("Firebase Blocking Thread #") + + private fun isBackgroundThread() = threadName.contains("Firebase Background Thread #") + + private fun checkThread(isCorrectThread: () -> Boolean, failureMessage: () -> String) { + if (!isCorrectThread()) { + Logger.getLogger().d(failureMessage()) + assert(enforcement, failureMessage) + } + } + } +} diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java index 50760aab38b..36ca7fda8a5 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/UserMetadata.java @@ -18,7 +18,7 @@ import androidx.annotation.VisibleForTesting; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.firebase.crashlytics.internal.common.CommonUtils; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; import com.google.firebase.crashlytics.internal.persistence.FileStore; import java.util.List; @@ -47,7 +47,7 @@ public class UserMetadata { private final MetaDataStore metaDataStore; - @VisibleForTesting final CrashlyticsWorker diskWriteWorker; + private final CrashlyticsWorkers crashlyticsWorkers; private String sessionIdentifier; // The following references contain a marker bit, which is true if the data maintained in the @@ -69,9 +69,9 @@ public static String readUserId(String sessionId, FileStore fileStore) { } public static UserMetadata loadFromExistingSession( - String sessionId, FileStore fileStore, CrashlyticsWorker diskWriteWorker) { + String sessionId, FileStore fileStore, CrashlyticsWorkers crashlyticsWorkers) { MetaDataStore store = new MetaDataStore(fileStore); - UserMetadata metadata = new UserMetadata(sessionId, fileStore, diskWriteWorker); + UserMetadata metadata = new UserMetadata(sessionId, fileStore, crashlyticsWorkers); // We don't use the set methods in this class, because they will attempt to re-serialize the // data, which is unnecessary because we just read them from disk. metadata.customKeys.map.getReference().setKeys(store.readKeyData(sessionId, false)); @@ -82,10 +82,10 @@ public static UserMetadata loadFromExistingSession( } public UserMetadata( - String sessionIdentifier, FileStore fileStore, CrashlyticsWorker diskWriteWorker) { + String sessionIdentifier, FileStore fileStore, CrashlyticsWorkers crashlyticsWorkers) { this.sessionIdentifier = sessionIdentifier; this.metaDataStore = new MetaDataStore(fileStore); - this.diskWriteWorker = diskWriteWorker; + this.crashlyticsWorkers = crashlyticsWorkers; } /** @@ -99,7 +99,7 @@ public void setNewSession(String sessionId) { sessionIdentifier = sessionId; Map keyData = customKeys.getKeys(); List rolloutAssignments = rolloutsState.getRolloutAssignmentList(); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> { if (getUserId() != null) { metaDataStore.writeUserData(sessionId, getUserId()); @@ -132,10 +132,12 @@ public void setUserId(String identifier) { } userId.set(sanitizedNewId, true); } - diskWriteWorker.submit(this::serializeUserDataIfNeeded); + crashlyticsWorkers.diskWrite.submit(this::serializeUserDataIfNeeded); } - /** @return defensive copy of the custom keys. */ + /** + * @return defensive copy of the custom keys. + */ public Map getCustomKeys() { return customKeys.getKeys(); } @@ -158,7 +160,9 @@ public void setCustomKeys(Map keysAndValues) { customKeys.setKeys(keysAndValues); } - /** @return defensive copy of the internal keys. */ + /** + * @return defensive copy of the internal keys. + */ public Map getInternalKeys() { return internalKeys.getKeys(); } @@ -188,7 +192,7 @@ public boolean updateRolloutsState(List rolloutAssignments) { } List updatedRolloutAssignments = rolloutsState.getRolloutAssignmentList(); - diskWriteWorker.submit( + crashlyticsWorkers.diskWrite.submit( () -> metaDataStore.writeRolloutState(sessionIdentifier, updatedRolloutAssignments)); return true; } @@ -268,7 +272,7 @@ private void scheduleSerializationTaskIfNeeded() { // Don't schedule the task if there's another queued task waiting, because the already-queued // task will write the latest data. if (queuedSerializer.compareAndSet(null, newRunnable)) { - diskWriteWorker.submit(newRunnable); + crashlyticsWorkers.diskWrite.submit(newRunnable); } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/network/HttpGetRequest.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/network/HttpGetRequest.java index ed9a423b4ce..f7fcfffc060 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/network/HttpGetRequest.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/network/HttpGetRequest.java @@ -14,8 +14,8 @@ package com.google.firebase.crashlytics.internal.network; -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions; import com.google.firebase.crashlytics.internal.Logger; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -57,7 +57,7 @@ public HttpGetRequest header(Map.Entry entry) { } public HttpResponse execute() throws IOException { - CrashlyticsPreconditions.checkBlockingThread(); // Network calls must be on a blocking thread. + CrashlyticsWorkers.checkBlockingThread(); // Network calls must be on a blocking thread. InputStream stream = null; HttpsURLConnection connection = null; String body = null; diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsSpiCall.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsSpiCall.java index e54cadfc651..b22c01c7be3 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsSpiCall.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsSpiCall.java @@ -15,9 +15,9 @@ package com.google.firebase.crashlytics.internal.settings; import android.text.TextUtils; -import com.google.firebase.crashlytics.internal.CrashlyticsPreconditions; import com.google.firebase.crashlytics.internal.Logger; import com.google.firebase.crashlytics.internal.common.CrashlyticsCore; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.network.HttpGetRequest; import com.google.firebase.crashlytics.internal.network.HttpRequestFactory; import com.google.firebase.crashlytics.internal.network.HttpResponse; @@ -97,7 +97,7 @@ protected HttpGetRequest createHttpGetRequest(Map queryParams) { @Override public JSONObject invoke(SettingsRequest requestData, boolean dataCollectionToken) { - CrashlyticsPreconditions.checkBlockingThread(); + CrashlyticsWorkers.checkBlockingThread(); if (!dataCollectionToken) { throw new RuntimeException("An invalid data collection token was used."); } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/SettingsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/SettingsController.java index 7c8c715ca31..3d87b78eafe 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/SettingsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/settings/SettingsController.java @@ -30,14 +30,12 @@ import com.google.firebase.crashlytics.internal.common.DeliveryMechanism; import com.google.firebase.crashlytics.internal.common.IdManager; import com.google.firebase.crashlytics.internal.common.SystemCurrentTimeProvider; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.network.HttpRequestFactory; import com.google.firebase.crashlytics.internal.persistence.FileStore; import java.util.Locale; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicReference; -import org.json.JSONException; import org.json.JSONObject; /** Implements the logic of when to use cached settings, and when to load them from the network. */ @@ -151,9 +149,8 @@ public Settings getSettingsSync() { * * @return a task that is resolved when loading is completely finished. */ - public Task loadSettingsData( - CrashlyticsWorker commonWorker, ExecutorService networkExecutor) { - return loadSettingsData(SettingsCacheBehavior.USE_CACHE, commonWorker, networkExecutor); + public Task loadSettingsData(CrashlyticsWorkers crashlyticsWorkers) { + return loadSettingsData(SettingsCacheBehavior.USE_CACHE, crashlyticsWorkers); } /** @@ -162,9 +159,7 @@ public Task loadSettingsData( * @return a task that is resolved when loading is completely finished. */ public Task loadSettingsData( - SettingsCacheBehavior cacheBehavior, - CrashlyticsWorker commonWorker, - ExecutorService networkExecutor) { + SettingsCacheBehavior cacheBehavior, CrashlyticsWorkers crashlyticsWorkers) { // TODO: Refactor this so that it doesn't do the cache lookup twice when settings are // expired. @@ -194,9 +189,9 @@ public Task loadSettingsData( // Kick off fetching fresh settings. // TODO(mrober): Refactor to call worker directly, not expose executor. return dataCollectionArbiter - .waitForDataCollectionPermission(commonWorker.getExecutor()) + .waitForDataCollectionPermission() .onSuccessTask( - commonWorker.getExecutor(), + crashlyticsWorkers.common, new SuccessContinuation() { @NonNull @Override @@ -204,8 +199,10 @@ public Task then(@Nullable Void aVoid) throws Exception { // Waited for data collection permission, so this is safe. final boolean dataCollectionToken = true; Future settingsFuture = - networkExecutor.submit( - () -> settingsSpiCall.invoke(settingsRequest, dataCollectionToken)); + 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(); @@ -266,7 +263,7 @@ private Settings getCachedSettingsData(SettingsCacheBehavior cacheBehavior) { return toReturn; } - private void logSettings(JSONObject json, String message) throws JSONException { + private void logSettings(JSONObject json, String message) { Logger.getLogger().d(message + json.toString()); } diff --git a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java index f6541502e8a..026d5ce2280 100644 --- a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java +++ b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java @@ -33,7 +33,7 @@ import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponentDeferredProxy; import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider; import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.persistence.FileStore; @@ -166,7 +166,6 @@ private CrashlyticsController createController() { final CrashlyticsController controller = new CrashlyticsController( testContext, - new CrashlyticsWorker(TestOnlyExecutors.background()), idManager, mockDataCollectionArbiter, testFileStore, @@ -178,7 +177,7 @@ private CrashlyticsController createController() { MISSING_NATIVE_COMPONENT, mock(AnalyticsEventLogger.class), mock(CrashlyticsAppQualitySessionsSubscriber.class), - new CrashlyticsWorker(TestOnlyExecutors.background())); + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking())); controller.openSession(SESSION_ID); return controller; } diff --git a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorRobolectricTest.java b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorRobolectricTest.java index 9b267be5de7..67be4920dad 100644 --- a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorRobolectricTest.java +++ b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorRobolectricTest.java @@ -25,11 +25,11 @@ import android.app.ActivityManager; import android.app.ApplicationExitInfo; import android.content.Context; -import android.os.Build; -import androidx.annotation.RequiresApi; +import android.os.Build.VERSION_CODES; import androidx.test.core.app.ApplicationProvider; +import androidx.test.filters.SdkSuppress; import com.google.firebase.concurrent.TestOnlyExecutors; -import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker; +import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers; import com.google.firebase.crashlytics.internal.metadata.LogFileManager; import com.google.firebase.crashlytics.internal.metadata.UserMetadata; import com.google.firebase.crashlytics.internal.model.CrashlyticsReport; @@ -63,7 +63,8 @@ public class SessionReportingCoordinatorRobolectricTest { private SessionReportingCoordinator reportingCoordinator; - private CrashlyticsWorker diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background()); + private final CrashlyticsWorkers crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); @Before public void setUp() { @@ -77,7 +78,7 @@ public void setUp() { logFileManager, reportMetadata, idManager, - diskWriteWorker); + crashlyticsWorkers); mockEventInteractions(); } @@ -202,7 +203,7 @@ private List getAppExitInfoList() { return activityManager.getHistoricalProcessExitReasons(null, 0, 0); } - @RequiresApi(api = Build.VERSION_CODES.R) + @SdkSuppress(minSdkVersion = VERSION_CODES.R) private static CrashlyticsReport.ApplicationExitInfo convertApplicationExitInfo( ApplicationExitInfo applicationExitInfo) { // The ApplicationExitInfo inserted by ShadowApplicationManager does not contain an input trace,