Skip to content

Save token on a separate thread. #3693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 4, 2022
Merged

Save token on a separate thread. #3693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions firebase-messaging/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.firebase.messaging;

import static com.google.firebase.messaging.FcmExecutors.newFileIOExecutor;
import static com.google.firebase.messaging.FcmExecutors.newInitExecutor;
import static com.google.firebase.messaging.FcmExecutors.newTaskExecutor;
import static com.google.firebase.messaging.FcmExecutors.newTopicsSyncExecutor;
Expand Down Expand Up @@ -110,8 +111,9 @@ public class FirebaseMessaging {
private final GmsRpc gmsRpc;
private final RequestDeduplicator requestDeduplicator;
private final AutoInit autoInit;
private final Executor fileIoExecutor;
private final Executor initExecutor;
private final Executor taskExecutor;
private final Executor fileExecutor;
private final Task<TopicsSubscriber> topicsSubscriberTask;
private final Metadata metadata;

Expand Down Expand Up @@ -195,7 +197,8 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
new GmsRpc(
firebaseApp, metadata, userAgentPublisher, heartBeatInfo, firebaseInstallationsApi),
/* taskExecutor= */ newTaskExecutor(),
/* fileIoExecutor= */ newInitExecutor());
/* initExecutor= */ newInitExecutor(),
/* fileExecutor= */ newFileIOExecutor());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: how about renaming it to newFileExecutor?

}

FirebaseMessaging(
Expand All @@ -207,7 +210,8 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
Metadata metadata,
GmsRpc gmsRpc,
Executor taskExecutor,
Executor fileIoExecutor) {
Executor initExecutor,
Executor fileExecutor) {

FirebaseMessaging.transportFactory = transportFactory;

Expand All @@ -221,7 +225,8 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
this.taskExecutor = taskExecutor;
this.gmsRpc = gmsRpc;
this.requestDeduplicator = new RequestDeduplicator(taskExecutor);
this.fileIoExecutor = fileIoExecutor;
this.initExecutor = initExecutor;
this.fileExecutor = fileExecutor;

Context appContext = firebaseApp.getApplicationContext();
if (appContext instanceof Application) {
Expand All @@ -243,7 +248,7 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
});
}

fileIoExecutor.execute(
initExecutor.execute(
() -> {
if (isAutoInitEnabled()) {
startSyncIfNecessary();
Expand All @@ -257,7 +262,7 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
// During FCM instantiation, as part of the initial setup, we spin up a couple of background
// threads to handle topic syncing and proxy notification configuration.
topicsSubscriberTask.addOnSuccessListener(
fileIoExecutor,
initExecutor,
topicsSubscriber -> {
// Topics operations relay on IID for token generation, thus the sync is also
// subject to an auto-init check.
Expand All @@ -266,7 +271,7 @@ static synchronized FirebaseMessaging getInstance(@NonNull FirebaseApp firebaseA
}
});

fileIoExecutor.execute(
initExecutor.execute(
() ->
// Initializes proxy notification support for the app.
ProxyNotificationInitializer.initialize(context));
Expand Down Expand Up @@ -363,7 +368,7 @@ public boolean isNotificationDelegationEnabled() {
* @return A Task that completes when the notification delegation has been set.
*/
public Task<Void> setNotificationDelegationEnabled(boolean enable) {
return ProxyNotificationInitializer.setEnableProxyNotification(fileIoExecutor, context, enable);
return ProxyNotificationInitializer.setEnableProxyNotification(initExecutor, context, enable);
}

/**
Expand All @@ -381,7 +386,7 @@ public Task<String> getToken() {
return iid.getTokenTask();
}
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
fileIoExecutor.execute(
initExecutor.execute(
() -> {
try {
taskCompletionSource.setResult(blockingGetToken());
Expand All @@ -405,7 +410,7 @@ public Task<String> getToken() {
public Task<Void> deleteToken() {
if (iid != null) {
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
fileIoExecutor.execute(
initExecutor.execute(
() -> {
try {
iid.deleteToken(Metadata.getDefaultSenderId(firebaseApp), INSTANCE_ID_SCOPE);
Expand Down Expand Up @@ -604,7 +609,7 @@ String blockingGetToken() throws IOException {
gmsRpc
.getToken()
.onSuccessTask(
Runnable::run, // direct executor
fileExecutor,
token -> {
getStore(context)
.saveToken(
Expand Down
7 changes: 7 additions & 0 deletions ...base-messaging/src/test/java/com/google/firebase/messaging/FirebaseMessagingRoboTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ public void testGetToken() throws Exception {
new Metadata(context),
mockGmsRpc,
Runnable::run,
Runnable::run,
Runnable::run);
when(mockGmsRpc.getToken()).thenReturn(Tasks.forResult("fake_token"));

Expand All @@ -304,6 +305,7 @@ public void getToken_withFiid() throws Exception {
mock(Metadata.class),
mockGmsRpc,
Runnable::run,
Runnable::run,
Runnable::run);
when(mockFiid.getTokenTask()).thenReturn(Tasks.forResult("fake_token"));

Expand All @@ -328,6 +330,7 @@ public void testDeleteToken() throws Exception {
new Metadata(context),
mockGmsRpc,
Runnable::run,
Runnable::run,
Runnable::run);
when(mockGmsRpc.getToken()).thenReturn(Tasks.forResult("fake_token"));
when(mockGmsRpc.deleteToken()).thenReturn(Tasks.forResult(null));
Expand Down Expand Up @@ -356,6 +359,7 @@ public void deleteToken_withFiid() throws Exception {
mock(Metadata.class),
mockGmsRpc,
Runnable::run,
Runnable::run,
Runnable::run);

Task<Void> deleteTokenTask = messaging.deleteToken();
Expand All @@ -380,6 +384,7 @@ public void isProxyNotificationEnabledDefaultsToTrueForNewerDevices() {
mock(Metadata.class),
mock(GmsRpc.class),
Runnable::run,
Runnable::run,
Runnable::run);

assertThat(messaging.isNotificationDelegationEnabled()).isTrue();
Expand All @@ -398,6 +403,7 @@ public void isProxyNotificationEnabledDefaultsToFalseForOlderDevices() {
mock(Metadata.class),
mock(GmsRpc.class),
Runnable::run,
Runnable::run,
Runnable::run);

assertThat(messaging.isNotificationDelegationEnabled()).isFalse();
Expand Down Expand Up @@ -508,6 +514,7 @@ public void blockingGetToken_calledTwice_OnNewTokenInvokedOnce() throws Exceptio
new Metadata(context),
mockGmsRpc,
Runnable::run,
Runnable::run,
Runnable::run);

// Call blockingGetToken() twice. Since GmsRpc.getToken() is blocked, neither call should see a
Expand Down