Commit 70e53cea authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Revert "Download notification: Simplify foreground notification events."

This reverts commit bfb35d53.

Reason for revert: Triggers a bug in the old code. see crbug/1056107.

Original change's description:
> 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/+/2063853
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#743368}

TBR=dtrainor@chromium.org,qinmin@chromium.org,xingliu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 922929
Change-Id: I467b19ba958a380a08e464339e735d5e30e376c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076977Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745156}
parent 695da801
......@@ -506,6 +506,7 @@ 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",
......@@ -514,6 +515,7 @@ 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",
......
......@@ -187,7 +187,7 @@ public class DownloadForegroundService extends Service {
public void onDestroy() {
DownloadNotificationUmaHelper.recordServiceStoppedHistogram(
DownloadNotificationUmaHelper.ServiceStopped.DESTROYED, true /* withForeground */);
DownloadNotificationService.getInstance().onForegroundServiceDestroyed();
DownloadForegroundServiceObservers.alertObserversServiceDestroyed();
super.onDestroy();
}
......@@ -195,7 +195,7 @@ public class DownloadForegroundService extends Service {
public void onTaskRemoved(Intent rootIntent) {
DownloadNotificationUmaHelper.recordServiceStoppedHistogram(
DownloadNotificationUmaHelper.ServiceStopped.TASK_REMOVED, true /*withForeground*/);
DownloadNotificationService.getInstance().onForegroundServiceTaskRemoved();
DownloadForegroundServiceObservers.alertObserversTaskRemoved();
super.onTaskRemoved(rootIntent);
}
......
......@@ -212,6 +212,8 @@ public class DownloadForegroundServiceManager {
return;
}
mBoundService = ((DownloadForegroundService.LocalBinder) service).getService();
DownloadForegroundServiceObservers.addObserver(
DownloadNotificationServiceObserver.class);
processDownloadUpdateQueue(true /* isProcessingPending */);
}
......@@ -282,6 +284,9 @@ public class DownloadForegroundServiceManager {
mBoundService.stopDownloadForegroundService(
stopForegroundStatus, pinnedNotificationId, pinnedNotification);
ContextUtils.getApplicationContext().unbindService(mConnection);
DownloadForegroundServiceObservers.removeObserver(
DownloadNotificationServiceObserver.class);
}
/** Helper code for testing. */
......
// 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;
}
}
}
......@@ -5,6 +5,7 @@
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;
......@@ -634,6 +635,16 @@ 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()) {
......@@ -646,6 +657,39 @@ 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();
......
// 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();
}
}
......@@ -246,6 +246,7 @@ 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 =
......@@ -694,7 +695,6 @@ public final class ChromePreferenceKeys {
static List<String> createDeprecatedKeysForTesting() {
// clang-format off
return Arrays.asList(
"ForegroundServiceObservers",
"PersistedNotificationId",
"PhysicalWeb.ActivityReferral",
"PhysicalWeb.HasDeferredMetrics",
......@@ -805,6 +805,7 @@ 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