Skip to content

Commit 0fc9a70

Browse files
authored
Forbid creating ui thread Handler. (#4385)
* Forbid creating ui thread Handler. * fix perf tests * ktfmt * actually fix perf
1 parent 0955f12 commit 0fc9a70

File tree

14 files changed

+72
-19
lines changed

14 files changed

+72
-19
lines changed

firebase-common/src/main/java/com/google/firebase/concurrent/UiExecutor.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.concurrent;
1616

17+
import android.annotation.SuppressLint;
1718
import android.os.Handler;
1819
import android.os.Looper;
1920
import java.util.concurrent.Executor;
@@ -22,6 +23,8 @@
2223
public enum UiExecutor implements Executor {
2324
INSTANCE;
2425

26+
// This is the only UI handler that is allowed in Firebase SDK.
27+
@SuppressLint("ThreadPoolCreation")
2528
private static final Handler HANDLER = new Handler(Looper.getMainLooper());
2629

2730
@Override

firebase-database/src/main/java/com/google/firebase/database/android/AndroidEventTarget.java

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

1515
package com.google.firebase.database.android;
1616

17+
import android.annotation.SuppressLint;
1718
import android.os.Handler;
1819
import android.os.Looper;
1920
import com.google.firebase.database.core.EventTarget;
2021

2122
public class AndroidEventTarget implements EventTarget {
2223
private final Handler handler;
2324

25+
// TODO(b/258277572): Migrate to go/firebase-android-executors
26+
@SuppressLint("ThreadPoolCreation")
2427
public AndroidEventTarget() {
2528
this.handler = new Handler(Looper.getMainLooper());
2629
}

firebase-database/src/main/java/com/google/firebase/database/android/AndroidPlatform.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.database.android;
1616

17+
import android.annotation.SuppressLint;
1718
import android.content.Context;
1819
import android.os.Build;
1920
import android.os.Handler;
@@ -88,6 +89,8 @@ public void handleException(final Throwable e) {
8889
// Rethrow on main thread, so the application will crash
8990
// The exception might indicate that there is something seriously wrong and better crash,
9091
// than continue run in an undefined state...
92+
// TODO(b/258277572): Migrate to go/firebase-android-executors
93+
@SuppressLint("ThreadPoolCreation")
9194
Handler handler = new Handler(applicationContext.getMainLooper());
9295
handler.post(
9396
new Runnable() {

firebase-firestore/src/main/java/com/google/firebase/firestore/util/AsyncQueue.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,9 @@ public void skipDelaysForTimerId(TimerId timerId) {
522522
*/
523523
public void panic(Throwable t) {
524524
executor.shutdownNow();
525+
526+
// TODO(b/258277574): Migrate to go/firebase-android-executors
527+
@SuppressLint("ThreadPoolCreation")
525528
Handler handler = new Handler(Looper.getMainLooper());
526529
handler.post(
527530
() -> {

firebase-firestore/src/main/java/com/google/firebase/firestore/util/Util.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.firestore.util;
1616

17+
import android.annotation.SuppressLint;
1718
import android.os.Handler;
1819
import android.os.Looper;
1920
import androidx.annotation.Nullable;
@@ -206,6 +207,8 @@ public static String typeName(@Nullable Object obj) {
206207
}
207208

208209
/** Raises an exception on Android's UI Thread and crashes the end user's app. */
210+
// TODO(b/258277574): Migrate to go/firebase-android-executors
211+
@SuppressLint("ThreadPoolCreation")
209212
public static void crashMainThread(RuntimeException exception) {
210213
new Handler(Looper.getMainLooper())
211214
.post(

firebase-messaging/src/main/java/com/google/firebase/messaging/FcmLifecycleCallbacks.java

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

1616
import static com.google.firebase.messaging.Constants.TAG;
1717

18+
import android.annotation.SuppressLint;
1819
import android.app.Activity;
1920
import android.app.Application;
2021
import android.content.Intent;
@@ -34,6 +35,8 @@ class FcmLifecycleCallbacks implements Application.ActivityLifecycleCallbacks {
3435
private final Set<Intent> seenIntents =
3536
Collections.newSetFromMap(new WeakHashMap<Intent, Boolean>());
3637

38+
// TODO(b/258424124): Migrate to go/firebase-android-executors
39+
@SuppressLint("ThreadPoolCreation")
3740
@Override
3841
public void onActivityCreated(Activity createdActivity, Bundle instanceState) {
3942
Intent startingIntent = createdActivity.getIntent();

firebase-perf/firebase-perf.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ android {
9292

9393
dependencies {
9494
// Firebase Deps
95+
implementation project(':firebase-annotations')
9596
implementation project(':firebase-common')
9697
implementation project(':firebase-components')
9798
implementation project(':firebase-config')

firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfEarly.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,26 @@
1515
package com.google.firebase.perf;
1616

1717
import android.content.Context;
18-
import android.os.Handler;
19-
import android.os.Looper;
20-
import androidx.annotation.NonNull;
2118
import androidx.annotation.Nullable;
2219
import com.google.firebase.FirebaseApp;
2320
import com.google.firebase.StartupTime;
2421
import com.google.firebase.perf.application.AppStateMonitor;
2522
import com.google.firebase.perf.config.ConfigResolver;
2623
import com.google.firebase.perf.metrics.AppStartTrace;
2724
import com.google.firebase.perf.session.SessionManager;
25+
import java.util.concurrent.Executor;
2826

2927
/**
3028
* The Firebase Performance early initialization.
3129
*
3230
* <p>Responsible for initializing the AppStartTrace, and early initialization of ConfigResolver
31+
*
32+
* @hide
3333
*/
3434
public class FirebasePerfEarly {
35-
@NonNull private final Handler mainHandler = new Handler(Looper.getMainLooper());
3635

37-
public FirebasePerfEarly(@NonNull FirebaseApp app, @Nullable StartupTime startupTime) {
36+
public FirebasePerfEarly(
37+
FirebaseApp app, @Nullable StartupTime startupTime, Executor uiExecutor) {
3838
Context context = app.getApplicationContext();
3939

4040
// Initialize ConfigResolver early for accessing device caching layer.
@@ -48,7 +48,7 @@ public FirebasePerfEarly(@NonNull FirebaseApp app, @Nullable StartupTime startup
4848
if (startupTime != null) {
4949
AppStartTrace appStartTrace = AppStartTrace.getInstance();
5050
appStartTrace.registerActivityLifecycleCallbacks(context);
51-
mainHandler.post(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
51+
uiExecutor.execute(new AppStartTrace.StartFromBackgroundRunnable(appStartTrace));
5252
}
5353

5454
// In the case of cold start, we create a session and start collecting gauges as early as

firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@
1818
import com.google.android.datatransport.TransportFactory;
1919
import com.google.firebase.FirebaseApp;
2020
import com.google.firebase.StartupTime;
21+
import com.google.firebase.annotations.concurrent.UiThread;
2122
import com.google.firebase.components.Component;
2223
import com.google.firebase.components.ComponentContainer;
2324
import com.google.firebase.components.ComponentRegistrar;
2425
import com.google.firebase.components.Dependency;
26+
import com.google.firebase.components.Qualified;
2527
import com.google.firebase.installations.FirebaseInstallationsApi;
2628
import com.google.firebase.perf.injection.components.DaggerFirebasePerformanceComponent;
2729
import com.google.firebase.perf.injection.components.FirebasePerformanceComponent;
@@ -30,6 +32,7 @@
3032
import com.google.firebase.remoteconfig.RemoteConfigComponent;
3133
import java.util.Arrays;
3234
import java.util.List;
35+
import java.util.concurrent.Executor;
3336

3437
/**
3538
* {@link com.google.firebase.components.ComponentRegistrar} for the Firebase Performance SDK.
@@ -47,6 +50,7 @@ public class FirebasePerfRegistrar implements ComponentRegistrar {
4750
@Override
4851
@Keep
4952
public List<Component<?>> getComponents() {
53+
Qualified<Executor> uiExecutor = Qualified.qualified(UiThread.class, Executor.class);
5054
return Arrays.asList(
5155
Component.builder(FirebasePerformance.class)
5256
.name(LIBRARY_NAME)
@@ -61,8 +65,14 @@ public List<Component<?>> getComponents() {
6165
.name(EARLY_LIBRARY_NAME)
6266
.add(Dependency.required(FirebaseApp.class))
6367
.add(Dependency.optionalProvider(StartupTime.class))
68+
.add(Dependency.required(uiExecutor))
6469
.eagerInDefaultApp()
65-
.factory(FirebasePerfRegistrar::providesFirebasePerformanceEarly)
70+
.factory(
71+
container ->
72+
new FirebasePerfEarly(
73+
container.get(FirebaseApp.class),
74+
container.getProvider(StartupTime.class).get(),
75+
container.get(uiExecutor)))
6676
.build(),
6777
/**
6878
* Fireperf SDK is lazily by {@link FirebasePerformanceInitializer} during {@link
@@ -89,9 +99,4 @@ private static FirebasePerformance providesFirebasePerformance(ComponentContaine
8999

90100
return component.getFirebasePerformance();
91101
}
92-
93-
private static FirebasePerfEarly providesFirebasePerformanceEarly(ComponentContainer container) {
94-
return new FirebasePerfEarly(
95-
container.get(FirebaseApp.class), container.getProvider(StartupTime.class).get());
96-
}
97102
}

firebase-perf/src/main/java/com/google/firebase/perf/util/FirstDrawDoneListener.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
package com.google.firebase.perf.util;
1515

16+
import android.annotation.SuppressLint;
1617
import android.os.Build;
1718
import android.os.Handler;
1819
import android.os.Looper;
@@ -25,7 +26,10 @@
2526
* 16+ implementation is an approximation of the initial-display-time defined by Android Vitals.
2627
*/
2728
public class FirstDrawDoneListener implements ViewTreeObserver.OnDrawListener {
29+
// TODO(b/258263016): Migrate to go/firebase-android-executors
30+
@SuppressLint("ThreadPoolCreation")
2831
private final Handler mainThreadHandler = new Handler(Looper.getMainLooper());
32+
2933
private final AtomicReference<View> viewReference;
3034
private final Runnable callback;
3135

firebase-perf/src/main/java/com/google/firebase/perf/util/PreDrawListener.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414
package com.google.firebase.perf.util;
1515

16+
import android.annotation.SuppressLint;
1617
import android.os.Handler;
1718
import android.os.Looper;
1819
import android.view.View;
@@ -24,7 +25,10 @@
2425
* OnPreDraw. This is an approximation of the initial-display time defined by Android Vitals.
2526
*/
2627
public class PreDrawListener implements ViewTreeObserver.OnPreDrawListener {
28+
// TODO(b/258263016): Migrate to go/firebase-android-executors
29+
@SuppressLint("ThreadPoolCreation")
2730
private final Handler mainThreadHandler = new Handler(Looper.getMainLooper());
31+
2832
private final AtomicReference<View> viewReference;
2933
private final Runnable callback;
3034

firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerfRegistrarTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
import com.google.android.datatransport.TransportFactory;
2020
import com.google.firebase.FirebaseApp;
2121
import com.google.firebase.StartupTime;
22+
import com.google.firebase.annotations.concurrent.UiThread;
2223
import com.google.firebase.components.Component;
2324
import com.google.firebase.components.Dependency;
25+
import com.google.firebase.components.Qualified;
2426
import com.google.firebase.installations.FirebaseInstallationsApi;
2527
import com.google.firebase.remoteconfig.RemoteConfigComponent;
2628
import java.util.List;
29+
import java.util.concurrent.Executor;
2730
import org.junit.Test;
2831
import org.junit.runner.RunWith;
2932
import org.robolectric.RobolectricTestRunner;
@@ -54,7 +57,9 @@ public void testGetComponents() {
5457

5558
assertThat(firebasePerfEarlyComponent.getDependencies())
5659
.containsExactly(
57-
Dependency.required(FirebaseApp.class), Dependency.optionalProvider(StartupTime.class));
60+
Dependency.required(Qualified.qualified(UiThread.class, Executor.class)),
61+
Dependency.required(FirebaseApp.class),
62+
Dependency.optionalProvider(StartupTime.class));
5863

5964
assertThat(firebasePerfEarlyComponent.isLazy()).isFalse();
6065
}

firebase-storage/src/main/java/com/google/firebase/storage/internal/SmartHandler.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.storage.internal;
1616

17+
import android.annotation.SuppressLint;
1718
import android.os.Handler;
1819
import android.os.Looper;
1920
import androidx.annotation.NonNull;
@@ -43,6 +44,8 @@ public class SmartHandler {
4344
/*package*/ static boolean testMode = false;
4445

4546
/** Constructs a SmartHandler */
47+
// TODO(b/258426744): Migrate to go/firebase-android-executors
48+
@SuppressLint("ThreadPoolCreation")
4649
public SmartHandler(@Nullable Executor executor) {
4750
this.executor = executor;
4851
if (this.executor == null) {

tools/lint/src/main/kotlin/ThreadPoolDetector.kt

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
4444
"java.lang.Thread",
4545
"java.util.concurrent.ForkJoinPool",
4646
"java.util.concurrent.ThreadPoolExecutor",
47-
"java.util.concurrent.ScheduledThreadPoolExecutor"
47+
"java.util.concurrent.ScheduledThreadPoolExecutor",
48+
"android.os.Handler"
4849
)
4950

5051
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
@@ -64,11 +65,23 @@ class ThreadPoolDetector : Detector(), SourceCodeScanner {
6465
node: UCallExpression,
6566
constructor: PsiMethod
6667
) {
67-
context.report(
68-
THREAD_POOL_CREATION,
69-
context.getCallLocation(node, includeReceiver = false, includeArguments = true),
70-
"Creating threads or thread pools is not allowed"
71-
)
68+
val cls = (constructor.parent as? PsiClass) ?: return
69+
if (cls.qualifiedName == "android.os.Handler") {
70+
if (node.valueArgumentCount == 0) return
71+
if (node.valueArguments[0].toString().endsWith("getMainLooper()")) {
72+
context.report(
73+
THREAD_POOL_CREATION,
74+
context.getCallLocation(node, includeReceiver = false, includeArguments = true),
75+
"Creating Ui thread loopers is not allowed, use a `@UiThread Executor` instead"
76+
)
77+
}
78+
} else {
79+
context.report(
80+
THREAD_POOL_CREATION,
81+
context.getCallLocation(node, includeReceiver = false, includeArguments = true),
82+
"Creating threads or thread pools is not allowed"
83+
)
84+
}
7285
}
7386

7487
private fun isExecutorMethod(method: PsiMethod): Boolean {

0 commit comments

Comments
 (0)