Skip to content

Commit 198a536

Browse files
committed
Revert "Add a CrashlyticsWorkers container to manage the workers (#6178)"
This reverts commit 9535f94.
1 parent 92fce90 commit 198a536

26 files changed

+612
-740
lines changed

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

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider;
4141
import com.google.firebase.crashlytics.internal.NativeSessionFileProvider;
4242
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger;
43-
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
43+
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker;
4444
import com.google.firebase.crashlytics.internal.metadata.LogFileManager;
4545
import com.google.firebase.crashlytics.internal.metadata.UserMetadata;
4646
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
@@ -63,8 +63,8 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase {
6363
private static final String GOOGLE_APP_ID = "google:app:id";
6464
private static final String SESSION_ID = "session_id";
6565

66-
private final CrashlyticsWorkers crashlyticsWorkers =
67-
new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
66+
private final CrashlyticsWorker commonWorker =
67+
new CrashlyticsWorker(TestOnlyExecutors.background());
6868

6969
private Context testContext;
7070
private IdManager idManager;
@@ -74,6 +74,8 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase {
7474
private DataCollectionArbiter mockDataCollectionArbiter;
7575
private CrashlyticsNativeComponent mockNativeComponent = mock(CrashlyticsNativeComponent.class);
7676

77+
private CrashlyticsWorker diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background());
78+
7779
@Override
7880
protected void setUp() throws Exception {
7981
super.setUp();
@@ -106,7 +108,7 @@ protected void setUp() throws Exception {
106108
@Override
107109
protected void tearDown() throws Exception {
108110
super.tearDown();
109-
crashlyticsWorkers.common.await();
111+
commonWorker.await();
110112
}
111113

112114
/** A convenience class for building CrashlyticsController instances for testing. */
@@ -175,6 +177,7 @@ public CrashlyticsController build() {
175177
final CrashlyticsController controller =
176178
new CrashlyticsController(
177179
testContext.getApplicationContext(),
180+
commonWorker,
178181
idManager,
179182
dataCollectionArbiter,
180183
testFileStore,
@@ -186,7 +189,7 @@ public CrashlyticsController build() {
186189
nativeComponent,
187190
analyticsEventLogger,
188191
mock(CrashlyticsAppQualitySessionsSubscriber.class),
189-
crashlyticsWorkers);
192+
diskWriteWorker);
190193
return controller;
191194
}
192195
}
@@ -215,7 +218,7 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal()
215218
controller.writeNonFatalException(thread, nonFatal);
216219
controller.doCloseSessions(testSettingsProvider);
217220

218-
crashlyticsWorkers.common.await();
221+
commonWorker.await();
219222

220223
verify(mockSessionReportingCoordinator)
221224
.persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong());
@@ -254,7 +257,7 @@ public void testOnDemandFatal_callLogFatalException() throws Exception {
254257
controller.enableExceptionHandling(SESSION_ID, exceptionHandler, testSettingsProvider);
255258
controller.logFatalException(thread, fatal);
256259

257-
crashlyticsWorkers.common.await();
260+
commonWorker.await();
258261

259262
verify(mockUserMetadata).setNewSession(not(eq(SESSION_ID)));
260263
}
@@ -333,8 +336,8 @@ public File getOsFile() {
333336
final CrashlyticsController controller =
334337
builder().setNativeComponent(mockNativeComponent).setLogFileManager(logFileManager).build();
335338

336-
crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider));
337-
crashlyticsWorkers.common.await();
339+
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
340+
commonWorker.await();
338341

339342
verify(mockSessionReportingCoordinator)
340343
.finalizeSessionWithNativeEvent(eq(previousSessionId), any(), any());
@@ -345,8 +348,8 @@ public File getOsFile() {
345348
@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
346349
public void testMissingNativeComponentCausesNoReports() throws Exception {
347350
final CrashlyticsController controller = createController();
348-
crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider));
349-
crashlyticsWorkers.common.await();
351+
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
352+
commonWorker.await();
350353

351354
List<String> sessions = testFileStore.getAllOpenSessionIds();
352355
for (String sessionId : sessions) {
@@ -382,9 +385,8 @@ public void testLogStringAfterCrashOk() throws Exception {
382385
testSettingsProvider, Thread.currentThread(), new RuntimeException());
383386

384387
// This should not throw.
385-
crashlyticsWorkers.diskWrite.submit(
386-
() -> controller.writeToLog(System.currentTimeMillis(), "Hi"));
387-
crashlyticsWorkers.diskWrite.await();
388+
diskWriteWorker.submit(() -> controller.writeToLog(System.currentTimeMillis(), "Hi"));
389+
diskWriteWorker.await();
388390
}
389391

