Call messageHandled after a message has been handled. (#5570)

* Called messageHandled() after a message has been handled to indicate
that the message has been handled successfully.

Author: gsakakihara

firebase-messaging/CHANGELOG.md

@@ -1,4 +1,6 @@
 # Unreleased
+* [changed] Called messageHandled() after a message has been handled to indicate
+  that the message has been handled successfully.
 * [changed] Added an internal identifier to Firelog logging for compliance.
 
 # 23.3.1

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

@@ -15,13 +15,16 @@
 
 import static com.google.firebase.messaging.Constants.TAG;
 
+import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
 import android.text.TextUtils;
 import android.util.Log;
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 import androidx.annotation.WorkerThread;
+import com.google.android.gms.cloudmessaging.CloudMessage;
+import com.google.android.gms.cloudmessaging.Rpc;
 import com.google.firebase.messaging.Constants.MessagePayloadKeys;
 import com.google.firebase.messaging.Constants.MessageTypes;
 import java.util.ArrayDeque;
@@ -82,6 +85,8 @@ public class FirebaseMessagingService extends EnhancedIntentService {
   private static final Queue<String> recentlyReceivedMessageIds =
       new ArrayDeque<>(RECENTLY_RECEIVED_MESSAGE_IDS_MAX_SIZE);
 
+  private Rpc rpc;
+
   /**
    * Called when a message is received.
    *
@@ -173,6 +178,7 @@ private void handleMessageIntent(Intent intent) {
     if (!alreadyReceivedMessage(messageId)) {
       passMessageIntentToSdk(intent);
     }
+    getRpc(this).messageHandled(new CloudMessage(intent));
   }
 
   private void passMessageIntentToSdk(Intent intent) {
@@ -263,8 +269,20 @@ private String getMessageId(Intent intent) {
     return messageId;
   }
 
+  private Rpc getRpc(Context context) {
+    if (rpc == null) {
+      rpc = new Rpc(context.getApplicationContext());
+    }
+    return rpc;
+  }
+
   @VisibleForTesting
   static void resetForTesting() {
     recentlyReceivedMessageIds.clear();
   }
+
+  @VisibleForTesting
+  void setRpcForTesting(Rpc rpc) {
+    this.rpc = rpc;
+  }
 }

firebase-messaging/src/test/java/com/google/firebase/messaging/FirebaseMessagingServiceRoboTest.java

@@ -26,12 +26,14 @@
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.robolectric.Shadows.shadowOf;
 
 import android.app.Activity;
@@ -53,6 +55,8 @@
 import android.os.Process;
 import androidx.test.core.app.ActivityScenario;
 import androidx.test.core.app.ApplicationProvider;
+import com.google.android.gms.cloudmessaging.CloudMessage;
+import com.google.android.gms.cloudmessaging.Rpc;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.firebase.FirebaseApp;
@@ -74,6 +78,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentMatcher;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.Robolectric;
@@ -211,6 +216,43 @@ public void testMessage_nullIntentExtras() throws Exception {
     assertThat(shadowOf(context).getNextStartedService()).isNull();
   }
 
+  @Test
+  public void messageHandled() throws Exception {
+    RemoteMessageBuilder builder =
+        new RemoteMessageBuilder()
+            .setFrom(DEFAULT_FROM)
+            .setTo(DEFAULT_TO)
+            .addData("key1", "value1")
+            .setMessageId("message_id_456")
+            .setSentTime(123456789);
+    Intent intent = builder.buildIntent();
+    Rpc mockRpc = mock(Rpc.class);
+    service.setRpcForTesting(mockRpc);
+    CountDownLatch latch = new CountDownLatch(1);
+    doAnswer(
+            invocation -> {
+              latch.await();
+              return null;
+            })
+        .when(service)
+        .onMessageReceived(any(RemoteMessage.class));
+
+    shadowOf(context).clearStartedServices();
+    sendBroadcastToReceiver(intent);
+    processInternalStartService(context);
+
+    // Shouldn't call messageHandled while onMessageReceived is still running.
+    verifyNoMoreInteractions(mockRpc);
+    // Finish onMessageReceived, messageHandled should now be called.
+    latch.countDown();
+    flushTasks();
+    ArgumentCaptor<CloudMessage> messageCaptor = ArgumentCaptor.forClass(CloudMessage.class);
+    verify(mockRpc).messageHandled(messageCaptor.capture());
+    CloudMessage message = messageCaptor.getValue();
+    assertThat(message).isNotNull();
+    assertThat(message.getIntent()).isEqualTo(intent);
+  }
+
   @Test
   public void testDuplicateMessageDropped() throws Exception {
     String messageId = "a.message.id";
@@ -308,6 +350,7 @@ public void testOnNewToken() throws Exception {
 
     ServiceStarter.getInstance().startMessagingService(context, intent);
     processInternalStartService(context);
+    flushTasks();
 
     verify(service).onNewToken("token123");
   }
@@ -685,6 +728,7 @@ public void startServiceViaReceiver(Intent intent) throws InterruptedException {
     shadowOf(context).clearStartedServices();
     sendBroadcastToReceiver(intent);
     processInternalStartService(context);
+    flushTasks();
   }
 
   /**
@@ -700,7 +744,6 @@ public void processInternalStartService(Application application) throws Interrup
     assertEquals(application.getPackageName(), serviceIntent.getPackage());
 
     service.onStartCommand(serviceIntent, 0 /* flags */, 1 /* startId */);
-    flushTasks();
   }
 
   // Flush the Service background tasks