Skip to content

Commit af0f42a

Browse files
mrobermrober
authored
Adjust time out and disk worker (#6199)
Adjust time out and disk worker as discussed. This moves some things off the disk worker, but we plan to put them back after more refactoring.
1 parent 17b7c2b commit af0f42a

File tree

8 files changed

+73
-62
lines changed

8 files changed

+73
-62
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@
3737
import android.os.BatteryManager;
3838
import androidx.test.core.app.ApplicationProvider;
3939
import androidx.test.ext.junit.runners.AndroidJUnit4;
40+
import com.google.android.gms.tasks.Task;
4041
import com.google.android.gms.tasks.Tasks;
42+
import com.google.firebase.concurrent.TestOnlyExecutors;
4143
import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider;
44+
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
4245
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport;
4346
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport.Session.Event.Application.Execution;
4447
import com.google.firebase.crashlytics.internal.settings.Settings;
@@ -66,6 +69,8 @@ public class CrashlyticsReportDataCaptureTest {
6669
private int eventThreadImportance;
6770
private int maxChainedExceptions;
6871
private SettingsProvider testSettingsProvider;
72+
private final CrashlyticsWorkers crashlyticsWorkers =
73+
new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
6974

7075
@Mock private DevelopmentPlatformProvider developmentPlatformProvider;
7176

@@ -118,7 +123,9 @@ public void testCaptureReport_containsUnityVersionInDeveloperPlatformFieldsWhenA
118123
.thenReturn(expectedUnityVersion);
119124
initDataCapture();
120125

121-
final CrashlyticsReport report = dataCapture.captureReportData("sessionId", 0);
126+
Task<CrashlyticsReport> task =
127+
crashlyticsWorkers.common.submit(() -> dataCapture.captureReportData("sessionId", 0));
128+
CrashlyticsReport report = Tasks.await(task);
122129

123130
assertNotNull(report.getSession());
124131
assertEquals(UNITY_PLATFORM, report.getSession().getApp().getDevelopmentPlatform());
@@ -132,7 +139,9 @@ public void testCaptureReport_containsNoDeveloperPlatformFieldsWhenUnityIsMissin
132139
when(developmentPlatformProvider.getDevelopmentPlatform()).thenReturn(null);
133140
initDataCapture();
134141

135-
final CrashlyticsReport report = dataCapture.captureReportData("sessionId", 0);
142+
Task<CrashlyticsReport> task =
143+
crashlyticsWorkers.common.submit(() -> dataCapture.captureReportData("sessionId", 0));
144+
CrashlyticsReport report = Tasks.await(task);
136145

137146
assertNotNull(report.getSession());
138147
assertNull(report.getSession().getApp().getDevelopmentPlatform());
@@ -307,10 +316,13 @@ public void testCaptureAnrEvent_noBuildIdInAppData_buildIdsNull() throws Excepti
307316
}
308317

309318
@Test
310-
public void testCaptureReportSessionFields() {
319+
public void testCaptureReportSessionFields() throws Exception {
311320
final String sessionId = "sessionId";
312321
final long timestamp = System.currentTimeMillis();
313-
final CrashlyticsReport report = dataCapture.captureReportData(sessionId, timestamp);
322+
323+
Task<CrashlyticsReport> task =
324+
crashlyticsWorkers.common.submit(() -> dataCapture.captureReportData(sessionId, timestamp));
325+
CrashlyticsReport report = Tasks.await(task);
314326

315327
assertEquals(sessionId, report.getSession().getIdentifier());
316328
assertEquals(timestamp, report.getSession().getStartedAt());

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

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,20 @@
2121

2222
import android.content.SharedPreferences;
2323
import com.google.android.gms.tasks.Tasks;
24+
import com.google.firebase.concurrent.TestOnlyExecutors;
2425
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
26+
import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
27+
import com.google.firebase.crashlytics.internal.concurrency.CrashlyticsWorkers;
2528
import com.google.firebase.installations.FirebaseInstallationsApi;
2629
import java.util.concurrent.TimeoutException;
2730

2831
public class IdManagerTest extends CrashlyticsTestCase {
2932

30-
SharedPreferences prefs;
31-
SharedPreferences legacyPrefs;
33+
private SharedPreferences prefs;
34+
private SharedPreferences legacyPrefs;
35+
36+
private final CrashlyticsWorkers crashlyticsWorkers =
37+
new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
3238

3339
@Override
3440
public void setUp() throws Exception {
@@ -66,21 +72,21 @@ private IdManager createIdManager(String instanceId, DataCollectionArbiter arbit
6672
return new IdManager(getContext(), getContext().getPackageName(), iid, arbiter);
6773
}
6874

69-
public void testCreateUUID() {
75+
public void testCreateUUID() throws Exception {
7076
final String fid = "test_fid";
7177
final IdManager idManager = createIdManager(fid, MOCK_ARBITER_ENABLED);
72-
final String installId = idManager.getInstallIds().getCrashlyticsInstallId();
78+
final String installId = getInstallIds(idManager).getCrashlyticsInstallId();
7379
assertNotNull(installId);
7480

7581
assertEquals(installId, prefs.getString(IdManager.PREFKEY_INSTALLATION_UUID, null));
7682
assertNull(prefs.getString(IdManager.PREFKEY_ADVERTISING_ID, null));
7783
assertEquals(fid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
7884

7985
// subsequent calls should return the same id
80-
assertEquals(installId, idManager.getInstallIds().getCrashlyticsInstallId());
86+
assertEquals(installId, getInstallIds(idManager).getCrashlyticsInstallId());
8187
}
8288

83-
public void testGetIdExceptionalCase_doesNotRotateInstallId() {
89+
public void testGetIdExceptionalCase_doesNotRotateInstallId() throws Exception {
8490
FirebaseInstallationsApi fis = mock(FirebaseInstallationsApi.class);
8591
final String expectedInstallId = "expectedInstallId";
8692
when(fis.getId())
@@ -93,12 +99,12 @@ public void testGetIdExceptionalCase_doesNotRotateInstallId() {
9399

94100
final IdManager idManager =
95101
new IdManager(getContext(), getContext().getPackageName(), fis, MOCK_ARBITER_ENABLED);
96-
final String actualInstallId = idManager.getInstallIds().getCrashlyticsInstallId();
102+
final String actualInstallId = getInstallIds(idManager).getCrashlyticsInstallId();
97103
assertNotNull(actualInstallId);
98104
assertEquals(expectedInstallId, actualInstallId);
99105
}
100106

101-
public void testInstanceIdChanges_dataCollectionEnabled() {
107+
public void testInstanceIdChanges_dataCollectionEnabled() throws Exception {
102108
// Set up the initial state with a valid iid and uuid.
103109
final String oldUuid = "old_uuid";
104110
final String newFid = "new_test_fid";
@@ -111,18 +117,18 @@ public void testInstanceIdChanges_dataCollectionEnabled() {
111117
// Initialize the manager with a different FID.
112118
IdManager idManager = createIdManager(newFid, MOCK_ARBITER_ENABLED);
113119

114-
String installId = idManager.getInstallIds().getCrashlyticsInstallId();
120+
String installId = getInstallIds(idManager).getCrashlyticsInstallId();
115121
assertNotNull(installId);
116122
assertFalse(installId.equals(oldUuid));
117123

118124
assertEquals(installId, prefs.getString(IdManager.PREFKEY_INSTALLATION_UUID, null));
119125
assertEquals(newFid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
120126

121127
// subsequent calls should return the same id
122-
assertEquals(installId, idManager.getInstallIds().getCrashlyticsInstallId());
128+
assertEquals(installId, getInstallIds(idManager).getCrashlyticsInstallId());
123129
}
124130

125-
void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
131+
void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) throws Exception {
126132
final String oldUuid = "test_uuid";
127133
final String fid = dataCollectionEnabled ? "test_fid" : IdManager.createSyntheticFid();
128134
// Set up the initial state with a valid iid and uuid.
@@ -136,7 +142,7 @@ void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
136142
IdManager idManager =
137143
createIdManager(fid, dataCollectionEnabled ? MOCK_ARBITER_ENABLED : MOCK_ARBITER_DISABLED);
138144

139-
String installId = idManager.getInstallIds().getCrashlyticsInstallId();
145+
String installId = getInstallIds(idManager).getCrashlyticsInstallId();
140146
assertNotNull(installId);
141147

142148
// Test that the UUID didn't change.
@@ -146,18 +152,18 @@ void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
146152
assertEquals(fid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
147153

148154
// subsequent calls should return the same id
149-
assertEquals(oldUuid, idManager.getInstallIds().getCrashlyticsInstallId());
155+
assertEquals(oldUuid, getInstallIds(idManager).getCrashlyticsInstallId());
150156
}
151157

152-
public void testInstanceIdDoesntChange_dataCollectionEnabled() {
158+
public void testInstanceIdDoesntChange_dataCollectionEnabled() throws Exception {
153159
validateInstanceIdDoesntChange(/* dataCollectionEnabled= */ true);
154160
}
155161

156-
public void testInstanceIdDoesntChange_dataCollectionDisabled() {
162+
public void testInstanceIdDoesntChange_dataCollectionDisabled() throws Exception {
157163
validateInstanceIdDoesntChange(/* dataCollectionEnabled= */ false);
158164
}
159165

160-
public void testInstanceIdRotatesWithDataCollectionFlag() {
166+
public void testInstanceIdRotatesWithDataCollectionFlag() throws Exception {
161167
final String originalUuid = "test_uuid";
162168
final String originalFid = "test_fid";
163169
// Set up the initial state with a valid iid and uuid.
@@ -169,44 +175,47 @@ public void testInstanceIdRotatesWithDataCollectionFlag() {
169175

170176
// Initialize the manager with the same FID.
171177
IdManager idManager = createIdManager(originalFid, MOCK_ARBITER_ENABLED);
172-
String firstUuid = idManager.getInstallIds().getCrashlyticsInstallId();
178+
String firstUuid = getInstallIds(idManager).getCrashlyticsInstallId();
173179
assertNotNull(firstUuid);
174180
assertEquals(originalUuid, firstUuid);
175181

176182
// subsequent calls should return the same id
177-
assertEquals(firstUuid, idManager.getInstallIds().getCrashlyticsInstallId());
183+
assertEquals(firstUuid, getInstallIds(idManager).getCrashlyticsInstallId());
178184
assertEquals(
179185
firstUuid,
180-
createIdManager(originalFid, MOCK_ARBITER_ENABLED)
181-
.getInstallIds()
186+
getInstallIds(createIdManager(originalFid, MOCK_ARBITER_ENABLED))
182187
.getCrashlyticsInstallId());
183188

184189
// Disable data collection manager and confirm we get a different id
185190
idManager = createIdManager(originalFid, MOCK_ARBITER_DISABLED);
186-
String secondUuid = idManager.getInstallIds().getCrashlyticsInstallId();
191+
String secondUuid = getInstallIds(idManager).getCrashlyticsInstallId();
187192
assertNotSame(secondUuid, firstUuid);
188-
assertEquals(secondUuid, idManager.getInstallIds().getCrashlyticsInstallId());
193+
assertEquals(secondUuid, getInstallIds(idManager).getCrashlyticsInstallId());
189194
assertEquals(
190195
secondUuid,
191-
createIdManager(null, MOCK_ARBITER_DISABLED).getInstallIds().getCrashlyticsInstallId());
196+
getInstallIds(createIdManager(null, MOCK_ARBITER_DISABLED)).getCrashlyticsInstallId());
192197
// Check that we cached an synthetic FID
193198
final SharedPreferences prefs = CommonUtils.getSharedPrefs(getContext());
194199
String cachedFid = prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null);
195200
assertTrue(IdManager.isSyntheticFid(cachedFid));
196201

197202
// re-enable data collection
198203
idManager = createIdManager(originalFid, MOCK_ARBITER_ENABLED);
199-
String thirdUuid = idManager.getInstallIds().getCrashlyticsInstallId();
204+
String thirdUuid = getInstallIds(idManager).getCrashlyticsInstallId();
200205
assertNotSame(thirdUuid, firstUuid);
201206
assertNotSame(thirdUuid, secondUuid);
202-
assertEquals(thirdUuid, idManager.getInstallIds().getCrashlyticsInstallId());
207+
assertEquals(thirdUuid, getInstallIds(idManager).getCrashlyticsInstallId());
203208
assertEquals(
204209
thirdUuid,
205-
createIdManager(originalFid, MOCK_ARBITER_ENABLED)
206-
.getInstallIds()
210+
getInstallIds(createIdManager(originalFid, MOCK_ARBITER_ENABLED))
207211
.getCrashlyticsInstallId());
208212
// The cached ID should be back to the original
209213
cachedFid = prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null);
210214
assertEquals(cachedFid, originalFid);
211215
}
216+
217+
/** Get the install ids on the common worker. */
218+
private InstallIds getInstallIds(IdManager idManager) throws Exception {
219+
return Tasks.await(crashlyticsWorkers.common.submit(idManager::getInstallIds));
220+
}
212221
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ public void testWriteUserData_emptyString() throws Exception {
145145
SESSION_ID_1, metadataWithUserId(SESSION_ID_1, "").getUserId());
146146
});
147147
crashlyticsWorkers.diskWrite.await();
148+
Thread.sleep(3);
148149
UserMetadata userData =
149150
UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers);
150151
assertEquals("", userData.getUserId());

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,11 +359,8 @@ Task<Void> submitAllReports(Task<Settings> settingsDataTask) {
359359
public Task<Void> then(@Nullable Boolean send) throws Exception {
360360
if (!send) {
361361
Logger.getLogger().v("Deleting cached crash reports...");
362-
crashlyticsWorkers.diskWrite.submit(
363-
() -> {
364-
deleteFiles(listAppExceptionMarkerFiles());
365-
reportingCoordinator.removeAllReports();
366-
});
362+
deleteFiles(listAppExceptionMarkerFiles());
363+
reportingCoordinator.removeAllReports();
367364
unsentReportsHandled.trySetResult(null);
368365
return Tasks.forResult(null);
369366
}

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -468,20 +468,15 @@ void markInitializationStarted() {
468468

469469
/** Enqueues a job to remove the Crashlytics initialization marker file */
470470
void markInitializationComplete() {
471-
crashlyticsWorkers.common.submit(
472-
() -> {
473-
try {
474-
boolean removed = initializationMarker.remove();
475-
if (!removed) {
476-
Logger.getLogger().w("Initialization marker file was not properly removed.");
477-
}
478-
return removed;
479-
} catch (Exception e) {
480-
Logger.getLogger()
481-
.e("Problem encountered deleting Crashlytics initialization marker.", e);
482-
return false;
483-
}
484-
});
471+
CrashlyticsWorkers.checkBackgroundThread();
472+
try {
473+
boolean removed = initializationMarker.remove();
474+
if (!removed) {
475+
Logger.getLogger().w("Initialization marker file was not properly removed.");
476+
}
477+
} catch (Exception ex) {
478+
Logger.getLogger().e("Problem encountered deleting Crashlytics initialization marker.", ex);
479+
}
485480
}
486481

487482
boolean didPreviousInitializationFail() {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ private static String formatId(@NonNull String id) {
104104
@Override
105105
@NonNull
106106
public synchronized InstallIds getInstallIds() {
107+
CrashlyticsWorkers.checkBackgroundThread();
107108
if (!shouldRefresh()) {
108109
return installIds;
109110
}

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -354,16 +354,12 @@ private boolean onReportSendComplete(@NonNull Task<CrashlyticsReportWithSessionI
354354
CrashlyticsReportWithSessionId report = task.getResult();
355355
Logger.getLogger()
356356
.d("Crashlytics report successfully enqueued to DataTransport: " + report.getSessionId());
357-
crashlyticsWorkers.diskWrite.submit(
358-
() -> {
359-
File reportFile = report.getReportFile();
360-
if (reportFile.delete()) {
361-
Logger.getLogger().d("Deleted report file: " + reportFile.getPath());
362-
} else {
363-
Logger.getLogger()
364-
.w("Crashlytics could not delete report file: " + reportFile.getPath());
365-
}
366-
});
357+
File reportFile = report.getReportFile();
358+
if (reportFile.delete()) {
359+
Logger.getLogger().d("Deleted report file: " + reportFile.getPath());
360+
} else {
361+
Logger.getLogger().w("Crashlytics could not delete report file: " + reportFile.getPath());
362+
}
367363
return true;
368364
}
369365
Logger.getLogger()

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
@SuppressWarnings({"ResultOfMethodCallIgnored", "UnusedReturnValue"})
2828
public final class Utils {
2929
/** Timeout in milliseconds for blocking on background threads. */
30-
private static final int BACKGROUND_TIMEOUT_MILLIS = 5_000;
30+
private static final int BACKGROUND_TIMEOUT_MILLIS = 3_000;
3131

3232
/** Timeout in milliseconds for blocking on the main thread. Be careful about ANRs. */
33-
private static final int MAIN_TIMEOUT_MILLIS = 4_000;
33+
private static final int MAIN_TIMEOUT_MILLIS = 2_000;
3434

3535
/**
3636
* Blocks until the given Task completes, and then returns the value the Task was resolved with,

0 commit comments

Comments
 (0)