Skip to content

Commit aa5785a

Browse files
gsakakiharagsakakihara
committed
Finish message broadcast after sending Intent to bound Service.
* Changed to finish the message broadcast as soon as the Intent is sent to the FirebaseMessagingService when binding to the Service. This should reduce the chance that the BroadcastReceiver ANRs in certain circumstances. * Acquired a WakeLock when using bindService() to send an Intent to FirebaseMessagingService to prevent the device from sleeping while processing the Intent now that the broadcast is finished before the Service has finished handling it. - Holding the WakeLock for up to 60 seconds for high priority messages to match the behavior for high priority messages when using startService(). - Holding the WakeLock for up to 9 seconds for normal priority messages to match the previous behavior where the broadcast was not forced to finish for 9 seconds.
1 parent 905eeb8 commit aa5785a

File tree

6 files changed

+204
-10
lines changed

6 files changed

+204
-10
lines changed

‎firebase-messaging/src/main/java/com/google/firebase/messaging/FcmBroadcastProcessor.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,14 @@ private static Task<Integer> bindToMessagingService(Context context, Intent inte
118118
if (Log.isLoggable(TAG, Log.DEBUG)) {
119119
Log.d(TAG, "Binding to service");
120120
}
121+
if (ServiceStarter.getInstance().hasWakeLockPermission(context)) {
122+
WakeLockHolder.sendWakefulServiceIntent(
123+
context, getServiceConnection(context, ServiceStarter.ACTION_MESSAGING_EVENT), intent);
124+
} else {
125+
getServiceConnection(context, ServiceStarter.ACTION_MESSAGING_EVENT).sendIntent(intent);
126+
}
121127

122-
return getServiceConnection(context, ServiceStarter.ACTION_MESSAGING_EVENT)
123-
.sendIntent(intent)
124-
// ok to use direct executor because we're just immediately returning an int
125-
.continueWith(Runnable::run, t -> ServiceStarter.SUCCESS);
128+
return Tasks.forResult(ServiceStarter.SUCCESS);
126129
}
127130

128131
/** Connect to a service via bind. This is used to process intents in Android O+ */

‎firebase-messaging/src/main/java/com/google/firebase/messaging/WakeLockHolder.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ final class WakeLockHolder {
3838
private static final String EXTRA_WAKEFUL_INTENT =
3939
"com.google.firebase.iid.WakeLockHolder.wakefulintent";
4040
/** Release wakelocks after 60s, because we don't expect operations to take longer than that. */
41-
private static final long WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(1);
41+
static final long WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(1);
4242
// Object to sync threads
4343
private static final Object syncObject = new Object();
4444

@@ -86,6 +86,34 @@ static ComponentName startWakefulService(@NonNull Context context, @NonNull Inte
8686
}
8787
}
8888

89+
/**
90+
* Sends an Intent to a Service, binding to it, if necessary. Acquires a WakeLock based on the
91+
* Intent and holds until the Service has finished processing the Intent or after a certain amount
92+
* of time.
93+
*
94+
* @param context Application context.
95+
* @param connection ServiceConnection to send the Intent to.
96+
* @param intent Intent for starting the service.
97+
*/
98+
static void sendWakefulServiceIntent(
99+
Context context, WithinAppServiceConnection connection, Intent intent) {
100+
synchronized (syncObject) {
101+
checkAndInitWakeLock(context);
102+
103+
boolean isWakeLockAlreadyAcquired = isWakefulIntent(intent);
104+
105+
setAsWakefulIntent(intent, true);
106+
107+
connection
108+
.sendIntent(intent)
109+
.addOnCompleteListener(Runnable::run, t -> completeWakefulIntent(intent));
110+
111+
if (!isWakeLockAlreadyAcquired) {
112+
wakeLock.acquire(WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS);
113+
}
114+
}
115+
}
116+
89117
private static void setAsWakefulIntent(@NonNull Intent intent, boolean isWakeful) {
90118
intent.putExtra(EXTRA_WAKEFUL_INTENT, isWakeful);
91119
}

