Commit 8578f770 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Foreground service: Fix a crash in OnePlus devices.

OnePlus devices uses modified Android OS and track analytics when foreground
service is stopped. This introduces a crash in the process that runs
ActivityManagerService, and send a signal to app process and triggers
NullPointerException.

This CL tries catch this exception, and make call sites to use the new
utility function.

TBR=mlamouri@chromium.org

Bug: 992347
Change-Id: I8ec0e6b1ccb1a23469369811f3f74bf83dbb4612
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1753675
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687393}
parent 5fce07b7
...@@ -249,7 +249,7 @@ public class DownloadForegroundService extends Service { ...@@ -249,7 +249,7 @@ public class DownloadForegroundService extends Service {
@VisibleForTesting @VisibleForTesting
void stopForegroundInternal(int flags) { void stopForegroundInternal(int flags) {
Log.w(TAG, "stopForegroundInternal flags: " + flags); Log.w(TAG, "stopForegroundInternal flags: " + flags);
ServiceCompat.stopForeground(this, flags); ForegroundServiceUtils.getInstance().stopForeground(this, flags);
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -369,7 +369,8 @@ public class MediaNotificationManager { ...@@ -369,7 +369,8 @@ public class MediaNotificationManager {
@VisibleForTesting @VisibleForTesting
void stopListenerService() { void stopListenerService() {
// Call stopForeground to guarantee Android unset the foreground bit. // Call stopForeground to guarantee Android unset the foreground bit.
stopForeground(true /* removeNotification */); ForegroundServiceUtils.getInstance().stopForeground(
this, Service.STOP_FOREGROUND_REMOVE);
stopSelf(); stopSelf();
} }
...@@ -893,7 +894,8 @@ public class MediaNotificationManager { ...@@ -893,7 +894,8 @@ public class MediaNotificationManager {
mMediaSession = null; mMediaSession = null;
} }
if (mService != null) { if (mService != null) {
mService.stopForeground(true /* removeNotification */); ForegroundServiceUtils.getInstance().stopForeground(
mService, Service.STOP_FOREGROUND_REMOVE);
mService.stopSelf(); mService.stopSelf();
} }
mMediaNotificationInfo = null; mMediaNotificationInfo = null;
...@@ -940,7 +942,8 @@ public class MediaNotificationManager { ...@@ -940,7 +942,8 @@ public class MediaNotificationManager {
if (mMediaNotificationInfo == null) { if (mMediaNotificationInfo == null) {
if (serviceStarting) { if (serviceStarting) {
finishStartingForegroundService(mService); finishStartingForegroundService(mService);
mService.stopForeground(true /* removeNotification */); ForegroundServiceUtils.getInstance().stopForeground(
mService, Service.STOP_FOREGROUND_REMOVE);
} }
return; return;
} }
...@@ -964,8 +967,8 @@ public class MediaNotificationManager { ...@@ -964,8 +967,8 @@ public class MediaNotificationManager {
// While the service is in foreground, the associated notification can't be swipped away. // While the service is in foreground, the associated notification can't be swipped away.
// Moving it back to background allows the user to remove the notification. // Moving it back to background allows the user to remove the notification.
if (mMediaNotificationInfo.supportsSwipeAway() && mMediaNotificationInfo.isPaused) { if (mMediaNotificationInfo.supportsSwipeAway() && mMediaNotificationInfo.isPaused) {
mService.stopForeground(false /* removeNotification */); ForegroundServiceUtils.getInstance().stopForeground(
mService, Service.STOP_FOREGROUND_DETACH);
NotificationManagerProxy manager = new NotificationManagerProxyImpl(getContext()); NotificationManagerProxy manager = new NotificationManagerProxyImpl(getContext());
manager.notify(notification); manager.notify(notification);
} else if (!foregroundedService) { } else if (!foregroundedService) {
......
...@@ -7,9 +7,11 @@ package org.chromium.chrome.browser.notifications; ...@@ -7,9 +7,11 @@ package org.chromium.chrome.browser.notifications;
import android.app.Notification; import android.app.Notification;
import android.app.Service; import android.app.Service;
import android.content.Intent; import android.content.Intent;
import android.support.v4.app.ServiceCompat;
import android.support.v4.content.ContextCompat; import android.support.v4.content.ContextCompat;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.AppHooks; import org.chromium.chrome.browser.AppHooks;
...@@ -18,6 +20,7 @@ import org.chromium.chrome.browser.AppHooks; ...@@ -18,6 +20,7 @@ import org.chromium.chrome.browser.AppHooks;
* compatibility for older Android versions and work around for Android API bugs. * compatibility for older Android versions and work around for Android API bugs.
*/ */
public class ForegroundServiceUtils { public class ForegroundServiceUtils {
private static final String TAG = "ForegroundService";
private ForegroundServiceUtils() {} private ForegroundServiceUtils() {}
/** /**
...@@ -67,4 +70,18 @@ public class ForegroundServiceUtils { ...@@ -67,4 +70,18 @@ public class ForegroundServiceUtils {
// TODO(xingliu): Remove startForeground call from AppHooks when Q sdk is available. // TODO(xingliu): Remove startForeground call from AppHooks when Q sdk is available.
AppHooks.get().startForeground(service, id, notification, foregroundServiceType); AppHooks.get().startForeground(service, id, notification, foregroundServiceType);
} }
/**
* Stops the foreground service. See {@link ServiceCompat#stopForeground(Service, int)}.
* @param service The foreground service to stop.
* @param flags The flags to stop foreground service.
*/
public void stopForeground(Service service, int flags) {
// OnePlus devices may throw NullPointerException, see https://crbug.com/992347.
try {
ServiceCompat.stopForeground(service, flags);
} catch (NullPointerException e) {
Log.e(TAG, "Failed to stop foreground service, ", e);
}
}
} }
...@@ -23,6 +23,7 @@ import static org.robolectric.Shadows.shadowOf; ...@@ -23,6 +23,7 @@ import static org.robolectric.Shadows.shadowOf;
import android.app.Notification; import android.app.Notification;
import android.app.NotificationManager; import android.app.NotificationManager;
import android.app.Service;
import android.content.Context; import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.os.Build; import android.os.Build;
...@@ -222,8 +223,9 @@ public class MediaNotificationManagerServiceLifecycleTest extends MediaNotificat ...@@ -222,8 +223,9 @@ public class MediaNotificationManagerServiceLifecycleTest extends MediaNotificat
verify(getManager(), never()).updateMediaSession(); verify(getManager(), never()).updateMediaSession();
verify(getManager(), never()).updateNotificationBuilder(); verify(getManager(), never()).updateNotificationBuilder();
verify(mService, never()).stopForeground(anyBoolean()); verify(mMockForegroundServiceUtils, never()).stopForeground(eq(mService), anyInt());
verify(mService, never()).startForeground(anyInt(), any(Notification.class)); verify(mMockForegroundServiceUtils, never())
.startForeground(eq(mService), anyInt(), any(Notification.class), anyInt());
} }
@Test @Test
...@@ -234,7 +236,8 @@ public class MediaNotificationManagerServiceLifecycleTest extends MediaNotificat ...@@ -234,7 +236,8 @@ public class MediaNotificationManagerServiceLifecycleTest extends MediaNotificat
getManager().mMediaNotificationInfo = mMediaNotificationInfoBuilder.build(); getManager().mMediaNotificationInfo = mMediaNotificationInfoBuilder.build();
getManager().updateNotification(false, false); getManager().updateNotification(false, false);
verify(mService).stopForeground(false); verify(mMockForegroundServiceUtils)
.stopForeground(eq(mService), eq(Service.STOP_FOREGROUND_DETACH));
assertEquals(1, getShadowNotificationManager().size()); assertEquals(1, getShadowNotificationManager().size());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment