Skip to content

Commit e461cb7

Browse files
vkryachkovkryachko
authored
Add lint check that detects UI thread continuations. (#4363)
* Add lint check that detects UI thread continuations. * Suppress lint failures pending actual fixes by product teams.
1 parent 012ccea commit e461cb7

File tree

29 files changed

+240
-83
lines changed

29 files changed

+240
-83
lines changed

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

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@
2121
import androidx.annotation.NonNull;
2222
import androidx.annotation.Nullable;
2323
import androidx.annotation.VisibleForTesting;
24-
import com.google.android.gms.tasks.Continuation;
2524
import com.google.android.gms.tasks.Task;
2625
import com.google.android.gms.tasks.TaskCompletionSource;
2726
import com.google.android.gms.tasks.Tasks;
2827
import com.google.firebase.FirebaseApp;
2928
import com.google.firebase.appcheck.AppCheckProvider;
3029
import com.google.firebase.appcheck.AppCheckToken;
31-
import com.google.firebase.appcheck.internal.AppCheckTokenResponse;
3230
import com.google.firebase.appcheck.internal.DefaultAppCheckToken;
3331
import com.google.firebase.appcheck.internal.NetworkClient;
3432
import com.google.firebase.appcheck.internal.RetryManager;
@@ -95,35 +93,31 @@ static Task<String> determineDebugSecret(
9593
return taskCompletionSource.getTask();
9694
}
9795

96+
// TODO(b/261013814): Use an explicit executor in continuations.
97+
@SuppressLint("TaskMainThread")
9898
@NonNull
9999
@Override
100100
public Task<AppCheckToken> getToken() {
101101
return debugSecretTask
102102
.continueWithTask(
103-
new Continuation<String, Task<AppCheckTokenResponse>>() {
104-
@Override
105-
public Task<AppCheckTokenResponse> then(@NonNull Task<String> task) throws Exception {
106-
ExchangeDebugTokenRequest request = new ExchangeDebugTokenRequest(task.getResult());
107-
return Tasks.call(
108-
backgroundExecutor,
109-
() ->
110-
networkClient.exchangeAttestationForAppCheckToken(
111-
request.toJsonString().getBytes(UTF_8),
112-
NetworkClient.DEBUG,
113-
retryManager));
114-
}
103+
task -> {
104+
ExchangeDebugTokenRequest request = new ExchangeDebugTokenRequest(task.getResult());
105+
return Tasks.call(
106+
backgroundExecutor,
107+
() ->
108+
networkClient.exchangeAttestationForAppCheckToken(
109+
request.toJsonString().getBytes(UTF_8),
110+
NetworkClient.DEBUG,
111+
retryManager));
115112
})
116113
.continueWithTask(
117-
new Continuation<AppCheckTokenResponse, Task<AppCheckToken>>() {
118-
@Override
119-
public Task<AppCheckToken> then(@NonNull Task<AppCheckTokenResponse> task) {
120-
if (task.isSuccessful()) {
121-
return Tasks.forResult(
122-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
123-
}
124-
// TODO: Surface more error details.
125-
return Tasks.forException(task.getException());
114+
task -> {
115+
if (task.isSuccessful()) {
116+
return Tasks.forResult(
117+
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
126118
}
119+
// TODO: Surface more error details.
120+
return Tasks.forException(task.getException());
127121
});
128122
}
129123
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ public PlayIntegrityAppCheckProvider(@NonNull FirebaseApp firebaseApp) {
6767
this.retryManager = retryManager;
6868
}
6969

70+
// TODO(b/261013814): Use an explicit executor in continuations.
71+
@SuppressLint("TaskMainThread")
7072
@NonNull
7173
@Override
7274
public Task<AppCheckToken> getToken() {
@@ -90,6 +92,8 @@ public Task<AppCheckToken> getToken() {
9092
});
9193
}
9294

95+
// TODO(b/261013814): Use an explicit executor in continuations.
96+
@SuppressLint("TaskMainThread")
9397
@NonNull
9498
private Task<IntegrityTokenResponse> getPlayIntegrityAttestation() {
9599
GeneratePlayIntegrityChallengeRequest generateChallengeRequest =

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

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import com.google.android.gms.safetynet.SafetyNet;
2727
import com.google.android.gms.safetynet.SafetyNetApi;
2828
import com.google.android.gms.safetynet.SafetyNetClient;
29-
import com.google.android.gms.tasks.Continuation;
3029
import com.google.android.gms.tasks.Task;
3130
import com.google.android.gms.tasks.TaskCompletionSource;
3231
import com.google.android.gms.tasks.Tasks;
@@ -140,39 +139,35 @@ Task<SafetyNetClient> getSafetyNetClientTask() {
140139
return safetyNetClientTask;
141140
}
142141

142+
// TODO(b/261013814): Use an explicit executor in continuations.
143+
@SuppressLint("TaskMainThread")
143144
@NonNull
144145
@Override
145146
public Task<AppCheckToken> getToken() {
146147
return safetyNetClientTask
147148
.continueWithTask(
148-
new Continuation<SafetyNetClient, Task<SafetyNetApi.AttestationResponse>>() {
149-
@Override
150-
public Task<SafetyNetApi.AttestationResponse> then(
151-
@NonNull Task<SafetyNetClient> task) {
152-
if (task.isSuccessful()) {
153-
return task.getResult().attest(NONCE.getBytes(), apiKey);
154-
}
155-
return Tasks.forException(task.getException());
149+
task -> {
150+
if (task.isSuccessful()) {
151+
return task.getResult().attest(NONCE.getBytes(), apiKey);
156152
}
153+
return Tasks.forException(task.getException());
157154
})
158155
.continueWithTask(
159-
new Continuation<SafetyNetApi.AttestationResponse, Task<AppCheckToken>>() {
160-
@Override
161-
public Task<AppCheckToken> then(
162-
@NonNull Task<SafetyNetApi.AttestationResponse> task) {
163-
if (!task.isSuccessful()) {
164-
// Proxies errors to the client directly; need to wrap to get the
165-
// types right.
166-
// TODO: more specific error mapping to help clients debug more
167-
// easily.
168-
return Tasks.forException(task.getException());
169-
} else {
170-
return exchangeSafetyNetAttestationResponseForToken(task.getResult());
171-
}
156+
task -> {
157+
if (!task.isSuccessful()) {
158+
// Proxies errors to the client directly; need to wrap to get the
159+
// types right.
160+
// TODO: more specific error mapping to help clients debug more
161+
// easily.
162+
return Tasks.forException(task.getException());
163+
} else {
164+
return exchangeSafetyNetAttestationResponseForToken(task.getResult());
172165
}
173166
});
174167
}
175168

169+
// TODO(b/261013814): Use an explicit executor in continuations.
170+
@SuppressLint("TaskMainThread")
176171
@NonNull
177172
Task<AppCheckToken> exchangeSafetyNetAttestationResponseForToken(
178173
@NonNull SafetyNetApi.AttestationResponse attestationResponse) {
@@ -191,16 +186,13 @@ Task<AppCheckToken> exchangeSafetyNetAttestationResponseForToken(
191186
NetworkClient.SAFETY_NET,
192187
retryManager));
193188
return networkTask.continueWithTask(
194-
new Continuation<AppCheckTokenResponse, Task<AppCheckToken>>() {
195-
@Override
196-
public Task<AppCheckToken> then(@NonNull Task<AppCheckTokenResponse> task) {
197-
if (task.isSuccessful()) {
198-
return Tasks.forResult(
199-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
200-
}
201-
// TODO: Surface more error details.
202-
return Tasks.forException(task.getException());
189+
task -> {
190+
if (task.isSuccessful()) {
191+
return Tasks.forResult(
192+
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
203193
}
194+
// TODO: Surface more error details.
195+
return Tasks.forException(task.getException());
204196
});
205197
}
206198
}

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import androidx.annotation.NonNull;
2121
import androidx.annotation.Nullable;
2222
import androidx.annotation.VisibleForTesting;
23-
import com.google.android.gms.tasks.Continuation;
2423
import com.google.android.gms.tasks.Task;
2524
import com.google.android.gms.tasks.TaskCompletionSource;
2625
import com.google.android.gms.tasks.Tasks;
@@ -176,6 +175,8 @@ public void removeAppCheckListener(@NonNull AppCheckListener listener) {
176175
appCheckTokenListenerList.size() + appCheckListenerList.size());
177176
}
178177

178+
// TODO(b/261013814): Use an explicit executor in continuations.
179+
@SuppressLint("TaskMainThread")
179180
@NonNull
180181
@Override
181182
public Task<AppCheckTokenResult> getToken(boolean forceRefresh) {
@@ -210,6 +211,8 @@ public Task<AppCheckTokenResult> getToken(boolean forceRefresh) {
210211
});
211212
}
212213

214+
// TODO(b/261013814): Use an explicit executor in continuations.
215+
@SuppressLint("TaskMainThread")
213216
@NonNull
214217
@Override
215218
public Task<AppCheckToken> getAppCheckToken(boolean forceRefresh) {
@@ -226,27 +229,26 @@ public Task<AppCheckToken> getAppCheckToken(boolean forceRefresh) {
226229
}
227230

228231
/** Fetches an {@link AppCheckToken} via the installed {@link AppCheckProvider}. */
232+
// TODO(b/261013814): Use an explicit executor in continuations.
233+
@SuppressLint("TaskMainThread")
229234
Task<AppCheckToken> fetchTokenFromProvider() {
230235
return appCheckProvider
231236
.getToken()
232237
.continueWithTask(
233-
new Continuation<AppCheckToken, Task<AppCheckToken>>() {
234-
@Override
235-
public Task<AppCheckToken> then(@NonNull Task<AppCheckToken> task) {
236-
if (task.isSuccessful()) {
237-
AppCheckToken token = task.getResult();
238-
updateStoredToken(token);
239-
for (AppCheckListener listener : appCheckListenerList) {
240-
listener.onAppCheckTokenChanged(token);
241-
}
242-
AppCheckTokenResult tokenResult =
243-
DefaultAppCheckTokenResult.constructFromAppCheckToken(token);
244-
for (AppCheckTokenListener listener : appCheckTokenListenerList) {
245-
listener.onAppCheckTokenChanged(tokenResult);
246-
}
238+
task -> {
239+
if (task.isSuccessful()) {
240+
AppCheckToken token = task.getResult();
241+
updateStoredToken(token);
242+
for (AppCheckListener listener : appCheckListenerList) {
243+
listener.onAppCheckTokenChanged(token);
244+
}
245+
AppCheckTokenResult tokenResult =
246+
DefaultAppCheckTokenResult.constructFromAppCheckToken(token);
247+
for (AppCheckTokenListener listener : appCheckTokenListenerList) {
248+
listener.onAppCheckTokenChanged(tokenResult);
247249
}
248-
return task;
249250
}
251+
return task;
250252
});
251253
}
252254

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import android.annotation.SuppressLint;
2222
import androidx.annotation.NonNull;
2323
import androidx.annotation.VisibleForTesting;
24-
import com.google.android.gms.tasks.OnFailureListener;
2524
import java.util.concurrent.Executors;
2625
import java.util.concurrent.ScheduledExecutorService;
2726
import java.util.concurrent.ScheduledFuture;
@@ -93,16 +92,12 @@ private long getNextRefreshMillis() {
9392
}
9493
}
9594

95+
// TODO(b/261013814): Use an explicit executor in continuations.
96+
@SuppressLint("TaskMainThread")
9697
private void onRefresh() {
9798
firebaseAppCheck
9899
.fetchTokenFromProvider()
99-
.addOnFailureListener(
100-
new OnFailureListener() {
101-
@Override
102-
public void onFailure(@NonNull Exception e) {
103-
scheduleRefreshAfterFailure();
104-
}
105-
});
100+
.addOnFailureListener(e -> scheduleRefreshAfterFailure());
106101
}
107102

108103
/** Cancels the in-flight scheduled refresh. */

‎firebase-appdistribution-api/src/main/java/com/google/firebase/appdistribution/internal/FirebaseAppDistributionStub.java

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

1515
package com.google.firebase.appdistribution.internal;
1616

17+
import android.annotation.SuppressLint;
1718
import android.app.Activity;
1819
import androidx.annotation.NonNull;
1920
import androidx.annotation.Nullable;
@@ -38,6 +39,8 @@
3839
* Tasks}/{@link UpdateTask UpdateTasks} with {@link
3940
* FirebaseAppDistributionException.Status#NOT_IMPLEMENTED}.
4041
*/
42+
// TODO(b/261013680): Use an explicit executor in continuations.
43+
@SuppressLint("TaskMainThread")
4144
public class FirebaseAppDistributionStub implements FirebaseAppDistribution {
4245
@NonNull
4346
@Override

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/AabUpdater.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class AabUpdater {
5353
@GuardedBy("updateAabLock")
5454
private boolean hasBeenSentToPlayForCurrentTask = false;
5555

56-
// TODO(b/258264924): Migrate to go/firebase-android-executors
56+
// TODO(b/261014422): Migrate to go/firebase-android-executors
5757
@SuppressLint("ThreadPoolCreation")
5858
AabUpdater() {
5959
this(

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/ApkUpdater.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class ApkUpdater {
6363

6464
private final Object updateTaskLock = new Object();
6565

66-
// TODO(b/258264924): Migrate to go/firebase-android-executors
66+
// TODO(b/261014422): Migrate to go/firebase-android-executors
6767
@SuppressLint("ThreadPoolCreation")
6868
public ApkUpdater(@NonNull FirebaseApp firebaseApp, @NonNull ApkInstaller apkInstaller) {
6969
this(

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
2222
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult;
2323

24+
import android.annotation.SuppressLint;
2425
import android.app.Activity;
2526
import android.app.AlertDialog;
2627
import android.content.Context;
@@ -101,6 +102,8 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
101102

102103
@Override
103104
@NonNull
105+
// TODO(b/261014422): Use an explicit executor in continuations.
106+
@SuppressLint("TaskMainThread")
104107
public UpdateTask updateIfNewReleaseAvailable() {
105108
synchronized (updateIfNewReleaseTaskLock) {
106109
if (updateIfNewReleaseAvailableIsTaskInProgress()) {
@@ -111,7 +114,6 @@ public UpdateTask updateIfNewReleaseAvailable() {
111114
remakeUpdateConfirmationDialog = false;
112115
dialogHostActivity = null;
113116
}
114-
115117
lifecycleNotifier
116118
.applyToForegroundActivityTask(this::showSignInConfirmationDialog)
117119
.onSuccessTask(unused -> signInTester())
@@ -219,6 +221,8 @@ public void signOutTester() {
219221

220222
@Override
221223
@NonNull
224+
// TODO(b/261014422): Use an explicit executor in continuations.
225+
@SuppressLint("TaskMainThread")
222226
public synchronized Task<AppDistributionRelease> checkForNewRelease() {
223227
if (cachedCheckForNewReleaseTask != null && !cachedCheckForNewReleaseTask.isComplete()) {
224228
LogWrapper.getInstance().v("Response in progress");

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private interface FidDependentJob<TResult> {
6565
private final Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider;
6666
private final TesterApiHttpClient testerApiHttpClient;
6767

68-
// TODO(b/258264924): Migrate to go/firebase-android-executors
68+
// TODO(b/261014422): Migrate to go/firebase-android-executors
6969
@SuppressLint("ThreadPoolCreation")
7070
private final Executor taskExecutor = Executors.newSingleThreadExecutor();
7171

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/NewReleaseFetcher.java

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

1717
import static com.google.firebase.appdistribution.impl.PackageInfoUtils.getPackageInfo;
1818

19+
import android.annotation.SuppressLint;
1920
import android.content.Context;
2021
import androidx.annotation.NonNull;
2122
import androidx.annotation.VisibleForTesting;
@@ -48,6 +49,8 @@ class NewReleaseFetcher {
4849
this.releaseIdentifier = releaseIdentifier;
4950
}
5051

52+
// TODO(b/261014422): Use an explicit executor in continuations.
53+
@SuppressLint("TaskMainThread")
5154
@NonNull
5255
public synchronized Task<AppDistributionReleaseInternal> checkForNewRelease() {
5356
if (cachedCheckForNewRelease != null && !cachedCheckForNewRelease.isComplete()) {

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TesterSignInManager.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
1919
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult;
2020

21+
import android.annotation.SuppressLint;
2122
import android.app.Activity;
2223
import android.content.Context;
2324
import android.content.Intent;
@@ -113,6 +114,8 @@ void onActivityResumed(Activity activity) {
113114
}
114115
}
115116

117+
// TODO(b/261014422): Use an explicit executor in continuations.
118+
@SuppressLint("TaskMainThread")
116119
@NonNull
117120
public Task<Void> signInTester() {
118121
if (signInStorage.getSignInStatus()) {

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java

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

1515
package com.google.firebase.appdistribution.impl;
1616

17+
import android.annotation.SuppressLint;
1718
import android.app.Activity;
1819
import androidx.annotation.GuardedBy;
1920
import androidx.annotation.NonNull;
@@ -34,6 +35,8 @@
3435
import java.util.concurrent.Executor;
3536

3637
/** Implementation of UpdateTask, the return type of updateApp. */
38+
// TODO(b/261013814): Use an explicit executor in continuations.
39+
@SuppressLint("TaskMainThread")
3740
class UpdateTaskImpl extends UpdateTask {
3841
@Nullable
3942
@GuardedBy("lock")

0 commit comments

Comments
 (0)