diff --git a/firebase-inappmessaging/CHANGELOG.md b/firebase-inappmessaging/CHANGELOG.md index cae4757c2ee..9a4d46edd93 100644 --- a/firebase-inappmessaging/CHANGELOG.md +++ b/firebase-inappmessaging/CHANGELOG.md @@ -1,4 +1,5 @@ # Unreleased +* [changed] Migrate firebase-inappmessaging SDK to use common executor pool. # 20.2.0 * [fixed] Fixed a bug that prevented marking more than one message as diff --git a/firebase-inappmessaging/firebase-inappmessaging.gradle b/firebase-inappmessaging/firebase-inappmessaging.gradle index b77b49c78f9..ce3b20a2477 100644 --- a/firebase-inappmessaging/firebase-inappmessaging.gradle +++ b/firebase-inappmessaging/firebase-inappmessaging.gradle @@ -143,6 +143,7 @@ dependencies { testImplementation "io.grpc:grpc-testing:$grpcVersion" testImplementation 'com.google.guava:guava:30.1-android' testImplementation 'androidx.test:core:1.2.0' + testImplementation project(":integ-testing") androidTestImplementation project(':integ-testing') androidTestImplementation 'androidx.test.ext:junit:1.1.1' diff --git a/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingFlowableTest.java b/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingFlowableTest.java index 0d6ec9e36c2..19f6ef80add 100644 --- a/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingFlowableTest.java +++ b/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingFlowableTest.java @@ -52,6 +52,7 @@ import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; import com.google.firebase.analytics.connector.AnalyticsConnector; +import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.events.Subscriber; import com.google.firebase.inappmessaging.CommonTypesProto.Event; import com.google.firebase.inappmessaging.CommonTypesProto.Priority; @@ -65,6 +66,7 @@ import com.google.firebase.inappmessaging.internal.TestDeviceHelper; import com.google.firebase.inappmessaging.internal.injection.modules.AppMeasurementModule; import com.google.firebase.inappmessaging.internal.injection.modules.ApplicationModule; +import com.google.firebase.inappmessaging.internal.injection.modules.ExecutorsModule; import com.google.firebase.inappmessaging.internal.injection.modules.GrpcClientModule; import com.google.firebase.inappmessaging.internal.injection.modules.ProgrammaticContextualTriggerFlowableModule; import com.google.firebase.inappmessaging.model.BannerMessage; @@ -270,7 +272,9 @@ public void setUp() { .testSystemClockModule(new TestSystemClockModule(NOW)) .programmaticContextualTriggerFlowableModule( new ProgrammaticContextualTriggerFlowableModule( - new ProgramaticContextualTriggers())); + new ProgramaticContextualTriggers())) + .executorsModule( + new ExecutorsModule(TestOnlyExecutors.background(), TestOnlyExecutors.blocking())); TestUniversalComponent universalComponent = universalComponentBuilder.build(); @@ -315,6 +319,8 @@ public void onAppOpen_whenAnalyticsAbsent_notifiesSubscriber() { TestUniversalComponent analyticsLessUniversalComponent = universalComponentBuilder .appMeasurementModule(new AppMeasurementModule(handler -> {}, firebaseEventSubscriber)) + .executorsModule( + new ExecutorsModule(TestOnlyExecutors.background(), TestOnlyExecutors.blocking())) .build(); TestAppComponent appComponent = appComponentBuilder.universalComponent(analyticsLessUniversalComponent).build(); diff --git a/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/TestUniversalComponent.java b/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/TestUniversalComponent.java index 1bcfa26cd19..92a0b972968 100644 --- a/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/TestUniversalComponent.java +++ b/firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/TestUniversalComponent.java @@ -18,6 +18,7 @@ import com.google.firebase.inappmessaging.internal.injection.modules.AnalyticsEventsModule; import com.google.firebase.inappmessaging.internal.injection.modules.AppMeasurementModule; import com.google.firebase.inappmessaging.internal.injection.modules.ApplicationModule; +import com.google.firebase.inappmessaging.internal.injection.modules.ExecutorsModule; import com.google.firebase.inappmessaging.internal.injection.modules.ProgrammaticContextualTriggerFlowableModule; import com.google.firebase.inappmessaging.internal.injection.modules.ProtoStorageClientModule; import com.google.firebase.inappmessaging.internal.injection.modules.RateLimitModule; @@ -41,5 +42,6 @@ ProtoStorageClientModule.class, RateLimitModule.class, AppMeasurementModule.class, + ExecutorsModule.class, }) public interface TestUniversalComponent extends UniversalComponent {} diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingRegistrar.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingRegistrar.java index 2fe096f8fe6..d94c40228e3 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingRegistrar.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingRegistrar.java @@ -22,10 +22,13 @@ import com.google.firebase.abt.FirebaseABTesting; import com.google.firebase.abt.component.AbtComponent; import com.google.firebase.analytics.connector.AnalyticsConnector; +import com.google.firebase.annotations.concurrent.Background; +import com.google.firebase.annotations.concurrent.Blocking; import com.google.firebase.components.Component; import com.google.firebase.components.ComponentContainer; import com.google.firebase.components.ComponentRegistrar; import com.google.firebase.components.Dependency; +import com.google.firebase.components.Qualified; import com.google.firebase.events.Subscriber; import com.google.firebase.inappmessaging.internal.AbtIntegrationHelper; import com.google.firebase.inappmessaging.internal.ProgramaticContextualTriggers; @@ -37,6 +40,7 @@ import com.google.firebase.inappmessaging.internal.injection.modules.ApiClientModule; import com.google.firebase.inappmessaging.internal.injection.modules.AppMeasurementModule; import com.google.firebase.inappmessaging.internal.injection.modules.ApplicationModule; +import com.google.firebase.inappmessaging.internal.injection.modules.ExecutorsModule; import com.google.firebase.inappmessaging.internal.injection.modules.GrpcClientModule; import com.google.firebase.inappmessaging.internal.injection.modules.ProgrammaticContextualTriggerFlowableModule; import com.google.firebase.inject.Deferred; @@ -44,6 +48,7 @@ import com.google.firebase.platforminfo.LibraryVersionComponent; import java.util.Arrays; import java.util.List; +import java.util.concurrent.Executor; /** * Registers {@link FirebaseInAppMessaging}. @@ -53,6 +58,10 @@ @Keep public class FirebaseInAppMessagingRegistrar implements ComponentRegistrar { private static final String LIBRARY_NAME = "fire-fiam"; + private Qualified backgroundExecutor = + Qualified.qualified(Background.class, Executor.class); + private Qualified blockingExecutor = + Qualified.qualified(Blocking.class, Executor.class); @Override @Keep @@ -67,6 +76,8 @@ public List> getComponents() { .add(Dependency.deferred(AnalyticsConnector.class)) .add(Dependency.required(TransportFactory.class)) .add(Dependency.required(Subscriber.class)) + .add(Dependency.required(backgroundExecutor)) + .add(Dependency.required(blockingExecutor)) .factory(this::providesFirebaseInAppMessaging) .eagerInDefaultApp() .build(), @@ -91,6 +102,9 @@ private FirebaseInAppMessaging providesFirebaseInAppMessaging(ComponentContainer .programmaticContextualTriggerFlowableModule( new ProgrammaticContextualTriggerFlowableModule( new ProgramaticContextualTriggers())) + .executorsModule( + new ExecutorsModule( + container.get(backgroundExecutor), container.get(blockingExecutor))) .build(); AppComponent instance = diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java index 1927a4cd1de..988568ed1ad 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/AbtIntegrationHelper.java @@ -14,11 +14,11 @@ package com.google.firebase.inappmessaging.internal; -import android.annotation.SuppressLint; import androidx.annotation.VisibleForTesting; import com.google.firebase.abt.AbtException; import com.google.firebase.abt.AbtExperimentInfo; import com.google.firebase.abt.FirebaseABTesting; +import com.google.firebase.annotations.concurrent.Blocking; import com.google.firebase.inappmessaging.ExperimentPayloadProto; import com.google.firebase.inappmessaging.internal.injection.scopes.FirebaseAppScope; import com.google.internal.firebase.inappmessaging.v1.CampaignProto; @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.Date; import java.util.concurrent.Executor; -import java.util.concurrent.Executors; import javax.inject.Inject; /** @hide */ @@ -34,10 +33,7 @@ public class AbtIntegrationHelper { private final FirebaseABTesting abTesting; - // TODO(b/258280977): Migrate to go/firebase-android-executors - @SuppressLint("ThreadPoolCreation") - @VisibleForTesting - Executor executor = Executors.newSingleThreadExecutor(); + @Inject @Blocking @VisibleForTesting Executor executor; @Inject public AbtIntegrationHelper(FirebaseABTesting abTesting) { diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java index bc6dc381445..0e122fe981b 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManager.java @@ -14,8 +14,7 @@ package com.google.firebase.inappmessaging.internal; -import android.annotation.SuppressLint; -import androidx.annotation.NonNull; +import com.google.firebase.annotations.concurrent.Background; import com.google.firebase.inappmessaging.FirebaseInAppMessagingClickListener; import com.google.firebase.inappmessaging.FirebaseInAppMessagingDismissListener; import com.google.firebase.inappmessaging.FirebaseInAppMessagingDisplayCallbacks; @@ -25,13 +24,7 @@ import com.google.firebase.inappmessaging.model.InAppMessage; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.BlockingQueue; import java.util.concurrent.Executor; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; /** * A class used to manage and schedule events to registered (ie: developer-defined) or expensive @@ -42,11 +35,7 @@ @SuppressWarnings("JavaDoc") public class DeveloperListenerManager { - // We limit to 1 so there is minimial impact to device performance - private static final int POOL_SIZE = 1; - // Keep alive to minimize chance of having to restart a thread to handle both impression and click - private static final int KEEP_ALIVE_TIME_SECONDS = 15; - public static DeveloperListenerManager instance = new DeveloperListenerManager(); + private final Executor backgroundExecutor; private Map registeredClickListeners = new HashMap<>(); private Map @@ -56,28 +45,15 @@ public class DeveloperListenerManager { private Map registeredImpressionListeners = new HashMap<>(); - private static BlockingQueue mCallbackQueue = new LinkedBlockingQueue<>(); - - // TODO(b/258280977): Migrate to go/firebase-android-executors - @SuppressLint("ThreadPoolCreation") - private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR = - new ThreadPoolExecutor( - POOL_SIZE, - POOL_SIZE, - KEEP_ALIVE_TIME_SECONDS, - TimeUnit.SECONDS, - mCallbackQueue, - new FIAMThreadFactory("EventListeners-")); - - static { - CALLBACK_QUEUE_EXECUTOR.allowCoreThreadTimeOut(true); + public DeveloperListenerManager(@Background Executor backgroundExecutor) { + this.backgroundExecutor = backgroundExecutor; } // Used internally by MetricsLoggerClient public void impressionDetected(InAppMessage inAppMessage) { for (ImpressionExecutorAndListener listener : registeredImpressionListeners.values()) { listener - .withExecutor(CALLBACK_QUEUE_EXECUTOR) + .withExecutor(backgroundExecutor) .execute(() -> listener.getListener().impressionDetected(inAppMessage)); } } @@ -87,7 +63,7 @@ public void displayErrorEncountered( FirebaseInAppMessagingDisplayCallbacks.InAppMessagingErrorReason errorReason) { for (ErrorsExecutorAndListener listener : registeredErrorListeners.values()) { listener - .withExecutor(CALLBACK_QUEUE_EXECUTOR) + .withExecutor(backgroundExecutor) .execute(() -> listener.getListener().displayErrorEncountered(inAppMessage, errorReason)); } } @@ -95,7 +71,7 @@ public void displayErrorEncountered( public void messageClicked(InAppMessage inAppMessage, Action action) { for (ClicksExecutorAndListener listener : registeredClickListeners.values()) { listener - .withExecutor(CALLBACK_QUEUE_EXECUTOR) + .withExecutor(backgroundExecutor) .execute(() -> listener.getListener().messageClicked(inAppMessage, action)); } } @@ -103,7 +79,7 @@ public void messageClicked(InAppMessage inAppMessage, Action action) { public void messageDismissed(InAppMessage inAppMessage) { for (DismissExecutorAndListener listener : registeredDismissListeners.values()) { listener - .withExecutor(CALLBACK_QUEUE_EXECUTOR) + .withExecutor(backgroundExecutor) .execute(() -> listener.getListener().messageDismissed(inAppMessage)); } } @@ -175,29 +151,6 @@ public void removeAllListeners() { registeredErrorListeners.clear(); } - /** The thread factory for Storage threads. */ - static class FIAMThreadFactory implements ThreadFactory { - private final AtomicInteger threadNumber = new AtomicInteger(1); - private final String mNameSuffix; - - FIAMThreadFactory(@NonNull String suffix) { - mNameSuffix = suffix; - } - - @SuppressWarnings("ThreadPriorityCheck") - @Override - // TODO(b/258280977): Migrate to go/firebase-android-executors - @SuppressLint("ThreadPoolCreation") - public Thread newThread(@NonNull Runnable r) { - Thread t = new Thread(r, "FIAM-" + mNameSuffix + threadNumber.getAndIncrement()); - t.setDaemon(false); - t.setPriority( - android.os.Process.THREAD_PRIORITY_BACKGROUND - + android.os.Process.THREAD_PRIORITY_MORE_FAVORABLE); - return t; - } - } - private abstract static class ExecutorAndListener { private final Executor executor; diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/components/UniversalComponent.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/components/UniversalComponent.java index c4f7f67cc16..1e5842a1ddf 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/components/UniversalComponent.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/components/UniversalComponent.java @@ -28,6 +28,7 @@ import com.google.firebase.inappmessaging.internal.injection.modules.AnalyticsEventsModule; import com.google.firebase.inappmessaging.internal.injection.modules.AppMeasurementModule; import com.google.firebase.inappmessaging.internal.injection.modules.ApplicationModule; +import com.google.firebase.inappmessaging.internal.injection.modules.ExecutorsModule; import com.google.firebase.inappmessaging.internal.injection.modules.ForegroundFlowableModule; import com.google.firebase.inappmessaging.internal.injection.modules.GrpcChannelModule; import com.google.firebase.inappmessaging.internal.injection.modules.ProgrammaticContextualTriggerFlowableModule; @@ -64,7 +65,8 @@ ProtoStorageClientModule.class, SystemClockModule.class, RateLimitModule.class, - AppMeasurementModule.class + AppMeasurementModule.class, + ExecutorsModule.class }) public interface UniversalComponent { ProviderInstaller probiderInstaller(); diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ApplicationModule.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ApplicationModule.java index 96d8b63e1cc..ec4c86ff337 100644 --- a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ApplicationModule.java +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ApplicationModule.java @@ -15,9 +15,11 @@ package com.google.firebase.inappmessaging.internal.injection.modules; import android.app.Application; +import com.google.firebase.annotations.concurrent.Background; import com.google.firebase.inappmessaging.internal.DeveloperListenerManager; import dagger.Module; import dagger.Provides; +import java.util.concurrent.Executor; import javax.inject.Singleton; /** @@ -41,7 +43,8 @@ public Application providesApplication() { @Provides @Singleton - public DeveloperListenerManager developerListenerManager() { - return new DeveloperListenerManager(); + public DeveloperListenerManager developerListenerManager( + @Background Executor backgroundExecutor) { + return new DeveloperListenerManager(backgroundExecutor); } } diff --git a/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ExecutorsModule.java b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ExecutorsModule.java new file mode 100644 index 00000000000..a8ae32abf04 --- /dev/null +++ b/firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ExecutorsModule.java @@ -0,0 +1,53 @@ +// Copyright 2018 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.inappmessaging.internal.injection.modules; + +import androidx.annotation.NonNull; +import com.google.firebase.annotations.concurrent.Background; +import com.google.firebase.annotations.concurrent.Blocking; +import dagger.Module; +import dagger.Provides; +import java.util.concurrent.Executor; +import javax.inject.Singleton; + +/** Provides executors for running tasks. */ +@Module +public class ExecutorsModule { + private final Executor backgroundExecutor; + private final Executor blockingExecutor; + + public ExecutorsModule( + @NonNull @Background Executor backgroundExecutor, + @NonNull @Blocking Executor blockingExecutor) { + this.backgroundExecutor = backgroundExecutor; + this.blockingExecutor = blockingExecutor; + } + + @Provides + @Singleton + @Background + @NonNull + public Executor providesBackgroundExecutor() { + return backgroundExecutor; + } + + @Provides + @Singleton + @Blocking + @NonNull + public Executor providesBlockingExecutor() { + return blockingExecutor; + } +} diff --git a/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingTest.java b/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingTest.java index 41e181dd8c3..c5c945c6439 100644 --- a/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingTest.java +++ b/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingTest.java @@ -33,6 +33,7 @@ import com.google.android.gms.tasks.Tasks; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseOptions; +import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.inappmessaging.CommonTypesProto.Event; import com.google.firebase.inappmessaging.CommonTypesProto.Priority; import com.google.firebase.inappmessaging.CommonTypesProto.TriggeringCondition; @@ -149,7 +150,11 @@ public Builder toBuilder() { @Mock private DisplayCallbacksFactory displayCallbacksFactory; @Mock private FirebaseInAppMessagingDisplayCallbacks displayCallbacks; @Mock private ProgramaticContextualTriggers programaticContextualTriggers; - @Mock DeveloperListenerManager developerListenerManager = new DeveloperListenerManager(); + + @Mock + DeveloperListenerManager developerListenerManager = + new DeveloperListenerManager(TestOnlyExecutors.background()); + FirebaseApp firebaseApp1; FirebaseOptions options; diff --git a/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManagerTest.java b/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManagerTest.java index 2bf8517f558..a8ae54c9b6d 100644 --- a/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManagerTest.java +++ b/firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/DeveloperListenerManagerTest.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import com.google.firebase.concurrent.TestOnlyExecutors; import com.google.firebase.inappmessaging.FirebaseInAppMessagingClickListener; import com.google.firebase.inappmessaging.FirebaseInAppMessagingDismissListener; import com.google.firebase.inappmessaging.FirebaseInAppMessagingDisplayCallbacks; @@ -51,7 +52,7 @@ public class DeveloperListenerManagerTest { @Before public void setup() { MockitoAnnotations.initMocks(this); - developerListenerManager = new DeveloperListenerManager(); + developerListenerManager = new DeveloperListenerManager(TestOnlyExecutors.background()); } @Test