Skip to content

GitLab

T
tangled
Commit bfb35d53 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot
Browse files
Options

Download notification: Simplify foreground notification events. 

Currently we use DownloadForegroundServiceObservers to broadcast
download foreground service events. It uses reflection to cache the
class name and then persist to android shared preference.

DownloadNotificationServiceObserver implements
DownloadForegroundServiceObservers.Observer, and then call methods of
the singleton object of DownloadNotificationService.

Also, onForegroundServiceRestarted() is not getting called, which make
the following calls dead code.

This is extremely weird, we can just call the one line singleton
method.

Bug: 922929
Change-Id: I28ff5552fd7fc0fd5a111cd4971d6bf4514bb45f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063853Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743368}
parent c3f7a7bf
Hide whitespace changes
Inline Side-by-side
Showing with 3 additions and 232 deletions
chrome/android/chrome_java_sources.gni
View file @ bfb35d53
......@@ -518,7 +518,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/download/DownloadController.java",
"java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java",
"java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java",
"java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceObservers.java",
"java/src/org/chromium/chrome/browser/download/DownloadInfoBarController.java",
"java/src/org/chromium/chrome/browser/download/DownloadItem.java",
"java/src/org/chromium/chrome/browser/download/DownloadLocationCustomView.java",
......@@ -527,7 +526,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/download/DownloadMetrics.java",
"java/src/org/chromium/chrome/browser/download/DownloadNotificationFactory.java",
"java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java",
"java/src/org/chromium/chrome/browser/download/DownloadNotificationServiceObserver.java",
"java/src/org/chromium/chrome/browser/download/DownloadNotificationUmaHelper.java",
"java/src/org/chromium/chrome/browser/download/DownloadNotifier.java",
"java/src/org/chromium/chrome/browser/download/DownloadPage.java",
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundService.java
View file @ bfb35d53
......@@ -187,7 +187,7 @@ public class DownloadForegroundService extends Service {
public void onDestroy() {
DownloadNotificationUmaHelper.recordServiceStoppedHistogram(
DownloadNotificationUmaHelper.ServiceStopped.DESTROYED, true /* withForeground */);
DownloadForegroundServiceObservers.alertObserversServiceDestroyed();
DownloadNotificationService.getInstance().onForegroundServiceDestroyed();
super.onDestroy();
}
......@@ -195,7 +195,7 @@ public class DownloadForegroundService extends Service {
public void onTaskRemoved(Intent rootIntent) {
DownloadNotificationUmaHelper.recordServiceStoppedHistogram(
DownloadNotificationUmaHelper.ServiceStopped.TASK_REMOVED, true /*withForeground*/);
DownloadForegroundServiceObservers.alertObserversTaskRemoved();
DownloadNotificationService.getInstance().onForegroundServiceTaskRemoved();
super.onTaskRemoved(rootIntent);
}
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceManager.java
View file @ bfb35d53
......@@ -212,8 +212,6 @@ public class DownloadForegroundServiceManager {
return;
}
mBoundService = ((DownloadForegroundService.LocalBinder) service).getService();
DownloadForegroundServiceObservers.addObserver(
DownloadNotificationServiceObserver.class);
processDownloadUpdateQueue(true /* isProcessingPending */);
}
......@@ -284,9 +282,6 @@ public class DownloadForegroundServiceManager {
mBoundService.stopDownloadForegroundService(
stopForegroundStatus, pinnedNotificationId, pinnedNotification);
ContextUtils.getApplicationContext().unbindService(mConnection);
DownloadForegroundServiceObservers.removeObserver(
DownloadNotificationServiceObserver.class);
}
/** Helper code for testing. */
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadForegroundServiceObservers.java deleted 100644 → 0
View file @ c3f7a7bf
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.download;
import androidx.annotation.Nullable;
import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import java.util.HashSet;
import java.util.Set;
/**
* A class that handles logic related to observers that are waiting to see when the
* DownloadsForegroundService is shutting down or starting back up.
*/
public final class DownloadForegroundServiceObservers {
private static final String TAG = "DownloadFgServiceObs";
/**
* An Observer interfaces that allows other classes to know when this service is shutting down.
* Implementing classes must be marked as @UsedByReflection in order to prevent the class name
* from being obfuscated.
* Implementing classes must also have a public parameterless constructor that is marked as
* @UsedByReflection.
* Implementing classes may never be renamed, as class names are persisted between app updates.
*/
public interface Observer {
/**
* Called when the foreground service was automatically restarted because of START_STICKY.
* @param pinnedNotificationId Id of the notification pinned to the service when it died.
*/
void onForegroundServiceRestarted(int pinnedNotificationId);
/**
* Called when any task (service or activity) is removed from the service's application.
*/
void onForegroundServiceTaskRemoved();
/**
* Called when the foreground service is destroyed.
*
*/
void onForegroundServiceDestroyed();
}
/**
* Adds an observer, which will be notified when this service attempts to start stopping itself.
* @param observerClass to be added to the list of observers in SharedPrefs.
*/
public static void addObserver(Class<? extends Observer> observerClass) {
ThreadUtils.assertOnUiThread();
Set<String> observers = getAllObservers();
String observerClassName = observerClass.getName();
if (observers.contains(observerClassName)) return;
// Set returned from getStringSet() should not be modified.
observers = new HashSet<>(observers);
observers.add(observerClassName);
SharedPreferencesManager.getInstance().writeStringSet(
ChromePreferenceKeys.DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS, observers);
}
/**
* Removes observer, which will no longer be notified when this class starts stopping itself.
* @param observerClass to be removed from the list of observers in SharedPrefs.
*/
public static void removeObserver(Class<? extends Observer> observerClass) {
ThreadUtils.assertOnUiThread();
Set<String> observers = getAllObservers();
String observerClassName = observerClass.getName();
if (!observers.contains(observerClassName)) return;
// Set returned from getStringSet() should not be modified.
observers = new HashSet<>(observers);
observers.remove(observerClassName);
// Clear observer list if there are none.
if (observers.size() == 0) {
removeAllObservers();
return;
}
SharedPreferencesManager.getInstance().writeStringSet(
ChromePreferenceKeys.DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS, observers);
}
static void alertObserversServiceRestarted(int pinnedNotificationId) {
Set<String> observers = getAllObservers();
removeAllObservers();
for (String observerClassName : observers) {
DownloadForegroundServiceObservers.Observer observer =
DownloadForegroundServiceObservers.getObserverFromClassName(observerClassName);
if (observer != null) observer.onForegroundServiceRestarted(pinnedNotificationId);
}
}
static void alertObserversServiceDestroyed() {
Set<String> observers = getAllObservers();
for (String observerClassName : observers) {
DownloadForegroundServiceObservers.Observer observer =
DownloadForegroundServiceObservers.getObserverFromClassName(observerClassName);
if (observer != null) observer.onForegroundServiceDestroyed();
}
}
static void alertObserversTaskRemoved() {
Set<String> observers = getAllObservers();
for (String observerClassName : observers) {
Observer observer = getObserverFromClassName(observerClassName);
if (observer != null) observer.onForegroundServiceTaskRemoved();
}
}
private static Set<String> getAllObservers() {
return SharedPreferencesManager.getInstance().readStringSet(
ChromePreferenceKeys.DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS);
}
private static void removeAllObservers() {
SharedPreferencesManager.getInstance().removeKey(
ChromePreferenceKeys.DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS);
}
@Nullable
private static Observer getObserverFromClassName(String observerClassName) {
try {
Class<?> observerClass = Class.forName(observerClassName);
return (Observer) observerClass.newInstance();
} catch (Throwable e) {
Log.w(TAG, "getObserverFromClassName(): %s", observerClassName, e);
return null;
}
}
}
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
View file @ bfb35d53
......@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.download;
import static org.chromium.chrome.browser.download.DownloadBroadcastManager.getServiceDelegate;
import static org.chromium.chrome.browser.download.DownloadSnackbarController.INVALID_NOTIFICATION_ID;
import android.app.Notification;
import android.content.Context;
......@@ -635,16 +634,6 @@ public class DownloadNotificationService {
ChromePreferenceKeys.DOWNLOAD_AUTO_RESUMPTION_ATTEMPT_LEFT);
}
void onForegroundServiceRestarted(int pinnedNotificationId) {
// In API < 24, notifications pinned to the foreground will get killed with the service.
// Fix this by relaunching the notification that was pinned to the service as the service
// dies, if there is one.
relaunchPinnedNotification(pinnedNotificationId);
updateNotificationsForShutdown();
resumeAllPendingDownloads();
}
void onForegroundServiceTaskRemoved() {
// If we've lost all Activities, cancel the off the record downloads.
if (ApplicationStatus.isEveryActivityDestroyed()) {
......@@ -657,39 +646,6 @@ public class DownloadNotificationService {
rescheduleDownloads();
}
/**
* Given the id of the notification that was pinned to the service when it died, give the
* notification a new id in order to rebuild and relaunch the notification.
* @param pinnedNotificationId Id of the notification pinned to the service when it died.
*/
private void relaunchPinnedNotification(int pinnedNotificationId) {
// If there was no notification pinned to the service, no correction is necessary.
if (pinnedNotificationId == INVALID_NOTIFICATION_ID) return;
List<DownloadSharedPreferenceEntry> entries = mDownloadSharedPreferenceHelper.getEntries();
List<DownloadSharedPreferenceEntry> copies =
new ArrayList<DownloadSharedPreferenceEntry>(entries);
for (DownloadSharedPreferenceEntry entry : copies) {
if (entry.notificationId == pinnedNotificationId) {
// Get new notification id that is not associated with the service.
DownloadSharedPreferenceEntry updatedEntry =
new DownloadSharedPreferenceEntry(entry.id, getNextNotificationId(),
entry.isOffTheRecord, entry.canDownloadWhileMetered, entry.fileName,
entry.isAutoResumable, entry.isTransient);
mDownloadSharedPreferenceHelper.addOrReplaceSharedPreferenceEntry(updatedEntry);
// Right now this only happens in the paused case, so re-build and re-launch the
// paused notification, with the updated notification id..
notifyDownloadPaused(updatedEntry.id, updatedEntry.fileName, true /* isResumable */,
updatedEntry.isAutoResumable, updatedEntry.isOffTheRecord,
updatedEntry.isTransient, null /* icon */, null /* originalUrl */,
false /* shouldPromoteOrigin */, true /* hasUserGesture */,
true /* forceRebuild */, PendingState.NOT_PENDING);
return;
}
}
}
private void updateNotificationsForShutdown() {
cancelOffTheRecordDownloads();
List<DownloadSharedPreferenceEntry> entries = mDownloadSharedPreferenceHelper.getEntries();
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationServiceObserver.java deleted 100644 → 0
View file @ c3f7a7bf
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.download;
import org.chromium.base.annotations.UsedByReflection;
/**
* A DownloadForegroundServiceObservers.Observer implementation for DownloadNotificationService.
*/
@UsedByReflection("DownloadForegroundServiceObservers")
public class DownloadNotificationServiceObserver
implements DownloadForegroundServiceObservers.Observer {
@UsedByReflection("DownloadForegroundServiceObservers")
public DownloadNotificationServiceObserver() {}
@Override
public void onForegroundServiceRestarted(int pinnedNotificationId) {
DownloadNotificationService.getInstance().onForegroundServiceRestarted(
pinnedNotificationId);
}
@Override
public void onForegroundServiceTaskRemoved() {
DownloadNotificationService.getInstance().onForegroundServiceTaskRemoved();
}
@Override
public void onForegroundServiceDestroyed() {
DownloadNotificationService.getInstance().onForegroundServiceDestroyed();
}
}
chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeys.java
View file @ bfb35d53
......@@ -246,7 +246,6 @@ public final class ChromePreferenceKeys {
"data_reduction_site_breakdown_allowed_date";
public static final String DOWNLOAD_AUTO_RESUMPTION_ATTEMPT_LEFT = "ResumptionAttemptLeft";
public static final String DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS = "ForegroundServiceObservers";
public static final String DOWNLOAD_IS_DOWNLOAD_HOME_ENABLED =
"org.chromium.chrome.browser.download.IS_DOWNLOAD_HOME_ENABLED";
public static final String DOWNLOAD_NEXT_DOWNLOAD_NOTIFICATION_ID =
......@@ -711,6 +710,7 @@ public final class ChromePreferenceKeys {
static List<String> createDeprecatedKeysForTesting() {
// clang-format off
return Arrays.asList(
"ForegroundServiceObservers",
"PersistedNotificationId",
"PhysicalWeb.ActivityReferral",
"PhysicalWeb.HasDeferredMetrics",
......@@ -818,7 +818,6 @@ public final class ChromePreferenceKeys {
DATA_REDUCTION_FRE_PROMO_OPT_OUT,
DATA_REDUCTION_SITE_BREAKDOWN_ALLOWED_DATE,
DOWNLOAD_AUTO_RESUMPTION_ATTEMPT_LEFT,
DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS,
DOWNLOAD_IS_DOWNLOAD_HOME_ENABLED,
DOWNLOAD_NEXT_DOWNLOAD_NOTIFICATION_ID,
DOWNLOAD_PENDING_DOWNLOAD_NOTIFICATIONS,
......
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