Skip to content

Commit 656882e

Browse files
mrobermrober
committed
Revert "User action refactor (#6120)"
This reverts commit 3a4ec18.
1 parent ad3266d commit 656882e

File tree

6 files changed

+79
-87
lines changed

6 files changed

+79
-87
lines changed

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ 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-
7977
@Override
8078
protected void setUp() throws Exception {
8179
super.setUp();
@@ -188,8 +186,7 @@ public CrashlyticsController build() {
188186
sessionReportingCoordinator,
189187
nativeComponent,
190188
analyticsEventLogger,
191-
mock(CrashlyticsAppQualitySessionsSubscriber.class),
192-
diskWriteWorker);
189+
mock(CrashlyticsAppQualitySessionsSubscriber.class));
193190
return controller;
194191
}
195192
}
@@ -379,14 +376,13 @@ public void testLoggedExceptionsAfterCrashOk() {
379376
*/
380377
// FIXME: Validate this test works as intended
381378
@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
382-
public void testLogStringAfterCrashOk() throws Exception {
379+
public void testLogStringAfterCrashOk() {
383380
final CrashlyticsController controller = builder().build();
384381
controller.handleUncaughtException(
385382
testSettingsProvider, Thread.currentThread(), new RuntimeException());
386383

387384
// This should not throw.
388-
diskWriteWorker.submit(() -> controller.writeToLog(System.currentTimeMillis(), "Hi"));
389-
diskWriteWorker.await();
385+
controller.writeToLog(System.currentTimeMillis(), "Hi");
390386
}
391387

392388
/**

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public void testCustomAttributes() throws Exception {
9393

9494
final String id = "id012345";
9595
crashlyticsCore.setUserId(id);
96-
crashlyticsCore.commonWorker.await();
96+
9797
assertEquals(id, metadata.getUserId());
9898

9999
final StringBuffer idBuffer = new StringBuffer(id);
@@ -104,13 +104,11 @@ public void testCustomAttributes() throws Exception {
104104
final String superLongId = longId + "more chars";
105105

106106
crashlyticsCore.setUserId(superLongId);
107-
crashlyticsCore.commonWorker.await();
108107
assertEquals(longId, metadata.getUserId());
109108

110109
final String key1 = "key1";
111110
final String value1 = "value1";
112111
crashlyticsCore.setCustomKey(key1, value1);
113-
crashlyticsCore.commonWorker.await();
114112
assertEquals(value1, metadata.getCustomKeys().get(key1));
115113

116114
// Adding an existing key with the same value should return false
@@ -124,7 +122,6 @@ public void testCustomAttributes() throws Exception {
124122

125123
// test truncation of custom keys and attributes
126124
crashlyticsCore.setCustomKey(superLongId, superLongValue);
127-
crashlyticsCore.commonWorker.await();
128125
assertNull(metadata.getCustomKeys().get(superLongId));
129126
assertEquals(longValue, metadata.getCustomKeys().get(longId));
130127

@@ -133,28 +130,23 @@ public void testCustomAttributes() throws Exception {
133130
final String key = "key" + i;
134131
final String value = "value" + i;
135132
crashlyticsCore.setCustomKey(key, value);
136-
crashlyticsCore.commonWorker.await();
137133
assertEquals(value, metadata.getCustomKeys().get(key));
138134
}
139135
// should be full now, extra key, value pairs will be dropped.
140136
final String key = "new key";
141137
crashlyticsCore.setCustomKey(key, "some value");
142-
crashlyticsCore.commonWorker.await();
143138
assertFalse(metadata.getCustomKeys().containsKey(key));
144139

145140
// should be able to update existing keys
146141
crashlyticsCore.setCustomKey(key1, longValue);
147-
crashlyticsCore.commonWorker.await();
148142
assertEquals(longValue, metadata.getCustomKeys().get(key1));
149143

150144
// when we set a key to null, it should still exist with an empty value
151145
crashlyticsCore.setCustomKey(key1, null);
152-
crashlyticsCore.commonWorker.await();
153146
assertEquals("", metadata.getCustomKeys().get(key1));
154147

155148
// keys and values are trimmed.
156149
crashlyticsCore.setCustomKey(" " + key1 + " ", " " + longValue + " ");
157-
crashlyticsCore.commonWorker.await();
158150
assertTrue(metadata.getCustomKeys().containsKey(key1));
159151
assertEquals(longValue, metadata.getCustomKeys().get(key1));
160152
}
@@ -205,7 +197,6 @@ public void testBulkCustomKeys() throws Exception {
205197
keysAndValues.put(intKey, String.valueOf(intValue));
206198

207199
crashlyticsCore.setCustomKeys(keysAndValues);
208-
crashlyticsCore.commonWorker.await();
209200

210201
assertEquals(stringValue, metadata.getCustomKeys().get(stringKey));
211202
assertEquals(trimmedValue, metadata.getCustomKeys().get(trimmedKey));
@@ -226,7 +217,6 @@ public void testBulkCustomKeys() throws Exception {
226217
addlKeysAndValues.put(key, value);
227218
}
228219
crashlyticsCore.setCustomKeys(addlKeysAndValues);
229-
crashlyticsCore.commonWorker.await();
230220

231221
// Ensure all keys have been set
232222
assertEquals(UserMetadata.MAX_ATTRIBUTES, metadata.getCustomKeys().size(), DELTA);
@@ -244,7 +234,6 @@ public void testBulkCustomKeys() throws Exception {
244234
extraKeysAndValues.put(key, value);
245235
}
246236
crashlyticsCore.setCustomKeys(extraKeysAndValues);
247-
crashlyticsCore.commonWorker.await();
248237

249238
// Make sure the extra keys were not added
250239
for (int i = UserMetadata.MAX_ATTRIBUTES; i < UserMetadata.MAX_ATTRIBUTES + 10; ++i) {
@@ -270,7 +259,6 @@ public void testBulkCustomKeys() throws Exception {
270259
updatedKeysAndValues.put(intKey, String.valueOf(updatedIntValue));
271260

272261
crashlyticsCore.setCustomKeys(updatedKeysAndValues);
273-
crashlyticsCore.commonWorker.await();
274262

275263
assertEquals(updatedStringValue, metadata.getCustomKeys().get(stringKey));
276264
assertFalse(Boolean.parseBoolean(metadata.getCustomKeys().get(booleanKey)));

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

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ class CrashlyticsController {
8686

8787
private final CrashlyticsWorker backgroundWorker;
8888

89-
private final CrashlyticsWorker diskWriteWorker;
90-
9189
private final IdManager idManager;
9290
private final FileStore fileStore;
9391

@@ -131,8 +129,7 @@ class CrashlyticsController {
131129
SessionReportingCoordinator sessionReportingCoordinator,
132130
CrashlyticsNativeComponent nativeComponent,
133131
AnalyticsEventLogger analyticsEventLogger,
134-
CrashlyticsAppQualitySessionsSubscriber sessionsSubscriber,
135-
CrashlyticsWorker diskWriteWorker) {
132+
CrashlyticsAppQualitySessionsSubscriber sessionsSubscriber) {
136133
this.context = context;
137134
this.backgroundWorker = commonWorker;
138135
this.idManager = idManager;
@@ -146,7 +143,6 @@ class CrashlyticsController {
146143
this.analyticsEventLogger = analyticsEventLogger;
147144
this.sessionsSubscriber = sessionsSubscriber;
148145
this.reportingCoordinator = sessionReportingCoordinator;
149-
this.diskWriteWorker = diskWriteWorker;
150146
}
151147

152148
private Context getContext() {
@@ -422,9 +418,16 @@ public Task<Void> then(@Nullable Settings appSettingsData)
422418

423419
/** Log a timestamped string to the log file. */
424420
void writeToLog(final long timestamp, final String msg) {
425-
if (!isHandlingException()) {
426-
logFileManager.writeToLog(timestamp, msg);
427-
}
421+
backgroundWorker.submit(
422+
new Callable<Void>() {
423+
@Override
424+
public Void call() throws Exception {
425+
if (!isHandlingException()) {
426+
logFileManager.writeToLog(timestamp, msg);
427+
}
428+
return null;
429+
}
430+
});
428431
}
429432

430433
/** Log a caught exception - write out Throwable as event section of protobuf */
@@ -433,15 +436,23 @@ void writeNonFatalException(@NonNull final Thread thread, @NonNull final Throwab
433436
// rather than the time at which the task executes.
434437
final long timestampMillis = System.currentTimeMillis();
435438

436-
if (!isHandlingException()) {
437-
long timestampSeconds = getTimestampSeconds(timestampMillis);
438-
final String currentSessionId = getCurrentSessionId();
439-
if (currentSessionId == null) {
440-
Logger.getLogger().w("Tried to write a non-fatal exception while no session was open.");
441-
return;
442-
}
443-
reportingCoordinator.persistNonFatalEvent(ex, thread, currentSessionId, timestampSeconds);
444-
}
439+
backgroundWorker.submit(
440+
new Runnable() {
441+
@Override
442+
public void run() {
443+
if (!isHandlingException()) {
444+
long timestampSeconds = getTimestampSeconds(timestampMillis);
445+
final String currentSessionId = getCurrentSessionId();
446+
if (currentSessionId == null) {
447+
Logger.getLogger()
448+
.w("Tried to write a non-fatal exception while no session was open.");
449+
return;
450+
}
451+
reportingCoordinator.persistNonFatalEvent(
452+
ex, thread, currentSessionId, timestampSeconds);
453+
}
454+
}
455+
});
445456
}
446457

447458
void logFatalException(Thread thread, Throwable ex) {
@@ -930,7 +941,7 @@ private void writeApplicationExitInfoEventIfRelevant(String sessionId) {
930941
if (applicationExitInfoList.size() != 0) {
931942
final LogFileManager relevantSessionLogManager = new LogFileManager(fileStore, sessionId);
932943
final UserMetadata relevantUserMetadata =
933-
UserMetadata.loadFromExistingSession(sessionId, fileStore, diskWriteWorker);
944+
UserMetadata.loadFromExistingSession(sessionId, fileStore, backgroundWorker);
934945
reportingCoordinator.persistRelevantAppExitInfoEvent(
935946
sessionId, applicationExitInfoList, relevantSessionLogManager, relevantUserMetadata);
936947
} else {

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

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ public class CrashlyticsCore {
9999

100100
private final RemoteConfigDeferredProxy remoteConfigDeferredProxy;
101101

102-
@VisibleForTesting final CrashlyticsWorker commonWorker;
103-
@VisibleForTesting final CrashlyticsWorker diskWriteWorker;
102+
private final CrashlyticsWorker commonWorker;
103+
private final CrashlyticsWorker diskWriteWorker;
104104

105105
// region Constructors
106106

@@ -156,7 +156,7 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider)
156156
initializationMarker = new CrashlyticsFileMarker(INITIALIZATION_MARKER_FILE_NAME, fileStore);
157157

158158
final UserMetadata userMetadata =
159-
new UserMetadata(sessionIdentifier, fileStore, diskWriteWorker);
159+
new UserMetadata(sessionIdentifier, fileStore, commonWorker);
160160
final LogFileManager logFileManager = new LogFileManager(fileStore);
161161
final StackTraceTrimmingStrategy stackTraceTrimmingStrategy =
162162
new MiddleOutFallbackStrategy(
@@ -191,8 +191,7 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider)
191191
sessionReportingCoordinator,
192192
nativeComponent,
193193
analyticsEventLogger,
194-
sessionsSubscriber,
195-
diskWriteWorker);
194+
sessionsSubscriber);
196195

197196
// If the file is present at this point, then the previous run's initialization
198197
// did not complete, and we want to perform initialization synchronously this time.
@@ -325,7 +324,7 @@ public static String getVersion() {
325324
* safe to invoke this method from the main thread.
326325
*/
327326
public void logException(@NonNull Throwable throwable) {
328-
commonWorker.submit(() -> controller.writeNonFatalException(Thread.currentThread(), throwable));
327+
controller.writeNonFatalException(Thread.currentThread(), throwable);
329328
}
330329

331330
/**
@@ -340,11 +339,11 @@ public void logException(@NonNull Throwable throwable) {
340339
*/
341340
public void log(final String msg) {
342341
final long timestamp = System.currentTimeMillis() - startTime;
343-
commonWorker.submit(() -> controller.writeToLog(timestamp, msg));
342+
controller.writeToLog(timestamp, msg);
344343
}
345344

346345
public void setUserId(String identifier) {
347-
commonWorker.submit(() -> controller.setUserId(identifier));
346+
controller.setUserId(identifier);
348347
}
349348

350349
/**
@@ -357,7 +356,7 @@ public void setUserId(String identifier) {
357356
* @throws NullPointerException if key is null.
358357
*/
359358
public void setCustomKey(String key, String value) {
360-
commonWorker.submit(() -> controller.setCustomKey(key, value));
359+
controller.setCustomKey(key, value);
361360
}
362361

363362
/**
@@ -374,7 +373,7 @@ public void setCustomKey(String key, String value) {
374373
* @throws NullPointerException if any key in keysAndValues is null.
375374
*/
376375
public void setCustomKeys(Map<String, String> keysAndValues) {
377-
commonWorker.submit(() -> controller.setCustomKeys(keysAndValues));
376+
controller.setCustomKeys(keysAndValues);
378377
}
379378

380379
// endregion
@@ -394,7 +393,7 @@ public void setCustomKeys(Map<String, String> keysAndValues) {
394393
* @throws NullPointerException if key is null.
395394
*/
396395
public void setInternalKey(String key, String value) {
397-
commonWorker.submit(() -> controller.setInternalKey(key, value));
396+
controller.setInternalKey(key, value);
398397
}
399398

400399
/** Logs a fatal Throwable on the Crashlytics servers on-demand. */
@@ -403,16 +402,11 @@ public void logFatalException(Throwable throwable) {
403402
.d("Recorded on-demand fatal events: " + onDemandCounter.getRecordedOnDemandExceptions());
404403
Logger.getLogger()
405404
.d("Dropped on-demand fatal events: " + onDemandCounter.getDroppedOnDemandExceptions());
406-
commonWorker.submit(
407-
() -> {
408-
controller.setInternalKey(
409-
ON_DEMAND_RECORDED_KEY,
410-
Integer.toString(onDemandCounter.getRecordedOnDemandExceptions()));
411-
controller.setInternalKey(
412-
ON_DEMAND_DROPPED_KEY,
413-
Integer.toString(onDemandCounter.getDroppedOnDemandExceptions()));
414-
controller.logFatalException(Thread.currentThread(), throwable);
415-
});
405+
controller.setInternalKey(
406+
ON_DEMAND_RECORDED_KEY, Integer.toString(onDemandCounter.getRecordedOnDemandExceptions()));
407+
controller.setInternalKey(
408+
ON_DEMAND_DROPPED_KEY, Integer.toString(onDemandCounter.getDroppedOnDemandExceptions()));
409+
controller.logFatalException(Thread.currentThread(), throwable);
416410
}
417411

418412
// endregion

0 commit comments

Comments
 (0)