Skip to content

Commit 2f8e8ac

Browse files
JasonAHeronJasonAHeron
authored
Fix FIAM ANR Issue 1430 (#1488)
Fix FIAM ANR caused by FIAM or IID not being available by correctly offloading work to IO thread
1 parent 1807529 commit 2f8e8ac

File tree

8 files changed

+203
-233
lines changed

8 files changed

+203
-233
lines changed

‎firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/TestApiClientModule.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,7 @@ TestDeviceHelper providesTestDeviceHelper() {
7878
@Provides
7979
@FirebaseAppScope
8080
ApiClient providesApiClient(
81-
Lazy<GrpcClient> grpcClient,
82-
Application application,
83-
DataCollectionHelper dataCollectionHelper,
84-
ProviderInstaller providerInstaller) {
85-
return new ApiClient(
86-
grpcClient,
87-
firebaseApp,
88-
application,
89-
firebaseInstanceId,
90-
dataCollectionHelper,
91-
clock,
92-
providerInstaller);
81+
Lazy<GrpcClient> grpcClient, Application application, ProviderInstaller providerInstaller) {
82+
return new ApiClient(grpcClient, firebaseApp, application, clock, providerInstaller);
9383
}
9484
}

‎firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/FirebaseInAppMessaging.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ public void triggerEvent(String eventName) {
342342

343343
private void triggerInAppMessage(TriggeredInAppMessage inAppMessage) {
344344
if (this.fiamDisplay != null) {
345+
// The APIs that control the UI are going to be called on the main thread. Yay!
345346
fiamDisplay.displayMessage(
346347
inAppMessage.getInAppMessage(),
347348
displayCallbacksFactory.generateDisplayCallback(

‎firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/ApiClient.java

Lines changed: 19 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,8 @@
1919
import android.content.pm.PackageManager.NameNotFoundException;
2020
import android.os.Build.VERSION;
2121
import android.text.TextUtils;
22-
import com.google.android.gms.tasks.Task;
23-
import com.google.android.gms.tasks.Tasks;
24-
import com.google.common.annotations.VisibleForTesting;
2522
import com.google.developers.mobile.targeting.proto.ClientSignalsProto.ClientSignals;
2623
import com.google.firebase.FirebaseApp;
27-
import com.google.firebase.iid.FirebaseInstanceId;
2824
import com.google.firebase.iid.InstanceIdResult;
2925
import com.google.firebase.inappmessaging.internal.injection.scopes.FirebaseAppScope;
3026
import com.google.firebase.inappmessaging.internal.time.Clock;
@@ -46,71 +42,43 @@
4642
@FirebaseAppScope
4743
public class ApiClient {
4844

49-
private static final String DATA_COLLECTION_DISABLED_ERROR =
50-
"Automatic data collection is disabled, not attempting campaign fetch from service.";
5145
private static final String FETCHING_CAMPAIGN_MESSAGE = "Fetching campaigns from service.";
5246

5347
private final Lazy<GrpcClient> grpcClient;
5448
private final FirebaseApp firebaseApp;
5549
private final Application application;
56-
private final FirebaseInstanceId firebaseInstanceId;
57-
private final DataCollectionHelper dataCollectionHelper;
5850
private final Clock clock;
5951
private final ProviderInstaller providerInstaller;
6052

6153
public ApiClient(
6254
Lazy<GrpcClient> grpcClient,
6355
FirebaseApp firebaseApp,
6456
Application application,
65-
FirebaseInstanceId firebaseInstanceId,
66-
DataCollectionHelper dataCollectionHelper,
6757
Clock clock,
6858
ProviderInstaller providerInstaller) {
6959
this.grpcClient = grpcClient;
7060
this.firebaseApp = firebaseApp;
7161
this.application = application;
72-
this.firebaseInstanceId = firebaseInstanceId;
73-
this.dataCollectionHelper = dataCollectionHelper;
7462
this.clock = clock;
7563
this.providerInstaller = providerInstaller;
7664
}
7765

78-
@VisibleForTesting
79-
static FetchEligibleCampaignsResponse createCacheExpiringResponse() {
80-
// Within the cache, we use '0' as a special case to 'never' expire. '1' is used when we want to
81-
// retry the getFiams call on subsequent event triggers, and force the cache to always expire
82-
return FetchEligibleCampaignsResponse.newBuilder().setExpirationEpochTimestampMillis(1).build();
83-
}
84-
85-
Task<FetchEligibleCampaignsResponse> getFiams(CampaignImpressionList impressionList) {
86-
if (!dataCollectionHelper.isAutomaticDataCollectionEnabled()) {
87-
Logging.logi(DATA_COLLECTION_DISABLED_ERROR);
88-
return Tasks.forResult(createCacheExpiringResponse());
89-
}
66+
FetchEligibleCampaignsResponse getFiams(
67+
InstanceIdResult instanceIdResult, CampaignImpressionList impressionList) {
9068
Logging.logi(FETCHING_CAMPAIGN_MESSAGE);
9169
providerInstaller.install();
92-
return firebaseInstanceId
93-
.getInstanceId()
94-
.continueWith(
95-
instanceIdResultTask -> {
96-
InstanceIdResult instanceIdResult = instanceIdResultTask.getResult();
97-
if (instanceIdResult == null) {
98-
Logging.logw("InstanceID is null, not calling backend");
99-
return createCacheExpiringResponse();
100-
}
101-
return withCacheExpirationSafeguards(
102-
grpcClient
103-
.get()
104-
.fetchEligibleCampaigns(
105-
FetchEligibleCampaignsRequest.newBuilder()
106-
// The project Id we expect is the gcm sender id
107-
.setProjectNumber(firebaseApp.getOptions().getGcmSenderId())
108-
.addAllAlreadySeenCampaigns(
109-
impressionList.getAlreadySeenCampaignsList())
110-
.setClientSignals(getClientSignals())
111-
.setRequestingClientApp(getClientAppInfo(instanceIdResult))
112-
.build()));
113-
});
70+
71+
return withCacheExpirationSafeguards(
72+
grpcClient
73+
.get()
74+
.fetchEligibleCampaigns(
75+
FetchEligibleCampaignsRequest.newBuilder()
76+
// The project Id we expect is the gcm sender id
77+
.setProjectNumber(firebaseApp.getOptions().getGcmSenderId())
78+
.addAllAlreadySeenCampaigns(impressionList.getAlreadySeenCampaignsList())
79+
.setClientSignals(getClientSignals())
80+
.setRequestingClientApp(getClientAppInfo(instanceIdResult))
81+
.build()));
11482
}
11583

11684
private FetchEligibleCampaignsResponse withCacheExpirationSafeguards(
@@ -143,17 +111,11 @@ private ClientSignals getClientSignals() {
143111
}
144112

145113
private ClientAppInfo getClientAppInfo(InstanceIdResult instanceIdResult) {
146-
ClientAppInfo.Builder builder =
147-
ClientAppInfo.newBuilder().setGmpAppId(firebaseApp.getOptions().getApplicationId());
148-
String instanceId = instanceIdResult.getId();
149-
String instanceToken = instanceIdResult.getToken();
150-
if (!TextUtils.isEmpty(instanceId) && !TextUtils.isEmpty(instanceToken)) {
151-
builder.setAppInstanceId(instanceId);
152-
builder.setAppInstanceIdToken(instanceToken);
153-
} else {
154-
Logging.logw("Empty instance ID or instance token");
155-
}
156-
return builder.build();
114+
return ClientAppInfo.newBuilder()
115+
.setGmpAppId(firebaseApp.getOptions().getApplicationId())
116+
.setAppInstanceId(instanceIdResult.getId())
117+
.setAppInstanceIdToken(instanceIdResult.getToken())
118+
.build();
157119
}
158120

159121
@Nullable

‎firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/GrpcClient.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.google.internal.firebase.inappmessaging.v1.sdkserving.FetchEligibleCampaignsRequest;
1919
import com.google.internal.firebase.inappmessaging.v1.sdkserving.FetchEligibleCampaignsResponse;
2020
import com.google.internal.firebase.inappmessaging.v1.sdkserving.InAppMessagingSdkServingGrpc.InAppMessagingSdkServingBlockingStub;
21+
import java.util.concurrent.TimeUnit;
2122
import javax.inject.Inject;
2223

2324
/**
@@ -35,6 +36,6 @@ public class GrpcClient {
3536
}
3637

3738
public FetchEligibleCampaignsResponse fetchEligibleCampaigns(FetchEligibleCampaignsRequest req) {
38-
return stub.fetchEligibleCampaigns(req);
39+
return stub.withDeadlineAfter(30000, TimeUnit.MILLISECONDS).fetchEligibleCampaigns(req);
3940
}
4041
}

‎firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/InAppMessageStreamManager.java

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414

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

17+
import android.text.TextUtils;
18+
import androidx.annotation.VisibleForTesting;
1719
import com.google.android.gms.tasks.Task;
20+
import com.google.firebase.iid.FirebaseInstanceId;
21+
import com.google.firebase.iid.InstanceIdResult;
1822
import com.google.firebase.inappmessaging.CommonTypesProto.TriggeringCondition;
1923
import com.google.firebase.inappmessaging.internal.injection.qualifiers.AppForeground;
2024
import com.google.firebase.inappmessaging.internal.injection.qualifiers.ProgrammaticTrigger;
@@ -36,6 +40,7 @@
3640
import io.reactivex.functions.Consumer;
3741
import io.reactivex.functions.Function;
3842
import java.util.Locale;
43+
import javax.annotation.Nullable;
3944
import javax.inject.Inject;
4045

4146
/**
@@ -59,6 +64,8 @@ public class InAppMessageStreamManager {
5964
private final AnalyticsEventsManager analyticsEventsManager;
6065
private final TestDeviceHelper testDeviceHelper;
6166
private final AbtIntegrationHelper abtIntegrationHelper;
67+
private final FirebaseInstanceId firebaseInstanceId;
68+
private final DataCollectionHelper dataCollectionHelper;
6269

6370
@Inject
6471
public InAppMessageStreamManager(
@@ -73,6 +80,8 @@ public InAppMessageStreamManager(
7380
RateLimiterClient rateLimiterClient,
7481
@AppForeground RateLimit appForegroundRateLimit,
7582
TestDeviceHelper testDeviceHelper,
83+
FirebaseInstanceId firebaseInstanceId,
84+
DataCollectionHelper dataCollectionHelper,
7685
AbtIntegrationHelper abtIntegrationHelper) {
7786
this.appForegroundEventFlowable = appForegroundEventFlowable;
7887
this.programmaticTriggerEventFlowable = programmaticTriggerEventFlowable;
@@ -85,6 +94,8 @@ public InAppMessageStreamManager(
8594
this.rateLimiterClient = rateLimiterClient;
8695
this.appForegroundRateLimit = appForegroundRateLimit;
8796
this.testDeviceHelper = testDeviceHelper;
97+
this.dataCollectionHelper = dataCollectionHelper;
98+
this.firebaseInstanceId = firebaseInstanceId;
8899
this.abtIntegrationHelper = abtIntegrationHelper;
89100
}
90101

@@ -232,22 +243,35 @@ public Flowable<TriggeredInAppMessage> createFirebaseInAppMessageStream() {
232243
.defaultIfEmpty(CampaignImpressionList.getDefaultInstance())
233244
.onErrorResumeNext(Maybe.just(CampaignImpressionList.getDefaultInstance()));
234245

246+
Maybe<InstanceIdResult> getIID = taskToMaybe(firebaseInstanceId.getInstanceId());
247+
235248
Function<CampaignImpressionList, Maybe<FetchEligibleCampaignsResponse>> serviceFetch =
236-
impressions ->
237-
taskToMaybe(apiClient.getFiams(impressions))
238-
.doOnSuccess(
239-
resp ->
240-
Logging.logi(
241-
String.format(
242-
Locale.US,
243-
"Successfully fetched %d messages from backend",
244-
resp.getMessagesList().size())))
245-
.doOnSuccess(
246-
resp -> impressionStorageClient.clearImpressions(resp).subscribe())
247-
.doOnSuccess(analyticsEventsManager::updateContextualTriggers)
248-
.doOnSuccess(testDeviceHelper::processCampaignFetch)
249-
.doOnError(e -> Logging.logw("Service fetch error: " + e.getMessage()))
250-
.onErrorResumeNext(Maybe.empty()); // Absorb service failures
249+
campaignImpressionList -> {
250+
if (!dataCollectionHelper.isAutomaticDataCollectionEnabled()) {
251+
Logging.logi(
252+
"Automatic data collection is disabled, not attempting campaign fetch from service.");
253+
return Maybe.just(cacheExpiringResponse());
254+
}
255+
256+
// blocking get occurs on the IO thread because that's what we observeOn above
257+
return Maybe.fromCallable(getIID::blockingGet)
258+
.filter(InAppMessageStreamManager::validIID)
259+
.map(iid -> apiClient.getFiams(iid, campaignImpressionList))
260+
.switchIfEmpty(Maybe.just(cacheExpiringResponse()))
261+
.doOnSuccess(
262+
resp ->
263+
Logging.logi(
264+
String.format(
265+
Locale.US,
266+
"Successfully fetched %d messages from backend",
267+
resp.getMessagesList().size())))
268+
.doOnSuccess(
269+
resp -> impressionStorageClient.clearImpressions(resp).subscribe())
270+
.doOnSuccess(analyticsEventsManager::updateContextualTriggers)
271+
.doOnSuccess(testDeviceHelper::processCampaignFetch)
272+
.doOnError(e -> Logging.logw("Service fetch error: " + e.getMessage()))
273+
.onErrorResumeNext(Maybe.empty()); // Absorb service failures
274+
};
251275

252276
if (shouldIgnoreCache(event)) {
253277
Logging.logi(
@@ -349,6 +373,17 @@ private Maybe<TriggeredInAppMessage> triggeredInAppMessage(ThickContent content,
349373
return Maybe.just(new TriggeredInAppMessage(inAppMessage, event));
350374
}
351375

376+
private static boolean validIID(@Nullable InstanceIdResult iid) {
377+
return iid != null && !TextUtils.isEmpty(iid.getId()) && !TextUtils.isEmpty(iid.getToken());
378+
}
379+
380+
@VisibleForTesting
381+
static FetchEligibleCampaignsResponse cacheExpiringResponse() {
382+
// Within the cache, we use '0' as a special case to 'never' expire. '1' is used when we want to
383+
// retry the getFiams call on subsequent event triggers, and force the cache to always expire
384+
return FetchEligibleCampaignsResponse.newBuilder().setExpirationEpochTimestampMillis(1).build();
385+
}
386+
352387
private static <T> Maybe<T> taskToMaybe(Task<T> task) {
353388
return Maybe.create(
354389
emitter -> {

‎firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/injection/modules/ApiClientModule.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,7 @@ TestDeviceHelper providesTestDeviceHelper(SharedPreferencesUtils sharedPreferenc
7777
@Provides
7878
@FirebaseAppScope
7979
ApiClient providesApiClient(
80-
Lazy<GrpcClient> grpcClient,
81-
Application application,
82-
DataCollectionHelper dataCollectionHelper,
83-
ProviderInstaller providerInstaller) {
84-
return new ApiClient(
85-
grpcClient,
86-
firebaseApp,
87-
application,
88-
firebaseInstanceId,
89-
dataCollectionHelper,
90-
clock,
91-
providerInstaller);
80+
Lazy<GrpcClient> grpcClient, Application application, ProviderInstaller providerInstaller) {
81+
return new ApiClient(grpcClient, firebaseApp, application, clock, providerInstaller);
9282
}
9383
}

0 commit comments

Comments
 (0)