Skip to content

Commit e2e08a8

Browse files
rosalyntanrosalyntan
authored
Move continuations off main thread for App Check SDKs. (#4453)
* Move continuations off main thread for `firebase-appcheck`. * One more continuation in `firebase-appcheck`. * Move continuations off main thread for `firebase-appcheck-debug`. * Rearrange executor order. * Move continuations off main thread for `firebase-appcheck-safetynet`. * Move continuations off main thread for `firebase-appcheck-playintegrity`. * Update changelog. * Fix `SafetyNetAppCheckProviderTest`. * Address review comments.
1 parent ffe046b commit e2e08a8

File tree

23 files changed

+183
-123
lines changed

23 files changed

+183
-123
lines changed

‎appcheck/firebase-appcheck-debug/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Unreleased
22
* [changed] Migrated [app_check] SDKs to use standard Firebase executors. (#4431, #4449)
33
* [changed] Integrated the [app_check] Debug SDK with Firebase Components. (#4436)
4+
* [changed] Moved Task continuations off the main thread. (#4453)
45

56
# 16.1.0
67
* [unchanged] Updated to accommodate the release of the updated

‎appcheck/firebase-appcheck-debug/src/main/java/com/google/firebase/appcheck/debug/FirebaseAppCheckDebugRegistrar.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.debug.internal.DebugAppCheckProvider;
2223
import com.google.firebase.components.Component;
2324
import com.google.firebase.components.ComponentRegistrar;
@@ -40,6 +41,7 @@ public class FirebaseAppCheckDebugRegistrar implements ComponentRegistrar {
4041

4142
@Override
4243
public List<Component<?>> getComponents() {
44+
Qualified<Executor> liteExecutor = Qualified.qualified(Lightweight.class, Executor.class);
4345
Qualified<Executor> backgroundExecutor = Qualified.qualified(Background.class, Executor.class);
4446
Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);
4547

@@ -48,13 +50,15 @@ public List<Component<?>> getComponents() {
4850
.name(LIBRARY_NAME)
4951
.add(Dependency.required(FirebaseApp.class))
5052
.add(Dependency.optionalProvider(InternalDebugSecretProvider.class))
53+
.add(Dependency.required(liteExecutor))
5154
.add(Dependency.required(backgroundExecutor))
5255
.add(Dependency.required(blockingExecutor))
5356
.factory(
5457
(container) ->
5558
new DebugAppCheckProvider(
5659
container.get(FirebaseApp.class),
5760
container.getProvider(InternalDebugSecretProvider.class),
61+
container.get(liteExecutor),
5862
container.get(backgroundExecutor),
5963
container.get(blockingExecutor)))
6064
.build(),

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

Lines changed: 14 additions & 15 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 android.util.Log;
2120
import androidx.annotation.NonNull;
2221
import androidx.annotation.VisibleForTesting;
@@ -26,6 +25,7 @@
2625
import com.google.firebase.FirebaseApp;
2726
import com.google.firebase.annotations.concurrent.Background;
2827
import com.google.firebase.annotations.concurrent.Blocking;
28+
import com.google.firebase.annotations.concurrent.Lightweight;
2929
import com.google.firebase.appcheck.AppCheckProvider;
3030
import com.google.firebase.appcheck.AppCheckToken;
3131
import com.google.firebase.appcheck.debug.InternalDebugSecretProvider;
@@ -42,17 +42,20 @@ public class DebugAppCheckProvider implements AppCheckProvider {
4242
private static final String UTF_8 = "UTF-8";
4343

4444
private final NetworkClient networkClient;
45+
private final Executor liteExecutor;
4546
private final Executor blockingExecutor;
4647
private final RetryManager retryManager;
4748
private final Task<String> debugSecretTask;
4849

4950
public DebugAppCheckProvider(
5051
@NonNull FirebaseApp firebaseApp,
5152
@NonNull Provider<InternalDebugSecretProvider> debugSecretProvider,
53+
@Lightweight Executor liteExecutor,
5254
@Background Executor backgroundExecutor,
5355
@Blocking Executor blockingExecutor) {
5456
checkNotNull(firebaseApp);
5557
this.networkClient = new NetworkClient(firebaseApp);
58+
this.liteExecutor = liteExecutor;
5659
this.blockingExecutor = blockingExecutor;
5760
this.retryManager = new RetryManager();
5861

@@ -70,9 +73,11 @@ public DebugAppCheckProvider(
7073
DebugAppCheckProvider(
7174
@NonNull String debugSecret,
7275
@NonNull NetworkClient networkClient,
76+
@NonNull Executor liteExecutor,
7377
@NonNull Executor blockingExecutor,
7478
@NonNull RetryManager retryManager) {
7579
this.networkClient = networkClient;
80+
this.liteExecutor = liteExecutor;
7681
this.blockingExecutor = blockingExecutor;
7782
this.retryManager = retryManager;
7883
this.debugSecretTask = Tasks.forResult(debugSecret);
@@ -102,15 +107,14 @@ static Task<String> determineDebugSecret(
102107
return taskCompletionSource.getTask();
103108
}
104109

105-
// TODO(b/261013814): Use an explicit executor in continuations.
106-
@SuppressLint("TaskMainThread")
107110
@NonNull
108111
@Override
109112
public Task<AppCheckToken> getToken() {
110113
return debugSecretTask
111-
.continueWithTask(
112-
task -> {
113-
ExchangeDebugTokenRequest request = new ExchangeDebugTokenRequest(task.getResult());
114+
.onSuccessTask(
115+
liteExecutor,
116+
debugSecret -> {
117+
ExchangeDebugTokenRequest request = new ExchangeDebugTokenRequest(debugSecret);
114118
return Tasks.call(
115119
blockingExecutor,
116120
() ->
@@ -119,14 +123,9 @@ public Task<AppCheckToken> getToken() {
119123
NetworkClient.DEBUG,
120124
retryManager));
121125
})
122-
.continueWithTask(
123-
task -> {
124-
if (task.isSuccessful()) {
125-
return Tasks.forResult(
126-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
127-
}
128-
// TODO: Surface more error details.
129-
return Tasks.forException(task.getException());
130-
});
126+
.onSuccessTask(
127+
liteExecutor,
128+
response ->
129+
Tasks.forResult(DefaultAppCheckToken.constructFromAppCheckTokenResponse(response)));
131130
}
132131
}

‎appcheck/firebase-appcheck-debug/src/test/java/com/google/firebase/appcheck/debug/FirebaseAppCheckDebugRegistrarTest.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;
@@ -42,6 +43,7 @@ public void testGetComponents() {
4243
.containsExactly(
4344
Dependency.required(FirebaseApp.class),
4445
Dependency.optionalProvider(InternalDebugSecretProvider.class),
46+
Dependency.required(Qualified.qualified(Lightweight.class, Executor.class)),
4547
Dependency.required(Qualified.qualified(Background.class, Executor.class)),
4648
Dependency.required(Qualified.qualified(Blocking.class, Executor.class)));
4749
assertThat(appCheckDebugComponent.isLazy()).isTrue();

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ public class DebugAppCheckProviderTest {
7474
private StorageHelper storageHelper;
7575
private SharedPreferences sharedPreferences;
7676
// TODO(b/258273630): Use TestOnlyExecutors instead of MoreExecutors.directExecutor().
77-
private Executor executor = MoreExecutors.directExecutor();
77+
private Executor liteExecutor = MoreExecutors.directExecutor();
78+
private Executor backgroundExecutor = MoreExecutors.directExecutor();
79+
private Executor blockingExecutor = MoreExecutors.directExecutor();
7880

7981
@Before
8082
public void setup() {
@@ -103,7 +105,11 @@ public void testPublicConstructor_nullFirebaseApp_expectThrows() {
103105
NullPointerException.class,
104106
() -> {
105107
new DebugAppCheckProvider(
106-
null, null, TestOnlyExecutors.background(), TestOnlyExecutors.blocking());
108+
null,
109+
null,
110+
TestOnlyExecutors.lite(),
111+
TestOnlyExecutors.background(),
112+
TestOnlyExecutors.blocking());
107113
});
108114
}
109115

@@ -113,7 +119,7 @@ public void testDetermineDebugSecret_noStoredSecret_createsNewSecret() {
113119
assertThat(storageHelper.retrieveDebugSecret()).isNull();
114120

115121
Task<String> debugSecretTask =
116-
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, executor);
122+
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, backgroundExecutor);
117123
assertThat(storageHelper.retrieveDebugSecret()).isNotNull();
118124
assertThat(storageHelper.retrieveDebugSecret()).isEqualTo(debugSecretTask.getResult());
119125
}
@@ -123,7 +129,7 @@ public void testDetermineDebugSecret_storedSecret_usesExistingSecret() {
123129
storageHelper.saveDebugSecret(DEBUG_SECRET);
124130

125131
Task<String> debugSecretTask =
126-
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, executor);
132+
DebugAppCheckProvider.determineDebugSecret(mockFirebaseApp, backgroundExecutor);
127133
assertThat(debugSecretTask.getResult()).isEqualTo(DEBUG_SECRET);
128134
assertThat(storageHelper.retrieveDebugSecret()).isEqualTo(DEBUG_SECRET);
129135
}
@@ -137,7 +143,8 @@ public void exchangeDebugToken_onSuccess_setsTaskResult() throws Exception {
137143
when(mockAppCheckTokenResponse.getTimeToLive()).thenReturn(TIME_TO_LIVE);
138144

139145
DebugAppCheckProvider provider =
140-
new DebugAppCheckProvider(DEBUG_SECRET, mockNetworkClient, executor, mockRetryManager);
146+
new DebugAppCheckProvider(
147+
DEBUG_SECRET, mockNetworkClient, liteExecutor, blockingExecutor, mockRetryManager);
141148
Task<AppCheckToken> task = provider.getToken();
142149

143150
verify(mockNetworkClient)
@@ -155,7 +162,8 @@ public void exchangeDebugToken_onFailure_setsTaskException() throws Exception {
155162
.thenThrow(new IOException());
156163

157164
DebugAppCheckProvider provider =
158-
new DebugAppCheckProvider(DEBUG_SECRET, mockNetworkClient, executor, mockRetryManager);
165+
new DebugAppCheckProvider(
166+
DEBUG_SECRET, mockNetworkClient, liteExecutor, blockingExecutor, mockRetryManager);
159167
Task<AppCheckToken> task = provider.getToken();
160168

161169
verify(mockNetworkClient)

‎appcheck/firebase-appcheck-playintegrity/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Unreleased
22
* [changed] Migrated [app_check] SDKs to use standard Firebase executors. (#4431, #4449)
33
* [changed] Integrated the [app_check] Play Integrity SDK with Firebase Components. (#4436)
4+
* [changed] Moved Task continuations off the main thread. (#4453)
45

56
# 16.1.0
67
* [unchanged] Updated to accommodate the release of the updated

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/FirebaseAppCheckPlayIntegrityRegistrar.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.google.android.gms.common.annotation.KeepForSdk;
1818
import com.google.firebase.FirebaseApp;
1919
import com.google.firebase.annotations.concurrent.Blocking;
20+
import com.google.firebase.annotations.concurrent.Lightweight;
2021
import com.google.firebase.appcheck.playintegrity.internal.PlayIntegrityAppCheckProvider;
2122
import com.google.firebase.components.Component;
2223
import com.google.firebase.components.ComponentRegistrar;
@@ -39,17 +40,21 @@ public class FirebaseAppCheckPlayIntegrityRegistrar implements ComponentRegistra
3940

4041
@Override
4142
public List<Component<?>> getComponents() {
43+
Qualified<Executor> liteExecutor = Qualified.qualified(Lightweight.class, Executor.class);
4244
Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);
4345

4446
return Arrays.asList(
4547
Component.builder(PlayIntegrityAppCheckProvider.class)
4648
.name(LIBRARY_NAME)
4749
.add(Dependency.required(FirebaseApp.class))
50+
.add(Dependency.required(liteExecutor))
4851
.add(Dependency.required(blockingExecutor))
4952
.factory(
5053
(container) ->
5154
new PlayIntegrityAppCheckProvider(
52-
container.get(FirebaseApp.class), container.get(blockingExecutor)))
55+
container.get(FirebaseApp.class),
56+
container.get(liteExecutor),
57+
container.get(blockingExecutor)))
5358
.build(),
5459
LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME));
5560
}

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

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package com.google.firebase.appcheck.playintegrity.internal;
1616

17-
import android.annotation.SuppressLint;
1817
import androidx.annotation.NonNull;
1918
import androidx.annotation.VisibleForTesting;
2019
import com.google.android.gms.tasks.Task;
@@ -25,6 +24,7 @@
2524
import com.google.android.play.core.integrity.IntegrityTokenResponse;
2625
import com.google.firebase.FirebaseApp;
2726
import com.google.firebase.annotations.concurrent.Blocking;
27+
import com.google.firebase.annotations.concurrent.Lightweight;
2828
import com.google.firebase.appcheck.AppCheckProvider;
2929
import com.google.firebase.appcheck.AppCheckToken;
3030
import com.google.firebase.appcheck.internal.DefaultAppCheckToken;
@@ -39,15 +39,19 @@ public class PlayIntegrityAppCheckProvider implements AppCheckProvider {
3939
private final String projectNumber;
4040
private final IntegrityManager integrityManager;
4141
private final NetworkClient networkClient;
42+
private final Executor liteExecutor;
4243
private final Executor blockingExecutor;
4344
private final RetryManager retryManager;
4445

4546
public PlayIntegrityAppCheckProvider(
46-
@NonNull FirebaseApp firebaseApp, @Blocking Executor blockingExecutor) {
47+
@NonNull FirebaseApp firebaseApp,
48+
@Lightweight Executor liteExecutor,
49+
@Blocking Executor blockingExecutor) {
4750
this(
4851
firebaseApp.getOptions().getGcmSenderId(),
4952
IntegrityManagerFactory.create(firebaseApp.getApplicationContext()),
5053
new NetworkClient(firebaseApp),
54+
liteExecutor,
5155
blockingExecutor,
5256
new RetryManager());
5357
}
@@ -57,22 +61,23 @@ public PlayIntegrityAppCheckProvider(
5761
@NonNull String projectNumber,
5862
@NonNull IntegrityManager integrityManager,
5963
@NonNull NetworkClient networkClient,
64+
@NonNull Executor liteExecutor,
6065
@NonNull Executor blockingExecutor,
6166
@NonNull RetryManager retryManager) {
6267
this.projectNumber = projectNumber;
6368
this.integrityManager = integrityManager;
6469
this.networkClient = networkClient;
70+
this.liteExecutor = liteExecutor;
6571
this.blockingExecutor = blockingExecutor;
6672
this.retryManager = retryManager;
6773
}
6874

69-
// TODO(b/261013814): Use an explicit executor in continuations.
70-
@SuppressLint("TaskMainThread")
7175
@NonNull
7276
@Override
7377
public Task<AppCheckToken> getToken() {
7478
return getPlayIntegrityAttestation()
7579
.onSuccessTask(
80+
liteExecutor,
7681
integrityTokenResponse -> {
7782
ExchangePlayIntegrityTokenRequest request =
7883
new ExchangePlayIntegrityTokenRequest(integrityTokenResponse.token());
@@ -85,32 +90,30 @@ public Task<AppCheckToken> getToken() {
8590
retryManager));
8691
})
8792
.onSuccessTask(
88-
appCheckTokenResponse -> {
89-
return Tasks.forResult(
90-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(appCheckTokenResponse));
91-
});
93+
liteExecutor,
94+
appCheckTokenResponse ->
95+
Tasks.forResult(
96+
DefaultAppCheckToken.constructFromAppCheckTokenResponse(
97+
appCheckTokenResponse)));
9298
}
9399

94-
// TODO(b/261013814): Use an explicit executor in continuations.
95-
@SuppressLint("TaskMainThread")
96100
@NonNull
97101
private Task<IntegrityTokenResponse> getPlayIntegrityAttestation() {
98102
GeneratePlayIntegrityChallengeRequest generateChallengeRequest =
99103
new GeneratePlayIntegrityChallengeRequest();
100-
Task<GeneratePlayIntegrityChallengeResponse> generateChallengeTask =
101-
Tasks.call(
104+
return Tasks.call(
102105
blockingExecutor,
103106
() ->
104107
GeneratePlayIntegrityChallengeResponse.fromJsonString(
105108
networkClient.generatePlayIntegrityChallenge(
106-
generateChallengeRequest.toJsonString().getBytes(UTF_8), retryManager)));
107-
return generateChallengeTask.onSuccessTask(
108-
generatePlayIntegrityChallengeResponse -> {
109-
return integrityManager.requestIntegrityToken(
110-
IntegrityTokenRequest.builder()
111-
.setCloudProjectNumber(Long.parseLong(projectNumber))
112-
.setNonce(generatePlayIntegrityChallengeResponse.getChallenge())
113-
.build());
114-
});
109+
generateChallengeRequest.toJsonString().getBytes(UTF_8), retryManager)))
110+
.onSuccessTask(
111+
liteExecutor,
112+
generatePlayIntegrityChallengeResponse ->
113+
integrityManager.requestIntegrityToken(
114+
IntegrityTokenRequest.builder()
115+
.setCloudProjectNumber(Long.parseLong(projectNumber))
116+
.setNonce(generatePlayIntegrityChallengeResponse.getChallenge())
117+
.build()));
115118
}
116119
}

‎appcheck/firebase-appcheck-playintegrity/src/test/java/com/google/firebase/appcheck/playintegrity/FirebaseAppCheckPlayIntegrityRegistrarTest.java

Lines changed: 2 additions & 0 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.annotations.concurrent.Blocking;
21+
import com.google.firebase.annotations.concurrent.Lightweight;
2122
import com.google.firebase.components.Component;
2223
import com.google.firebase.components.Dependency;
2324
import com.google.firebase.components.Qualified;
@@ -40,6 +41,7 @@ public void testGetComponents() {
4041
assertThat(appCheckPlayIntegrityComponent.getDependencies())
4142
.containsExactly(
4243
Dependency.required(FirebaseApp.class),
44+
Dependency.required(Qualified.qualified(Lightweight.class, Executor.class)),
4345
Dependency.required(Qualified.qualified(Blocking.class, Executor.class)));
4446
assertThat(appCheckPlayIntegrityComponent.isLazy()).isTrue();
4547
}

0 commit comments

Comments
 (0)