Index: chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java |
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java b/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java |
index d60f9bfd916807bcfea72ba1ae893365edcebf63..7f6323ecac675ff806852db522d9b31649dcbb43 100644 |
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java |
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java |
@@ -6,29 +6,51 @@ package org.chromium.chrome.browser.download.items; |
import org.chromium.chrome.browser.download.DownloadInfo; |
import org.chromium.chrome.browser.download.DownloadItem; |
-import org.chromium.chrome.browser.download.DownloadManagerService; |
import org.chromium.chrome.browser.download.DownloadNotifier; |
import org.chromium.chrome.browser.download.DownloadServiceDelegate; |
import org.chromium.components.offline_items_collection.ContentId; |
import org.chromium.components.offline_items_collection.OfflineContentProvider; |
import org.chromium.components.offline_items_collection.OfflineItem; |
import org.chromium.components.offline_items_collection.OfflineItemState; |
+import org.chromium.components.offline_items_collection.OfflineItemVisuals; |
import java.util.ArrayList; |
+import java.util.HashMap; |
/** |
* A glue class that bridges the Profile-attached OfflineContentProvider with the |
* download notification code (SystemDownloadNotifier and DownloadServiceDelegate). |
*/ |
public class OfflineContentAggregatorNotificationBridgeUi |
- implements DownloadServiceDelegate, OfflineContentProvider.Observer { |
+ implements DownloadServiceDelegate, OfflineContentProvider.Observer, |
+ OfflineContentProvider.VisualsCallback { |
+ // TODO(dtrainor): Should this just be part of the OfflineContentProvider callback guarantee? |
+ private static final OfflineItemVisuals sEmptyOfflineItemVisuals = new OfflineItemVisuals(); |
+ |
private final OfflineContentProvider mProvider; |
+ private final DownloadNotifier mUi; |
+ |
+ /** Holds a list of {@link OfflineItem} updates that are waiting for visuals. */ |
+ private final HashMap<ContentId, OfflineItem> mOutstandingRequests = new HashMap<>(); |
+ |
+ /** |
+ * Holds a list of {@link OfflineItemVisuals} for all {@link OfflineItem}s that are currently in |
+ * progress. Once an {@link OfflineItem} is no longer in progress it will be removed from this |
+ * cache. |
+ * TODO(dtrainor): Flush this list aggressively if we get onLowMemory/onTrimMemory. |
+ * TODO(dtrainor): Add periodic clean up in case something goes wrong with the underlying |
+ * downloads. |
+ */ |
+ private final HashMap<ContentId, OfflineItemVisuals> mVisualsCache = new HashMap<>(); |
+ |
/** |
* Creates a new OfflineContentAggregatorNotificationBridgeUi based on {@code provider}. |
*/ |
- public OfflineContentAggregatorNotificationBridgeUi(OfflineContentProvider provider) { |
+ public OfflineContentAggregatorNotificationBridgeUi( |
+ OfflineContentProvider provider, DownloadNotifier notifier) { |
mProvider = provider; |
+ mUi = notifier; |
mProvider.addObserver(this); |
} |
@@ -49,21 +71,35 @@ public class OfflineContentAggregatorNotificationBridgeUi |
public void onItemsAdded(ArrayList<OfflineItem> items) { |
for (int i = 0; i < items.size(); i++) { |
OfflineItem item = items.get(i); |
- |
- // Only update the UI for new OfflineItems that are in progress or pending. |
- if (item.state == OfflineItemState.IN_PROGRESS |
- || item.state == OfflineItemState.PENDING) { |
- visuallyUpdateOfflineItem(item); |
- } |
+ if (shouldPushNewItemToUi(item)) getVisualsAndUpdateItem(item); |
} |
} |
@Override |
- public void onItemRemoved(ContentId id) {} |
+ public void onItemRemoved(ContentId id) { |
+ mOutstandingRequests.remove(id); |
+ mVisualsCache.remove(id); |
+ mUi.notifyDownloadCanceled(id); |
+ } |
@Override |
public void onItemUpdated(OfflineItem item) { |
- visuallyUpdateOfflineItem(item); |
+ // Assume that any item sending updates should have them reflected in the UI. |
+ getVisualsAndUpdateItem(item); |
+ } |
+ |
+ // OfflineContentProvider.VisualsCallback implementation. |
+ @Override |
+ public void onVisualsAvailable(ContentId id, OfflineItemVisuals visuals) { |
+ OfflineItem item = mOutstandingRequests.remove(id); |
+ if (item == null) return; |
+ |
+ if (visuals == null) visuals = sEmptyOfflineItemVisuals; |
+ |
+ // Only cache the visuals if the update we are about to push is interesting and we think we |
+ // will need them in the future. |
+ if (shouldCacheVisuals(item)) mVisualsCache.put(id, visuals); |
+ pushItemToUi(item, visuals); |
} |
// DownloadServiceDelegate implementation. |
@@ -85,34 +121,98 @@ public class OfflineContentAggregatorNotificationBridgeUi |
@Override |
public void destroyServiceDelegate() {} |
- /** |
- * Calls into the proper {@link DownloadNotifier} by converting an {@link OfflineItem} to a |
- * {@link DownloadInfo}. |
- * @param item The {@link OfflineItem} that needs a UI refresh. |
- */ |
- private void visuallyUpdateOfflineItem(OfflineItem item) { |
- DownloadInfo info = DownloadInfo.fromOfflineItem(item); |
- DownloadNotifier notifier = |
- DownloadManagerService.getDownloadManagerService().getDownloadNotifier(); |
+ private void getVisualsAndUpdateItem(OfflineItem item) { |
+ OfflineItemVisuals visuals = mVisualsCache.get(item.id); |
+ |
+ if (!needsVisualsForUi(item)) { |
nyquist
2017/04/13 05:08:24
If you flip this else around you don't need the !
David Trainor- moved to gerrit
2017/04/13 07:20:23
Done.
|
+ // We don't need the visuals to show this item at this point. Cancel any requests. |
+ mOutstandingRequests.remove(item.id); |
+ mVisualsCache.remove(item.id); |
+ } else { |
+ if (visuals == null) { |
+ // We don't have any visuals for this item yet. Stash the current OfflineItem and, |
+ // if we haven't already, queue up a request for the visuals. |
+ // TODO(dtrainor): Check if this delay is too much. If so, just send the update |
+ // through. |
+ boolean requestVisuals = !mOutstandingRequests.containsKey(item.id); |
+ mOutstandingRequests.put(item.id, item); |
+ if (requestVisuals) mProvider.getVisualsForItem(item.id, this); |
+ return; |
+ } |
+ } |
+ |
+ pushItemToUi(item, visuals); |
+ // We will no longer be needing the visuals for this item after this notification. |
+ if (!shouldCacheVisuals(item)) mVisualsCache.remove(item.id); |
+ } |
+ |
+ private void pushItemToUi(OfflineItem item, OfflineItemVisuals visuals) { |
+ DownloadInfo info = DownloadInfo.fromOfflineItem(item, visuals); |
switch (item.state) { |
case OfflineItemState.IN_PROGRESS: |
- notifier.notifyDownloadProgress(info, item.creationTimeMs, item.allowMetered); |
+ mUi.notifyDownloadProgress(info, item.creationTimeMs, item.allowMetered); |
break; |
case OfflineItemState.COMPLETE: |
- notifier.notifyDownloadSuccessful(info, -1L, false, false); |
+ mUi.notifyDownloadSuccessful(info, -1L, false, false); |
break; |
case OfflineItemState.CANCELLED: |
- notifier.notifyDownloadCanceled(item.id); |
+ mUi.notifyDownloadCanceled(item.id); |
break; |
case OfflineItemState.INTERRUPTED: |
// TODO(dtrainor): Push the correct value for auto resume. |
- notifier.notifyDownloadInterrupted(info, true); |
+ mUi.notifyDownloadInterrupted(info, true); |
break; |
case OfflineItemState.PAUSED: |
- notifier.notifyDownloadPaused(info); |
+ mUi.notifyDownloadPaused(info); |
+ break; |
+ case OfflineItemState.FAILED: |
+ mUi.notifyDownloadFailed(info); |
break; |
+ } |
+ } |
+ |
+ private boolean needsVisualsForUi(OfflineItem item) { |
+ switch (item.state) { |
+ case OfflineItemState.IN_PROGRESS: |
nyquist
2017/04/13 05:08:24
Often I want to ask the author to add . // Intenti
David Trainor- moved to gerrit
2017/04/13 07:20:23
Yeah I wasn't sure what looked nicer, but this loo
|
+ case OfflineItemState.PENDING: |
+ case OfflineItemState.COMPLETE: |
+ case OfflineItemState.INTERRUPTED: |
+ case OfflineItemState.FAILED: |
+ case OfflineItemState.PAUSED: |
+ return true; |
+ case OfflineItemState.CANCELLED: |
+ default: |
+ return false; |
+ } |
+ } |
+ |
+ private boolean shouldPushNewItemToUi(OfflineItem item) { |
+ switch (item.state) { |
+ case OfflineItemState.IN_PROGRESS: |
+ return true; |
+ case OfflineItemState.PENDING: |
+ case OfflineItemState.COMPLETE: |
+ case OfflineItemState.INTERRUPTED: |
+ case OfflineItemState.FAILED: |
+ case OfflineItemState.PAUSED: |
+ case OfflineItemState.CANCELLED: |
+ default: |
+ return false; |
+ } |
+ } |
+ |
+ private boolean shouldCacheVisuals(OfflineItem item) { |
+ switch (item.state) { |
+ case OfflineItemState.IN_PROGRESS: |
+ case OfflineItemState.PENDING: |
+ case OfflineItemState.INTERRUPTED: |
+ case OfflineItemState.PAUSED: |
+ return true; |
+ case OfflineItemState.FAILED: |
+ case OfflineItemState.COMPLETE: |
+ case OfflineItemState.CANCELLED: |
default: |
- assert false; |
+ return false; |
} |
} |
} |