Revert "move more jobs to disk write (#6156)"

This reverts commit db42bae.

Author: mrober

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

@@ -213,10 +213,9 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal()
         .thenReturn(new TreeSet<>(Collections.singleton(sessionId)));
 
     controller.writeNonFatalException(thread, nonFatal);
-    crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(testSettingsProvider));
+    controller.doCloseSessions(testSettingsProvider);
 
     crashlyticsWorkers.common.await();
-    crashlyticsWorkers.diskWrite.await();
 
     verify(mockSessionReportingCoordinator)
         .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong());
@@ -490,8 +489,6 @@ public void testUploadDisabledThenOptOut() throws Exception {
     await(controller.deleteUnsentReports());
     await(task);
 
-    crashlyticsWorkers.diskWrite.await();
-
     verify(mockSessionReportingCoordinator).removeAllReports();
     verifyNoMoreInteractions(mockSessionReportingCoordinator);
   }

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java

@@ -170,7 +170,6 @@ public void testWriteUserData_escaped() throws Exception {
     crashlyticsWorkers.diskWrite.await();
     UserMetadata userData =
         UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers);
-    Thread.sleep(10);
     assertEquals(ESCAPED.trim(), userData.getUserId());
   }
 

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

@@ -205,6 +205,7 @@ public Task<Void> call() throws Exception {
 
                 // We've fatally crashed, so write the marker file that indicates a crash occurred.
                 crashMarker.create();
+
                 reportingCoordinator.persistFatalEvent(
                     ex, thread, currentSessionId, timestampSeconds);
 
@@ -362,11 +363,8 @@ public Task<Void> then(@Nullable Boolean send) throws Exception {
                       public Task<Void> call() throws Exception {
                         if (!send) {
                           Logger.getLogger().v("Deleting cached crash reports...");
-                          crashlyticsWorkers.diskWrite.submit(
-                              () -> {
-                                deleteFiles(listAppExceptionMarkerFiles());
-                                reportingCoordinator.removeAllReports();
-                              });
+                          deleteFiles(listAppExceptionMarkerFiles());
+                          reportingCoordinator.removeAllReports();
                           unsentReportsHandled.trySetResult(null);
                           return Tasks.forResult(null);
                         }
@@ -514,7 +512,7 @@ boolean finalizeSessions(SettingsProvider settingsProvider) {
 
     Logger.getLogger().v("Finalizing previously open sessions.");
     try {
-      doCloseSessions(true, settingsProvider, true);
+      doCloseSessions(true, settingsProvider);
     } catch (Exception e) {
       Logger.getLogger().e("Unable to finalize previously open sessions.", e);
       return false;
@@ -559,19 +557,15 @@ private void doOpenSession(String sessionIdentifier, Boolean isOnDemand) {
     reportingCoordinator.onBeginSession(sessionIdentifier, startedAtSeconds);
   }
 
-  // This is only used for exception handler close session (we have another close session in
-  // background initialization)
   void doCloseSessions(SettingsProvider settingsProvider) {
-    doCloseSessions(false, settingsProvider, false);
+    doCloseSessions(false, settingsProvider);
   }
 
   /**
-   *
-   * Not synchronized/locked. Must be executed from the executor service runs tasks in serial order
+   * Not synchronized/locked. Must be executed from the single thread executor service used by this
+   * class.
    */
-  private void doCloseSessions(
-      boolean skipCurrentSession, SettingsProvider settingsProvider, boolean isInitProcess) {
-    CrashlyticsWorkers.checkBackgroundThread();
+  private void doCloseSessions(boolean skipCurrentSession, SettingsProvider settingsProvider) {
     final int offset = skipCurrentSession ? 1 : 0;
 
     // :TODO HW2021 this implementation can be cleaned up.
@@ -585,15 +579,13 @@ private void doCloseSessions(
 
     final String mostRecentSessionIdToClose = sortedOpenSessions.get(offset);
 
-    // We only collect ANR info for finalize report during initialization process
-    if (isInitProcess && settingsProvider.getSettingsSync().featureFlagData.collectAnrs) {
+    if (settingsProvider.getSettingsSync().featureFlagData.collectAnrs) {
       writeApplicationExitInfoEventIfRelevant(mostRecentSessionIdToClose);
     } else {
       Logger.getLogger().v("ANR feature disabled.");
     }
 
-    // We only collect native crash info for finalize report during initialization process
-    if (isInitProcess && nativeComponent.hasCrashDataForSession(mostRecentSessionIdToClose)) {
+    if (nativeComponent.hasCrashDataForSession(mostRecentSessionIdToClose)) {
       // We only finalize the current session if it's a Java crash, so only finalize native crash
       // data when we aren't including current.
       finalizePreviousNativeSession(mostRecentSessionIdToClose);

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

@@ -354,16 +354,12 @@ private boolean onReportSendComplete(@NonNull Task<CrashlyticsReportWithSessionI
       CrashlyticsReportWithSessionId report = task.getResult();
       Logger.getLogger()
           .d("Crashlytics report successfully enqueued to DataTransport: " + report.getSessionId());
-      crashlyticsWorkers.diskWrite.submit(
-          () -> {
-            File reportFile = report.getReportFile();
-            if (reportFile.delete()) {
-              Logger.getLogger().d("Deleted report file: " + reportFile.getPath());
-            } else {
-              Logger.getLogger()
-                  .w("Crashlytics could not delete report file: " + reportFile.getPath());
-            }
-          });
+      File reportFile = report.getReportFile();
+      if (reportFile.delete()) {
+        Logger.getLogger().d("Deleted report file: " + reportFile.getPath());
+      } else {
+        Logger.getLogger().w("Crashlytics could not delete report file: " + reportFile.getPath());
+      }
       return true;
     }
     Logger.getLogger()

firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java

@@ -43,7 +43,6 @@
 import com.google.firebase.crashlytics.internal.settings.SettingsProvider;
 import com.google.firebase.inject.Deferred;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.TreeSet;
@@ -66,8 +65,6 @@ public class CrashlyticsControllerRobolectricTest {
   @Mock private SessionReportingCoordinator mockSessionReportingCoordinator;
   @Mock private DataCollectionArbiter mockDataCollectionArbiter;
 
-  private CrashlyticsWorkers crashlyticsWorkers;
-
   private static final CrashlyticsNativeComponent MISSING_NATIVE_COMPONENT =
       new CrashlyticsNativeComponentDeferredProxy(
           new Deferred<CrashlyticsNativeComponent>() {
@@ -83,24 +80,17 @@ public void setUp() {
     MockitoAnnotations.openMocks(this);
     testContext = getApplicationContext();
     testFileStore = new FileStore(testContext);
-    crashlyticsWorkers =
-        new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
   }
 
   @Test
-  public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist()
-      throws Exception {
+  public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist() {
     final String sessionId = "sessionId";
     final CrashlyticsController controller = createController();
 
     when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
         .thenReturn(new TreeSet<>(Collections.singletonList(sessionId)));
     mockSettingsProvider(true, false);
-
-    crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(mockSettingsProvider));
-    // cannot use await since it check preconditions if blocking main thread
-    Thread.sleep(10);
-
+    controller.doCloseSessions(mockSettingsProvider);
     // Since we haven't added any app exit info to the shadow activity manager, there won't exist a
     // single app exit info, and so this method won't be called.
     verify(mockSessionReportingCoordinator, never())
@@ -109,8 +99,7 @@ public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntE
   }
 
   @Test
-  public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() throws Exception {
-    final String sessionIdPrevious = "sessionIdPrevious";
+  public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() {
     final String sessionId = "sessionId";
     final CrashlyticsController controller = createController();
     // Adds multiple AppExitInfos to confirm that Crashlytics loops through
@@ -120,30 +109,26 @@ public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() throw
     List<ApplicationExitInfo> testApplicationExitInfo = getApplicationExitInfoList();
 
     when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
-        .thenReturn(new TreeSet<>(Arrays.asList(sessionId, sessionIdPrevious)));
+        .thenReturn(new TreeSet<>(Collections.singletonList(sessionId)));
     mockSettingsProvider(true, false);
-    crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(mockSettingsProvider));
-    // cannot use await since it check preconditions if blocking main thread
-    Thread.sleep(100);
+    controller.doCloseSessions(mockSettingsProvider);
     verify(mockSessionReportingCoordinator)
         .persistRelevantAppExitInfoEvent(
-            eq(sessionIdPrevious),
+            eq(sessionId),
             eq(testApplicationExitInfo),
             any(LogFileManager.class),
             any(UserMetadata.class));
   }
 
   @Test
-  public void testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo() throws Exception {
+  public void testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo() {
     final String sessionId = "sessionId";
     final CrashlyticsController controller = createController();
 
     when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
         .thenReturn(new TreeSet<>(Collections.singletonList(sessionId)));
     mockSettingsProvider(false, false);
-    crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(mockSettingsProvider));
-    // cannot use await since it check preconditions if blocking main thread
-    Thread.sleep(10);
+    controller.doCloseSessions(mockSettingsProvider);
     verify(mockSessionReportingCoordinator, never())
         .persistRelevantAppExitInfoEvent(
             eq(sessionId), any(), any(LogFileManager.class), any(UserMetadata.class));
@@ -192,7 +177,7 @@ private CrashlyticsController createController() {
             MISSING_NATIVE_COMPONENT,
             mock(AnalyticsEventLogger.class),
             mock(CrashlyticsAppQualitySessionsSubscriber.class),
-            crashlyticsWorkers);
+            new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()));
     controller.openSession(SESSION_ID);
     return controller;
   }