390392
/**
@@ -399,8 +401,8 @@ public void testFinalizeSessionAfterCrashOk() throws Exception {
399401
testSettingsProvider, Thread.currentThread(), new RuntimeException());
400402

401403
// This should not throw.
402-
crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(testSettingsProvider));
403-
crashlyticsWorkers.common.await();
404+
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
405+
commonWorker.await();
404406
}
405407

406408
@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
@@ -443,7 +445,7 @@ public void testUploadDisabledThenOptIn() throws Exception {
443445

444446
final DataCollectionArbiter arbiter = mock(DataCollectionArbiter.class);
445447
when(arbiter.isAutomaticDataCollectionEnabled()).thenReturn(false);
446-
when(arbiter.waitForDataCollectionPermission())
448+
when(arbiter.waitForDataCollectionPermission(any(Executor.class)))
447449
.thenReturn(new TaskCompletionSource<Void>().getTask());
448450
when(arbiter.waitForAutomaticDataCollectionEnabled())
449451
.thenReturn(new TaskCompletionSource<Void>().getTask());
@@ -564,14 +566,14 @@ public void testFatalEvent_sendsAppExceptionEvent() throws Exception {
564566
when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
565567
.thenReturn(new TreeSet<>(Collections.singleton(sessionId)));
566568

567-
crashlyticsWorkers.common.submit(
569+
commonWorker.submit(
568570
() -> {
569571
controller.openSession(SESSION_ID);
570572
controller.handleUncaughtException(
571573
testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal"));
572574
controller.finalizeSessions(testSettingsProvider);
573575
});
574-
crashlyticsWorkers.common.await();
576+
commonWorker.await();
575577

576578
assertFirebaseAnalyticsCrashEvent(mockFirebaseAnalyticsLogger);
577579
}

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import com.google.firebase.crashlytics.internal.RemoteConfigDeferredProxy;
3636
import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger;
3737
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
38-
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
38+
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker;
3939
import com.google.firebase.crashlytics.internal.persistence.FileStore;
4040
import com.google.firebase.crashlytics.internal.settings.Settings;
4141
import com.google.firebase.crashlytics.internal.settings.SettingsController;
@@ -91,7 +91,8 @@ private static final class CoreBuilder {
9191
private CrashlyticsNativeComponent nativeComponent;
9292
private DataCollectionArbiter arbiter;
9393
private FileStore fileStore;
94-
private CrashlyticsWorkers crashlyticsWorkers;
94+
private CrashlyticsWorker commonWorker;
95+
private CrashlyticsWorker diskWriteWorker;
9596

9697
public CoreBuilder(Context context, FirebaseOptions firebaseOptions) {
9798
app = mock(FirebaseApp.class);
@@ -120,8 +121,8 @@ public void whenAvailable(
120121
arbiter = mock(DataCollectionArbiter.class);
121122
when(arbiter.isAutomaticDataCollectionEnabled()).thenReturn(true);
122123

123-
crashlyticsWorkers =
124-
new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
124+
commonWorker = new CrashlyticsWorker(TestOnlyExecutors.background());
125+
diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background());
125126
fileStore = new FileStore(context);
126127
}
127128

@@ -146,7 +147,8 @@ public CrashlyticsCore build() {
146147
fileStore,
147148
mock(CrashlyticsAppQualitySessionsSubscriber.class),
148149
mock(RemoteConfigDeferredProxy.class),
149-
crashlyticsWorkers);
150+
commonWorker,
151+
diskWriteWorker);
150152
}
151153
}
152154

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

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import com.google.firebase.crashlytics.internal.breadcrumbs.BreadcrumbHandler;
4040
import com.google.firebase.crashlytics.internal.breadcrumbs.BreadcrumbSource;
4141
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
42-
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
42+
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorker;
4343
import com.google.firebase.crashlytics.internal.metadata.UserMetadata;
4444
import com.google.firebase.crashlytics.internal.persistence.FileStore;
4545
import com.google.firebase.crashlytics.internal.settings.Settings;
@@ -70,8 +70,10 @@ public void whenAvailable(
7070

7171
private CrashlyticsCore crashlyticsCore;
7272
private BreadcrumbSource mockBreadcrumbSource;
73-
private static final CrashlyticsWorkers crashlyticsWorkers =
74-
new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
73+
private static final CrashlyticsWorker commonWorker =
74+
new CrashlyticsWorker(TestOnlyExecutors.background());
75+
private static final CrashlyticsWorker diskWriteWorker =
76+
new CrashlyticsWorker(TestOnlyExecutors.background());
7577

7678
@Override
7779
protected void setUp() throws Exception {
@@ -95,7 +97,7 @@ public void testCustomAttributes() throws Exception {
9597

9698
final String id = "id012345";
9799
crashlyticsCore.setUserId(id);
98-
crashlyticsWorkers.common.await();
100+
commonWorker.await();
99101
assertEquals(id, metadata.getUserId());
100102

101103
final StringBuffer idBuffer = new StringBuffer(id);
@@ -106,13 +108,13 @@ public void testCustomAttributes() throws Exception {
106108
final String superLongId = longId + "more chars";
107109

108110
crashlyticsCore.setUserId(superLongId);
109-
crashlyticsWorkers.common.await();
111+
commonWorker.await();
110112
assertEquals(longId, metadata.getUserId());
111113

112114
final String key1 = "key1";
113115
final String value1 = "value1";
114116
crashlyticsCore.setCustomKey(key1, value1);
115-
crashlyticsWorkers.common.await();
117+
commonWorker.await();
116118
assertEquals(value1, metadata.getCustomKeys().get(key1));
117119

118120
// Adding an existing key with the same value should return false
@@ -126,7 +128,7 @@ public void testCustomAttributes() throws Exception {
126128

127129
// test truncation of custom keys and attributes
128130
crashlyticsCore.setCustomKey(superLongId, superLongValue);
129-
crashlyticsWorkers.common.await();
131+
commonWorker.await();
130132
assertNull(metadata.getCustomKeys().get(superLongId));
131133
assertEquals(longValue, metadata.getCustomKeys().get(longId));
132134

@@ -135,28 +137,28 @@ public void testCustomAttributes() throws Exception {
135137
final String key = "key" + i;
136138
final String value = "value" + i;
137139
crashlyticsCore.setCustomKey(key, value);
138-
crashlyticsWorkers.common.await();
140+
commonWorker.await();
139141
assertEquals(value, metadata.getCustomKeys().get(key));
140142
}
141143
// should be full now, extra key, value pairs will be dropped.
142144
final String key = "new key";
143145
crashlyticsCore.setCustomKey(key, "some value");
144-
crashlyticsWorkers.common.await();
146+
commonWorker.await();
145147
assertFalse(metadata.getCustomKeys().containsKey(key));
146148

147149
// should be able to update existing keys
148150
crashlyticsCore.setCustomKey(key1, longValue);
149-
crashlyticsWorkers.common.await();
151+
commonWorker.await();
150152
assertEquals(longValue, metadata.getCustomKeys().get(key1));
151153

152154
// when we set a key to null, it should still exist with an empty value
153155
crashlyticsCore.setCustomKey(key1, null);
154-
crashlyticsWorkers.common.await();
156+
commonWorker.await();
155157
assertEquals("", metadata.getCustomKeys().get(key1));
156158

157159
// keys and values are trimmed.
158160
crashlyticsCore.setCustomKey(" " + key1 + " ", " " + longValue + " ");
159-
crashlyticsWorkers.common.await();
161+
commonWorker.await();
160162
assertTrue(metadata.getCustomKeys().containsKey(key1));
161163
assertEquals(longValue, metadata.getCustomKeys().get(key1));
162164
}
@@ -207,7 +209,7 @@ public void testBulkCustomKeys() throws Exception {
207209
keysAndValues.put(intKey, String.valueOf(intValue));
208210

209211
crashlyticsCore.setCustomKeys(keysAndValues);
210-
crashlyticsWorkers.common.await();
212+
commonWorker.await();
211213

212214
assertEquals(stringValue, metadata.getCustomKeys().get(stringKey));
213215
assertEquals(trimmedValue, metadata.getCustomKeys().get(trimmedKey));
@@ -228,7 +230,7 @@ public void testBulkCustomKeys() throws Exception {
228230
addlKeysAndValues.put(key, value);
229231
}
230232
crashlyticsCore.setCustomKeys(addlKeysAndValues);
231-
crashlyticsWorkers.common.await();
233+
commonWorker.await();
232234

233235
// Ensure all keys have been set
234236
assertEquals(UserMetadata.MAX_ATTRIBUTES, metadata.getCustomKeys().size(), DELTA);
@@ -246,7 +248,7 @@ public void testBulkCustomKeys() throws Exception {
246248
extraKeysAndValues.put(key, value);
247249
}
248250
crashlyticsCore.setCustomKeys(extraKeysAndValues);
249-
crashlyticsWorkers.common.await();
251+
commonWorker.await();
250252

251253
// Make sure the extra keys were not added
252254
for (int i = UserMetadata.MAX_ATTRIBUTES; i < UserMetadata.MAX_ATTRIBUTES + 10; ++i) {
@@ -272,7 +274,7 @@ public void testBulkCustomKeys() throws Exception {
272274
updatedKeysAndValues.put(intKey, String.valueOf(updatedIntValue));
273275

274276
crashlyticsCore.setCustomKeys(updatedKeysAndValues);
275-
crashlyticsWorkers.common.await();
277+
commonWorker.await();
276278

277279
assertEquals(updatedStringValue, metadata.getCustomKeys().get(stringKey));
278280
assertFalse(Boolean.parseBoolean(metadata.getCustomKeys().get(booleanKey)));
@@ -445,7 +447,8 @@ CrashlyticsCore build(Context context) {
445447
new FileStore(context),
446448
mock(CrashlyticsAppQualitySessionsSubscriber.class),
447449
mock(RemoteConfigDeferredProxy.class),
448-
crashlyticsWorkers);
450+
commonWorker,
451+
diskWriteWorker);
449452
return crashlyticsCore;
450453
}
451454
}

0 commit comments

Comments
 (0)