Skip to content

Commit e6fca8b

Browse files
rosalyntanrosalyntan
committed
Move continuations off main thread for firebase-appcheck.
1 parent f133454 commit e6fca8b

File tree

4 files changed

+33
-24
lines changed

4 files changed

+33
-24
lines changed

‎appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.google.firebase.FirebaseApp;
1919
import com.google.firebase.annotations.concurrent.Background;
2020
import com.google.firebase.annotations.concurrent.Blocking;
21+
import com.google.firebase.annotations.concurrent.Lightweight;
2122
import com.google.firebase.appcheck.internal.DefaultFirebaseAppCheck;
2223
import com.google.firebase.appcheck.interop.InternalAppCheckTokenProvider;
2324
import com.google.firebase.components.Component;
@@ -45,6 +46,7 @@ public class FirebaseAppCheckRegistrar implements ComponentRegistrar {
4546
@Override
4647
public List<Component<?>> getComponents() {
4748
Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
49+
Qualified<Executor> liteExecutor = Qualified.qualified(Lightweight.class, Executor.class);
4850
Qualified<ScheduledExecutorService> blockingScheduledExecutorService =
4951
Qualified.qualified(Blocking.class, ScheduledExecutorService.class);
5052

@@ -53,6 +55,7 @@ public List<Component<?>> getComponents() {
5355
.name(LIBRARY_NAME)
5456
.add(Dependency.required(FirebaseApp.class))
5557
.add(Dependency.required(backgroundExecutor))
58+
.add(Dependency.required(liteExecutor))
5659
.add(Dependency.required(blockingScheduledExecutorService))
5760
.add(Dependency.optionalProvider(HeartBeatController.class))
5861
.factory(
@@ -61,6 +64,7 @@ public List<Component<?>> getComponents() {
6164
container.get(FirebaseApp.class),
6265
container.getProvider(HeartBeatController.class),
6366
container.get(backgroundExecutor),
67+
container.get(liteExecutor),
6468
container.get(blockingScheduledExecutorService)))
6569
.alwaysEager()
6670
.build(),

‎appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static com.google.android.gms.common.internal.Preconditions.checkNotNull;
1818

19-
import android.annotation.SuppressLint;
2019
import androidx.annotation.NonNull;
2120
import androidx.annotation.Nullable;
2221
import androidx.annotation.VisibleForTesting;
@@ -27,6 +26,7 @@
2726
import com.google.firebase.FirebaseException;
2827
import com.google.firebase.annotations.concurrent.Background;
2928
import com.google.firebase.annotations.concurrent.Blocking;
29+
import com.google.firebase.annotations.concurrent.Lightweight;
3030
import com.google.firebase.appcheck.AppCheckProvider;
3131
import com.google.firebase.appcheck.AppCheckProviderFactory;
3232
import com.google.firebase.appcheck.AppCheckToken;
@@ -52,6 +52,7 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
5252
private final StorageHelper storageHelper;
5353
private final TokenRefreshManager tokenRefreshManager;
5454
private final Executor backgroundExecutor;
55+
private final Executor liteExecutor;
5556
private final Task<Void> retrieveStoredTokenTask;
5657
private final Clock clock;
5758

@@ -63,6 +64,7 @@ public DefaultFirebaseAppCheck(
6364
@NonNull FirebaseApp firebaseApp,
6465
@NonNull Provider<HeartBeatController> heartBeatController,
6566
@Background Executor backgroundExecutor,
67+
@Lightweight Executor liteExecutor,
6668
@Blocking ScheduledExecutorService scheduledExecutorService) {
6769
checkNotNull(firebaseApp);
6870
checkNotNull(heartBeatController);
@@ -78,6 +80,7 @@ public DefaultFirebaseAppCheck(
7880
/* firebaseAppCheck= */ this,
7981
scheduledExecutorService);
8082
this.backgroundExecutor = backgroundExecutor;
83+
this.liteExecutor = liteExecutor;
8184
this.retrieveStoredTokenTask = retrieveStoredAppCheckTokenInBackground(backgroundExecutor);
8285
this.clock = new Clock.DefaultClock();
8386
}
@@ -169,12 +172,11 @@ public void removeAppCheckListener(@NonNull AppCheckListener listener) {
169172
appCheckTokenListenerList.size() + appCheckListenerList.size());
170173
}
171174

172-
// TODO(b/261013814): Use an explicit executor in continuations.
173-
@SuppressLint("TaskMainThread")
174175
@NonNull
175176
@Override
176177
public Task<AppCheckTokenResult> getToken(boolean forceRefresh) {
177178
return retrieveStoredTokenTask.continueWithTask(
179+
liteExecutor,
178180
unused -> {
179181
if (!forceRefresh && hasValidToken()) {
180182
return Tasks.forResult(
@@ -188,6 +190,7 @@ public Task<AppCheckTokenResult> getToken(boolean forceRefresh) {
188190
// TODO: Cache the in-flight task.
189191
return fetchTokenFromProvider()
190192
.continueWithTask(
193+
liteExecutor,
191194
appCheckTokenTask -> {
192195
if (appCheckTokenTask.isSuccessful()) {
193196
return Tasks.forResult(
@@ -205,12 +208,11 @@ public Task<AppCheckTokenResult> getToken(boolean forceRefresh) {
205208
});
206209
}
207210

208-
// TODO(b/261013814): Use an explicit executor in continuations.
209-
@SuppressLint("TaskMainThread")
210211
@NonNull
211212
@Override
212213
public Task<AppCheckToken> getAppCheckToken(boolean forceRefresh) {
213214
return retrieveStoredTokenTask.continueWithTask(
215+
liteExecutor,
214216
unused -> {
215217
if (!forceRefresh && hasValidToken()) {
216218
return Tasks.forResult(cachedToken);
@@ -223,26 +225,22 @@ public Task<AppCheckToken> getAppCheckToken(boolean forceRefresh) {
223225
}
224226

225227
/** Fetches an {@link AppCheckToken} via the installed {@link AppCheckProvider}. */
226-
// TODO(b/261013814): Use an explicit executor in continuations.
227-
@SuppressLint("TaskMainThread")
228228
Task<AppCheckToken> fetchTokenFromProvider() {
229229
return appCheckProvider
230230
.getToken()
231-
.continueWithTask(
232-
task -> {
233-
if (task.isSuccessful()) {
234-
AppCheckToken token = task.getResult();
235-
updateStoredToken(token);
236-
for (AppCheckListener listener : appCheckListenerList) {
237-
listener.onAppCheckTokenChanged(token);
238-
}
239-
AppCheckTokenResult tokenResult =
240-
DefaultAppCheckTokenResult.constructFromAppCheckToken(token);
241-
for (AppCheckTokenListener listener : appCheckTokenListenerList) {
242-
listener.onAppCheckTokenChanged(tokenResult);
243-
}
231+
.onSuccessTask(
232+
liteExecutor,
233+
token -> {
234+
updateStoredToken(token);
235+
for (AppCheckListener listener : appCheckListenerList) {
236+
listener.onAppCheckTokenChanged(token);
244237
}
245-
return task;
238+
AppCheckTokenResult tokenResult =
239+
DefaultAppCheckTokenResult.constructFromAppCheckToken(token);
240+
for (AppCheckTokenListener listener : appCheckTokenListenerList) {
241+
listener.onAppCheckTokenChanged(tokenResult);
242+
}
243+
return Tasks.forResult(token);
246244
});
247245
}
248246

‎appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.firebase.FirebaseApp;
2020
import com.google.firebase.annotations.concurrent.Background;
2121
import com.google.firebase.annotations.concurrent.Blocking;
22+
import com.google.firebase.annotations.concurrent.Lightweight;
2223
import com.google.firebase.components.Component;
2324
import com.google.firebase.components.Dependency;
2425
import com.google.firebase.components.Qualified;
@@ -45,6 +46,7 @@ public void testGetComponents() {
4546
.containsExactly(
4647
Dependency.required(FirebaseApp.class),
4748
Dependency.required(Qualified.qualified(Background.class, Executor.class)),
49+
Dependency.required(Qualified.qualified(Lightweight.class, Executor.class)),
4850
Dependency.required(
4951
Qualified.qualified(Blocking.class, ScheduledExecutorService.class)),
5052
Dependency.optionalProvider(HeartBeatController.class));

‎appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ public void setup() {
7777
when(mockAppCheckProviderFactory.create(any())).thenReturn(mockAppCheckProvider);
7878
when(mockAppCheckProvider.getToken()).thenReturn(Tasks.forResult(validDefaultAppCheckToken));
7979

80-
// TODO(b/258273630): Use TestOnlyExecutors.background() instead of
81-
// MoreExecutors.directExecutor().
80+
// TODO(b/258273630): Use TestOnlyExecutors instead of MoreExecutors.directExecutor().
8281
defaultFirebaseAppCheck =
8382
new DefaultFirebaseAppCheck(
8483
mockFirebaseApp,
8584
() -> mockHeartBeatController,
8685
MoreExecutors.directExecutor(),
86+
MoreExecutors.directExecutor(),
8787
TestOnlyExecutors.blocking());
8888
}
8989

@@ -96,6 +96,7 @@ public void testConstructor_nullFirebaseApp_expectThrows() {
9696
null,
9797
() -> mockHeartBeatController,
9898
TestOnlyExecutors.background(),
99+
TestOnlyExecutors.lite(),
99100
TestOnlyExecutors.blocking());
100101
});
101102
}
@@ -106,7 +107,11 @@ public void testConstructor_nullHeartBeatControllerProvider_expectThrows() {
106107
NullPointerException.class,
107108
() -> {
108109
new DefaultFirebaseAppCheck(
109-
mockFirebaseApp, null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
110+
mockFirebaseApp,
111+
null,
112+
TestOnlyExecutors.background(),
113+
TestOnlyExecutors.lite(),
114+
TestOnlyExecutors.blocking());
110115
});
111116
}
112117

0 commit comments

Comments
 (0)