Skip to content

Commit 7b9b80e

Browse files
raymondlamraymondlam
authored
Removing the isPerformanceMonitoringEnabled check in FirebasePerfProvider and moving the getSharedPreferences to a separate thread. (firebase#2458)
Removing the `isPerformanceMonitoringEnabled()` check in `FirebasePerfProvider` and moving the `getSharedPreferences()` call to a separate thread to address StrictMode violation in b/171411615.
1 parent f54d96a commit 7b9b80e

File tree

8 files changed

+165
-33
lines changed

8 files changed

+165
-33
lines changed

‎firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ public static void clearInstance() {
9494
configResolver = null;
9595
}
9696

97+
@VisibleForTesting
98+
public void setDeviceCacheManager(DeviceCacheManager deviceCacheManager) {
99+
this.deviceCacheManager = deviceCacheManager;
100+
}
101+
97102
public void setContentProviderContext(Context context) {
98103
setApplicationContext(context.getApplicationContext());
99104
}

‎firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import com.google.firebase.perf.logging.AndroidLogger;
2323
import com.google.firebase.perf.util.Constants;
2424
import com.google.firebase.perf.util.Optional;
25+
import java.util.concurrent.ExecutorService;
26+
import java.util.concurrent.Executors;
2527

2628
/**
2729
* Utilizes platform supported APIs for storing and retrieving Firebase Performance related
@@ -40,13 +42,19 @@ public class DeviceCacheManager {
4042
// The key-value pairs are written to XML files that persist across user sessions, even if the
4143
// app is killed.
4244
// https://developer.android.com/guide/topics/data/data-storage.html#pref
43-
private SharedPreferences sharedPref;
45+
private volatile SharedPreferences sharedPref;
4446

45-
private DeviceCacheManager() {}
47+
// Used for retrieving the shared preferences on a separate thread.
48+
private ExecutorService serialExecutor;
49+
50+
@VisibleForTesting
51+
public DeviceCacheManager(ExecutorService serialExecutor) {
52+
this.serialExecutor = serialExecutor;
53+
}
4654

4755
public static synchronized DeviceCacheManager getInstance() {
4856
if (instance == null) {
49-
instance = new DeviceCacheManager();
57+
instance = new DeviceCacheManager(Executors.newSingleThreadExecutor());
5058
}
5159
return instance;
5260
}
@@ -56,9 +64,19 @@ public static void clearInstance() {
5664
instance = null;
5765
}
5866

67+
/**
68+
* Triggers a getSharedPreferences call in a separate thread if shared preferences is not set.
69+
* This method returns immediately without waiting for the getSharedPreferences call to be
70+
* complete.
71+
*/
5972
public synchronized void setContext(Context context) {
6073
if (sharedPref == null && context != null) {
61-
this.sharedPref = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE);
74+
serialExecutor.execute(
75+
() -> {
76+
if (sharedPref == null && context != null) {
77+
this.sharedPref = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE);
78+
}
79+
});
6280
}
6381
}
6482