‎firebase-messaging/src/main/java/com/google/firebase/messaging/WithinAppServiceConnection.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.google.firebase.messaging;
1515

1616
import static com.google.firebase.messaging.FirebaseMessaging.TAG;
17+
import static com.google.firebase.messaging.WakeLockHolder.WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS;
1718

1819
import android.content.ComponentName;
1920
import android.content.Context;
@@ -41,8 +42,8 @@
4142
*/
4243
class WithinAppServiceConnection implements ServiceConnection {
4344

44-
// Time out requests by finishing the pending result and thus unblocking the receiver after
45-
// 9s to avoid causing the receiver to time out after 10s.
45+
// Time out WakeLock after 9s to match previous behavior of forcing the broadcast to finish after
46+
// that much time.
4647
private static final int REQUEST_TIMEOUT_MS = 9000;
4748

4849
static class BindRequest {
@@ -54,17 +55,21 @@ static class BindRequest {
5455
}
5556

5657
void arrangeTimeout(ScheduledExecutorService executor) {
58+
// Allow the same maximum WakeLock duration for high priority messages as when using
59+
// startService(), otherwise allow up to 9 seconds to match the time that the service was
60+
// allowed to run before finishing the BroadcastReceiver.
61+
boolean isHighPriority = (intent.getFlags() & Intent.FLAG_RECEIVER_FOREGROUND) != 0;
5762
ScheduledFuture<?> timeoutFuture =
5863
executor.schedule(
5964
() -> {
6065
Log.w(
6166
TAG,
6267
"Service took too long to process intent: "
6368
+ intent.getAction()
64-
+ " App may get closed.");
69+
+ " Releasing WakeLock.");
6570
finish();
6671
},
67-
REQUEST_TIMEOUT_MS,
72+
isHighPriority ? WAKE_LOCK_ACQUIRE_TIMEOUT_MILLIS : REQUEST_TIMEOUT_MS,
6873
TimeUnit.MILLISECONDS);
6974

7075
getTask()

‎firebase-messaging/src/test/java/com/google/firebase/messaging/FcmBroadcastProcessorRoboTest.java

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.firebase.messaging;
15+
16+
import static com.google.common.truth.Truth.assertThat;
17+
import static org.mockito.ArgumentMatchers.any;
18+
import static org.mockito.Mockito.never;
19+
import static org.mockito.Mockito.verify;
20+
import static org.mockito.Mockito.when;
21+
import static org.robolectric.Shadows.shadowOf;
22+
23+
import android.app.Application;
24+
import android.content.Context;
25+
import android.content.Intent;
26+
import android.os.Build.VERSION_CODES;
27+
import androidx.test.core.app.ApplicationProvider;
28+
import com.google.android.gms.tasks.Task;
29+
import com.google.firebase.messaging.testing.FakeScheduledExecutorService;
30+
import org.junit.After;
31+
import org.junit.Before;
32+
import org.junit.Rule;
33+
import org.junit.Test;
34+
import org.junit.runner.RunWith;
35+
import org.mockito.Mock;
36+
import org.mockito.junit.MockitoJUnit;
37+
import org.mockito.junit.MockitoRule;
38+
import org.robolectric.RobolectricTestRunner;
39+
import org.robolectric.annotation.Config;
40+
import org.robolectric.shadows.ShadowPowerManager;
41+
42+
/** Robolectric test for FcmBroadcastProcessor. */
43+
@RunWith(RobolectricTestRunner.class)
44+
public class FcmBroadcastProcessorRoboTest {
45+
46+
private static final String ACTION_FCM_MESSAGE = "com.google.android.c2dm.intent.RECEIVE";
47+
48+
@Rule public final MockitoRule mocks = MockitoJUnit.rule();
49+
50+
private Application context;
51+
private FcmBroadcastProcessor processor;
52+
private FakeScheduledExecutorService fakeExecutorService;
53+
@Mock private ServiceStarter serviceStarter;
54+
55+
@Before
56+
public void setUp() {
57+
context = ApplicationProvider.getApplicationContext();
58+
ServiceStarter.setForTesting(serviceStarter);
59+
fakeExecutorService = new FakeScheduledExecutorService();
60+
processor = new FcmBroadcastProcessor(context, fakeExecutorService);
61+
}
62+
63+
@After
64+
public void resetStaticState() {
65+
ServiceStarter.setForTesting(null);
66+
FcmBroadcastProcessor.reset();
67+
WakeLockHolder.reset();
68+
ShadowPowerManager.clearWakeLocks();
69+
}
70+
71+
@Test
72+
@Config(sdk = VERSION_CODES.O)
73+
public void testStartMessagingService_NormalPriorityBackgroundCheck() {
74+
// Subject to background check when run on Android O and targetSdkVersion set to O.
75+
context.getApplicationInfo().targetSdkVersion = VERSION_CODES.O;
76+
when(serviceStarter.hasWakeLockPermission(any(Context.class))).thenReturn(true);
77+
78+
Task<Integer> startServiceTask =
79+
processor.startMessagingService(context, new Intent(ACTION_FCM_MESSAGE));
80+
81+
// Should return immediately with SUCCESS, bind to the Service, and acquire a WakeLock.
82+
assertThat(startServiceTask.getResult()).isEqualTo(ServiceStarter.SUCCESS);
83+
verify(serviceStarter, never()).startMessagingService(any(), any());
84+
assertThat(shadowOf(context).getBoundServiceConnections()).hasSize(1);
85+
assertThat(ShadowPowerManager.getLatestWakeLock()).isNotNull();
86+
assertThat(ShadowPowerManager.getLatestWakeLock().isHeld()).isTrue();
87+
}
88+
89+
@Test
90+
@Config(sdk = VERSION_CODES.O)
91+
public void testStartMessagingService_bindNoWakeLockPermission() {
92+
// Subject to background check when run on Android O and targetSdkVersion set to O.
93+
context.getApplicationInfo().targetSdkVersion = VERSION_CODES.O;
94+
when(serviceStarter.hasWakeLockPermission(any(Context.class))).thenReturn(false);
95+
96+
Task<Integer> startServiceTask =
97+
processor.startMessagingService(context, new Intent(ACTION_FCM_MESSAGE));
98+
99+
// Should return immediately with SUCCESS and bind to the Service, but not try to acquire a
100+
// WakeLock since it doesn't hold the permission.
101+
assertThat(startServiceTask.getResult()).isEqualTo(ServiceStarter.SUCCESS);
102+
verify(serviceStarter, never()).startMessagingService(any(), any());
103+
assertThat(shadowOf(context).getBoundServiceConnections()).hasSize(1);
104+
assertThat(ShadowPowerManager.getLatestWakeLock()).isNull();
105+
}
106+
}

‎firebase-messaging/src/test/java/com/google/firebase/messaging/WakeLockHolderRoboTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@
1414
package com.google.firebase.messaging;
1515

1616
import static com.google.common.truth.Truth.assertThat;
17+
import static org.mockito.ArgumentMatchers.any;
18+
import static org.mockito.Mockito.mock;
19+
import static org.mockito.Mockito.when;
1720

1821
import android.app.Application;
1922
import android.content.Intent;
2023
import android.os.PowerManager.WakeLock;
2124
import androidx.test.core.app.ApplicationProvider;
25+
import com.google.android.gms.tasks.TaskCompletionSource;
2226
import org.junit.Before;
2327
import org.junit.Test;
2428
import org.junit.runner.RunWith;
@@ -51,6 +55,31 @@ public void testStartWakefulService_AcquiresWakeLock() throws Exception {
5155
assertThat(ShadowPowerManager.getLatestWakeLock().isHeld()).isTrue();
5256
}
5357

58+
@Test
59+
public void testSendWakefulServiceIntent_AcquiresWakeLock() {
60+
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
61+
WithinAppServiceConnection mockConnection = mock(WithinAppServiceConnection.class);
62+
when(mockConnection.sendIntent(any(Intent.class))).thenReturn(taskCompletionSource.getTask());
63+
WakeLockHolder.sendWakefulServiceIntent(context, mockConnection, new Intent());
64+
65+
assertThat(ShadowPowerManager.getLatestWakeLock()).isNotNull();
66+
assertThat(ShadowPowerManager.getLatestWakeLock().isHeld()).isTrue();
67+
}
68+
69+
@Test
70+
public void testSendWakefulServiceIntent_ReleasesWakeLock() {
71+
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
72+
WithinAppServiceConnection mockConnection = mock(WithinAppServiceConnection.class);
73+
when(mockConnection.sendIntent(any(Intent.class))).thenReturn(taskCompletionSource.getTask());
74+
WakeLockHolder.sendWakefulServiceIntent(context, mockConnection, new Intent());
75+
76+
// Verify that the WakeLock is released once the Intent has been handled by the Service.
77+
WakeLock wakeLock = ShadowPowerManager.getLatestWakeLock();
78+
taskCompletionSource.setResult(null);
79+
80+
assertThat(wakeLock.isHeld()).isFalse();
81+
}
82+
5483
@Test
5584
public void testCompleteWakefulIntent_ReleasesWakeLockIfPresent() throws Exception {
5685
Intent intent = newIntent("Any_Action");

‎firebase-messaging/src/test/java/com/google/firebase/messaging/WithinAppServiceConnectionRoboTest.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ public class WithinAppServiceConnectionRoboTest {
4646
// The amount of time that a broadcast receiver takes to time out
4747
private static final long RECEIVER_TIMEOUT_S = 10;
4848

49+
// The amount of time to allow a foreground broadcast's service to run.
50+
private static final long FOREGROUND_RECEIVER_TIMEOUT_S = 60;
51+
4952
private Application context;
5053
private FakeScheduledExecutorService fakeExecutor;
5154

@@ -66,14 +69,34 @@ public void testReceiverTimesOut() {
6669
Task<Void> pendingResult = connection.sendIntent(new Intent());
6770
assertThat(pendingResult.isComplete()).isFalse();
6871

69-
// Check the runnable doesn't run early, and that after it shuld have run the pending
72+
// Check the runnable doesn't run early, and that after it should have run the pending
7073
// result is finished.
7174
fakeExecutor.simulateNormalOperationFor(RECEIVER_TIMEOUT_S - 2, TimeUnit.SECONDS);
7275
assertThat(pendingResult.isComplete()).isFalse();
7376
fakeExecutor.simulateNormalOperationFor(1, TimeUnit.SECONDS);
7477
assertThat(pendingResult.isComplete()).isTrue();
7578
}
7679

80+
@Test
81+
public void testReceiverTimesOut_ForegroundBroadcast() {
82+
WithinAppServiceConnection connection =
83+
new WithinAppServiceConnection(context, TEST_BIND_ACTION, fakeExecutor);
84+
setMockBinder(TEST_BIND_ACTION);
85+
86+
// Send a foreground broadcst intent, verify the pending result isn't finished
87+
Intent foregroundBroadcastIntent = new Intent();
88+
foregroundBroadcastIntent.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
89+
Task<Void> pendingResult = connection.sendIntent(foregroundBroadcastIntent);
90+
assertThat(pendingResult.isComplete()).isFalse();
91+
92+
// Check the runnable doesn't run early, and that after it should have run the pending
93+
// result is finished.
94+
fakeExecutor.simulateNormalOperationFor(FOREGROUND_RECEIVER_TIMEOUT_S - 1, TimeUnit.SECONDS);
95+
assertThat(pendingResult.isComplete()).isFalse();
96+
fakeExecutor.simulateNormalOperationFor(1, TimeUnit.SECONDS);
97+
assertThat(pendingResult.isComplete()).isTrue();
98+
}
99+
77100
@Test
78101
public void testTimeoutGetsCancelled() {
79102
WithinAppServiceConnection connection =

0 commit comments

Comments
 (0)