Downloads: Properly prune processed actions

chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java

Index: chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java b/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
index 108435aa619931d60450b1a5c060f88c613a270f..e7f39b2b185d9fd381fa82350f01448cc9dec5b0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier.java
@@ -114,6 +114,9 @@ public class SystemDownloadNotifier implements DownloadNotifier, Observer {
             // If we lose the service mid-loop retrigger the service load and quit.

[qinmin, 2017/03/23 23:21:00]

can this actually happen in the middle of a running function? 
autoRelease only happen for the last notification in the list, and all other
method that tries to call unbindService happens on the same UI thread, right?

[David Trainor- moved to gerrit, 2017/03/24 00:32:35]

I think the service can try to stop itself if it has no more notifications to
show, which could in theory make it attempt to shutdown (e.g. the cancel case?).

[qinmin, 2017/03/24 05:05:09]

you mean service.stopself()? I don't think that matters. Stopself() won't be
executed immediately, it still put a task on a message queue to wait the current
code to finish.

[David Trainor- moved to gerrit, 2017/03/25 03:56:24]

Gah lost a big reply by closing the tab :(.  So tl;dr there is an issue but it
might not end up being a problem.  The issue is Service.stopSelf() doesn't stop
the service if there are clients bound.  But once the client unbinds the service
will be destroyed.  To make this deterministic and make sense, I just have the
service tell this notifier to unbind itself now so we don't have this weird 'in
between' state.

If we don't manually tell the service to unbind, we'll only unbind when we have
no running downloads.  So the scenario is:

Download ID | State    | Batch
1             PROGRESS   1
1             CANCEL     2   // Service calls stopSelf.  Destroy gets queued.
2             PROGRESS   2   // Gets processed because we're still bound.
                             // Now we are a foreground service that will die?
2             PAUSED     3   // No longer foreground.  Unbind.  Does the service
die?

If the service dies here, we should be good as long as we've detached the
notification.  I need to test what happens.  I can recreate it this weekend or
on Monday.

If this isn't a problem I need to remove the callback method.  I'll write up a
patch that does that and send it to you while I investigate.  Thanks!

             if (mBoundService == null) {
                 startAndBindToServiceIfNeeded();
+                // Prune the list by pulling out the items we've already processed.
+                mPendingNotifications = new ArrayList<PendingNotificationInfo>(
+                        mPendingNotifications.subList(i, mPendingNotifications.size()));
                 return;
             }
             updateDownloadNotification(