‎firebase-perf/src/main/java/com/google/firebase/perf/provider/FirebasePerfProvider.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,11 @@ public void attachInfo(Context context, ProviderInfo info) {
6161
ConfigResolver configResolver = ConfigResolver.getInstance();
6262
configResolver.setContentProviderContext(getContext());
6363

64-
if (configResolver.isPerformanceMonitoringEnabled()) {
65-
AppStateMonitor.getInstance().registerActivityLifecycleCallbacks(getContext());
66-
AppStartTrace appStartTrace = AppStartTrace.getInstance();
67-
appStartTrace.registerActivityLifecycleCallbacks(getContext());
64+
AppStateMonitor.getInstance().registerActivityLifecycleCallbacks(getContext());
65+
AppStartTrace appStartTrace = AppStartTrace.getInstance();
66+
appStartTrace.registerActivityLifecycleCallbacks(getContext());
6867

69-
mHandler.post(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
70-
}
68+
mHandler.post(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
7169

7270
// In the case of cold start, we create a session and start collecting gauges as early as
7371
// possible.

‎firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerformanceTest.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.google.firebase.perf.util.Constants;
4040
import com.google.firebase.perf.util.ImmutableBundle;
4141
import com.google.firebase.remoteconfig.RemoteConfigComponent;
42+
import com.google.testing.timing.FakeDirectExecutorService;
4243
import java.util.Map;
4344
import org.junit.After;
4445
import org.junit.Assert;
@@ -68,6 +69,8 @@ public class FirebasePerformanceTest {
6869

6970
@Rule public MockitoRule initRule = MockitoJUnit.rule();
7071

72+
private FakeDirectExecutorService fakeDirectExecutorService;
73+
7174
@Before
7275
public void setUp() throws NameNotFoundException {
7376
FirebaseOptions options =
@@ -98,6 +101,7 @@ public void setUp() throws NameNotFoundException {
98101
spyConfigResolver = spy(ConfigResolver.getInstance());
99102

100103
spyGaugeManager = spy(GaugeManager.getInstance());
104+
fakeDirectExecutorService = new FakeDirectExecutorService();
101105
}
102106

103107
@After
@@ -542,8 +546,9 @@ private FirebasePerformance initializeFirebasePerformancePreferences(
542546
Provider<RemoteConfigComponent> firebaseRemoteConfigProvider,
543547
Provider<TransportFactory> transportFactoryProvider) {
544548
if (sharedPreferencesEnabledDisabledKey != null) {
545-
DeviceCacheManager.getInstance()
546-
.setValue(Constants.ENABLE_DISABLE, sharedPreferencesEnabledDisabledKey);
549+
DeviceCacheManager deviceCacheManager = new DeviceCacheManager(fakeDirectExecutorService);
550+
deviceCacheManager.setContext(ApplicationProvider.getApplicationContext());
551+
deviceCacheManager.setValue(Constants.ENABLE_DISABLE, sharedPreferencesEnabledDisabledKey);
547552
}
548553

549554
Bundle bundle = new Bundle();

‎firebase-perf/src/test/java/com/google/firebase/perf/config/DeviceCacheManagerTest.java‎

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.google.firebase.FirebaseApp;
2020
import com.google.firebase.perf.FirebasePerformanceTestBase;
21+
import com.google.testing.timing.FakeScheduledExecutorService;
2122
import org.junit.Before;
2223
import org.junit.Test;
2324
import org.junit.runner.RunWith;
@@ -28,27 +29,32 @@
2829
public final class DeviceCacheManagerTest extends FirebasePerformanceTestBase {
2930

3031
private DeviceCacheManager deviceCacheManager;
32+
private FakeScheduledExecutorService fakeScheduledExecutorService;
3133

3234
@Before
3335
public void setUp() {
34-
deviceCacheManager = DeviceCacheManager.getInstance();
36+
fakeScheduledExecutorService = new FakeScheduledExecutorService();
37+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
3538
}
3639

3740
@Test
3841
public void getBoolean_valueIsNotSet_returnsEmpty() {
3942
deviceCacheManager.setContext(context);
43+
fakeScheduledExecutorService.runAll();
4044

4145
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
4246
}
4347

4448
@Test
4549
public void getBoolean_contextAndValueNotSet_returnsEmpty() {
50+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
4651
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
4752
}
4853

4954
@Test
5055
public void getBoolean_valueIsSet_returnsSetValue() {
5156
deviceCacheManager.setContext(context);
57+
fakeScheduledExecutorService.runAll();
5258
deviceCacheManager.setValue("some_key", true);
5359

5460
assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
@@ -57,7 +63,7 @@ public void getBoolean_valueIsSet_returnsSetValue() {
5763
@Test
5864
public void clear_setBooleanThenCleared_returnsEmpty() {
5965
deviceCacheManager.setContext(context);
60-
66+
fakeScheduledExecutorService.runAll();
6167
deviceCacheManager.setValue("some_key", true);
6268

6369
assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
@@ -70,14 +76,17 @@ public void clear_setBooleanThenCleared_returnsEmpty() {
7076
public void getBoolean_firebaseAppNotExist_returnsEmpty() {
7177
DeviceCacheManager.clearInstance();
7278
FirebaseApp.clearInstancesForTest();
73-
deviceCacheManager = DeviceCacheManager.getInstance();
79+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
7480
deviceCacheManager.setValue("some_key", true);
7581

82+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
7683
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
7784
}
7885

7986
@Test
8087
public void setValueBoolean_setTwice_canGetLatestValue() {
88+
deviceCacheManager.setContext(context);
89+
fakeScheduledExecutorService.runAll();
8190
deviceCacheManager.setValue("some_key", true);
8291
assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
8392

@@ -86,10 +95,10 @@ public void setValueBoolean_setTwice_canGetLatestValue() {
8695
}
8796

8897
@Test
89-
public void setValueBoolean_contextNotSet_canGetValue() {
98+
public void setValueBoolean_contextNotSet_returnsEmpty() {
9099
deviceCacheManager.setValue("some_key", true);
91100

92-
assertThat(deviceCacheManager.getBoolean("some_key").get()).isTrue();
101+
assertThat(deviceCacheManager.getBoolean("some_key").isAvailable()).isFalse();
93102
}
94103

95104
@Test
@@ -100,6 +109,7 @@ public void setValueBoolean_keyIsNull_returnsFalse() {
100109
@Test
101110
public void getString_valueIsNotSet_returnsEmpty() {
102111
deviceCacheManager.setContext(context);
112+
fakeScheduledExecutorService.runAll();
103113

104114
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
105115
}
@@ -112,23 +122,26 @@ public void getString_contextAndValueNotSet_returnsEmpty() {
112122
@Test
113123
public void getString_valueIsSet_returnsSetValue() {
114124
deviceCacheManager.setContext(context);
115-
deviceCacheManager.setValue("some_key", "speicalValue");
125+
fakeScheduledExecutorService.runAll();
126+
deviceCacheManager.setValue("some_key", "specialValue");
116127

117-
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("speicalValue");
128+
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("specialValue");
118129
}
119130

120131
@Test
121132
public void getString_firebaseAppNotExist_returnsEmpty() {
122133
DeviceCacheManager.clearInstance();
123134
FirebaseApp.clearInstancesForTest();
124-
deviceCacheManager = DeviceCacheManager.getInstance();
125-
deviceCacheManager.setValue("some_key", "speicalValue");
135+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
136+
deviceCacheManager.setValue("some_key", "specialValue");
126137

127138
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
128139
}
129140

130141
@Test
131142
public void setValueString_setTwice_canGetLatestValue() {
143+
deviceCacheManager.setContext(context);
144+
fakeScheduledExecutorService.runAll();
132145
deviceCacheManager.setValue("some_key", "EarliestValue");
133146
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("EarliestValue");
134147

@@ -137,40 +150,47 @@ public void setValueString_setTwice_canGetLatestValue() {
137150
}
138151

139152
@Test
140-
public void setValueString_contextNotSet_canGetValue() {
153+
public void setValueString_contextNotSet_returnsEmpty() {
141154
deviceCacheManager.setValue("some_key", "newValue");
142-
assertThat(deviceCacheManager.getString("some_key").get()).isEqualTo("newValue");
155+
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
143156
}
144157

145158
@Test
146159
public void setValueString_setNullString_returnsEmpty() {
160+
deviceCacheManager.setContext(context);
161+
fakeScheduledExecutorService.runAll();
147162
deviceCacheManager.setValue("some_key", null);
148163
assertThat(deviceCacheManager.getString("some_key").isAvailable()).isFalse();
149164
}
150165

151166
@Test
152167
public void setValueString_keyIsNull_returnsFalse() {
168+
deviceCacheManager.setContext(context);
169+
fakeScheduledExecutorService.runAll();
153170
assertThat(deviceCacheManager.setValue(null, "value")).isFalse();
154171
}
155172

156173
@Test
157174
public void getFloat_valueIsNotSet_returnsEmpty() {
158175
deviceCacheManager.setContext(context);
176+
fakeScheduledExecutorService.runAll();
159177

160178
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
161179
}
162180

163181
@Test
164182
public void getFloat_contextAndValueNotSet_returnsEmpty() {
165183
DeviceCacheManager.clearInstance();
166-
deviceCacheManager = DeviceCacheManager.getInstance();
184+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
167185

186+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
168187
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
169188
}
170189

171190
@Test
172191
public void getFloat_valueIsSet_returnsSetValue() {
173192
deviceCacheManager.setContext(context);
193+
fakeScheduledExecutorService.runAll();
174194
deviceCacheManager.setValue("some_key", 1.2f);
175195

176196
assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(1.2f);
@@ -180,14 +200,17 @@ public void getFloat_valueIsSet_returnsSetValue() {
180200
public void getFloat_firebaseAppNotExist_returnsEmpty() {
181201
DeviceCacheManager.clearInstance();
182202
FirebaseApp.clearInstancesForTest();
183-
deviceCacheManager = DeviceCacheManager.getInstance();
203+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
184204
deviceCacheManager.setValue("some_key", 1.2f);
185205

206+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
186207
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
187208
}
188209

189210
@Test
190211
public void setValueFloat_setTwice_canGetLatestValue() {
212+
deviceCacheManager.setContext(context);
213+
fakeScheduledExecutorService.runAll();
191214
deviceCacheManager.setValue("some_key", 1.01f);
192215
assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(1.01f);
193216

@@ -196,36 +219,42 @@ public void setValueFloat_setTwice_canGetLatestValue() {
196219
}
197220

198221
@Test
199-
public void setValueFloat_contextNotSet_canGetValue() {
222+
public void setValueFloat_contextNotSet_returnsEmpty() {
200223
deviceCacheManager.setValue("some_key", 100.0f);
201-
assertThat(deviceCacheManager.getFloat("some_key").get()).isEqualTo(100.0f);
224+
assertThat(deviceCacheManager.getFloat("some_key").isAvailable()).isFalse();
202225
}
203226

204227
@Test
205228
public void setValueFloat_keyIsNull_returnsFalse() {
229+
deviceCacheManager.setContext(context);
230+
fakeScheduledExecutorService.runAll();
206231
assertThat(deviceCacheManager.setValue(null, 10.0f)).isFalse();
207232
}
208233

209234
@Test
210235
public void getLong_valueIsNotSet_returnsEmpty() {
211236
DeviceCacheManager.clearInstance();
212-
deviceCacheManager = DeviceCacheManager.getInstance();
237+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
213238
deviceCacheManager.setContext(context);
239+
fakeScheduledExecutorService.runAll();
214240

241+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
215242
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
216243
}
217244

218245
@Test
219246
public void getLong_contextAndValueNotSet_returnsEmpty() {
220247
DeviceCacheManager.clearInstance();
221-
deviceCacheManager = DeviceCacheManager.getInstance();
248+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
222249

250+
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
223251
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
224252
}
225253

226254
@Test
227255
public void getLong_valueIsSet_returnsSetValue() {
228256
deviceCacheManager.setContext(context);
257+
fakeScheduledExecutorService.runAll();
229258
deviceCacheManager.setValue("some_key", 1L);
230259

231260
assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(1L);
@@ -235,14 +264,16 @@ public void getLong_valueIsSet_returnsSetValue() {
235264
public void getLong_firebaseAppNotExist_returnsEmpty() {
236265
DeviceCacheManager.clearInstance();
237266
FirebaseApp.clearInstancesForTest();
238-
deviceCacheManager = DeviceCacheManager.getInstance();
267+
deviceCacheManager = new DeviceCacheManager(fakeScheduledExecutorService);
239268
deviceCacheManager.setValue("some_key", 1L);
240269

241270
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
242271
}
243272

244273
@Test
245274
public void setValueLong_setTwice_canGetLatestValue() {
275+
deviceCacheManager.setContext(context);
276+
fakeScheduledExecutorService.runAll();
246277
deviceCacheManager.setValue("some_key", 2L);
247278
assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(2L);
248279

@@ -251,13 +282,16 @@ public void setValueLong_setTwice_canGetLatestValue() {
251282
}
252283

253284
@Test
254-
public void setValueLong_contextNotSet_canGetValue() {
285+
public void setValueLong_contextNotSet_returnsEmpty() {
255286
deviceCacheManager.setValue("some_key", 100L);
256-
assertThat(deviceCacheManager.getLong("some_key").get()).isEqualTo(100L);
287+
// The key is not set if the shared preference is not fetched and available.
288+
assertThat(deviceCacheManager.getLong("some_key").isAvailable()).isFalse();
257289
}
258290

259291
@Test
260292
public void setValueLong_keyIsNull_returnsFalse() {
293+
deviceCacheManager.setContext(context);
294+
fakeScheduledExecutorService.runAll();
261295
assertThat(deviceCacheManager.setValue(null, 10.0f)).isFalse();
262296
}
263297
}

0 commit comments

Comments
 (0)