Commit 0fed551e authored by David Trainor's avatar David Trainor Committed by Commit Bot

Prune out downloads with invalid states from DHv2

- We currently prune out cancelled and non-resumable failed/interrupted
downloads from downloads home.  In the V2 version this didn't happen, so
this adds the filter to address this.

- In the process of adding the filter, I realized that the filter's
onItemUpdated() logic does not handle items updating to/from a filtered
out state.  So this also adds that support as well as tests for it.

BUG=889213

Change-Id: I5ae6b836abf77f914bd3f106a2d21b015e03aad2
Reviewed-on: https://chromium-review.googlesource.com/1244123
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595671}
parent e89bb969
// Copyright 2018 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.home.filter;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemState;
/** A {@link OfflineItemFilter} responsible for pruning out items that do not have the right state
* to show in the UI.
*/
public class InvalidStateOfflineItemFilter extends OfflineItemFilter {
/** Creates an instance of this filter and wraps {@code source}. */
public InvalidStateOfflineItemFilter(OfflineItemFilterSource source) {
super(source);
onFilterChanged();
}
// OfflineItemFilter implementation.
@Override
protected boolean isFilteredOut(OfflineItem item) {
switch (item.state) {
case OfflineItemState.CANCELLED:
case OfflineItemState.FAILED:
return true;
case OfflineItemState.INTERRUPTED:
return !item.isResumable;
default:
return false;
}
}
}
\ No newline at end of file
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.download.home.filter;
import org.chromium.base.CollectionUtil;
import org.chromium.base.ObserverList;
import org.chromium.components.offline_items_collection.OfflineItem;
......@@ -111,10 +112,20 @@ public abstract class OfflineItemFilter
@Override
public void onItemUpdated(OfflineItem oldItem, OfflineItem item) {
if (!mItems.remove(oldItem)) return;
mItems.add(item);
for (OfflineItemFilterObserver obs : mObservers) obs.onItemUpdated(oldItem, item);
boolean oldInList = mItems.remove(oldItem);
boolean newInList = !isFilteredOut(item);
if (oldInList && newInList) {
mItems.add(item);
for (OfflineItemFilterObserver obs : mObservers) obs.onItemUpdated(oldItem, item);
} else if (!oldInList && newInList) {
mItems.add(item);
Collection<OfflineItem> newItems = CollectionUtil.newHashSet(item);
for (OfflineItemFilterObserver obs : mObservers) obs.onItemsAdded(newItems);
} else if (oldInList && !newInList) {
Collection<OfflineItem> oldItems = CollectionUtil.newHashSet(oldItem);
for (OfflineItemFilterObserver obs : mObservers) obs.onItemsRemoved(oldItems);
}
}
@Override
......
......@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.ChromeApplication;
import org.chromium.chrome.browser.download.home.OfflineItemSource;
import org.chromium.chrome.browser.download.home.filter.DeleteUndoOfflineItemFilter;
import org.chromium.chrome.browser.download.home.filter.Filters.FilterType;
import org.chromium.chrome.browser.download.home.filter.InvalidStateOfflineItemFilter;
import org.chromium.chrome.browser.download.home.filter.OffTheRecordOfflineItemFilter;
import org.chromium.chrome.browser.download.home.filter.OfflineItemFilter;
import org.chromium.chrome.browser.download.home.filter.OfflineItemFilterObserver;
......@@ -69,6 +70,7 @@ class DateOrderedListMediator {
private final SelectionDelegate<ListItem> mSelectionDelegate;
private final OffTheRecordOfflineItemFilter mOffTheRecordFilter;
private final InvalidStateOfflineItemFilter mInvalidStateFilter;
private final DeleteUndoOfflineItemFilter mDeleteUndoFilter;
private final TypeOfflineItemFilter mTypeFilter;
private final SearchOfflineItemFilter mSearchFilter;
......@@ -120,11 +122,12 @@ class DateOrderedListMediator {
// [OfflineContentProvider] ->
// [OfflineItemSource] ->
// [OffTheRecordOfflineItemFilter] ->
// [DeleteUndoOfflineItemFilter] ->
// [TypeOfflineItemFilter] ->
// [SearchOfflineItemFitler] ->
// [DateOrderedListMutator] ->
// [ListItemModel]
// [InvalidStateOfflineItemFilter] ->
// [DeleteUndoOfflineItemFilter] ->
// [TypeOfflineItemFilter] ->
// [SearchOfflineItemFitler] ->
// [DateOrderedListMutator] ->
// [ListItemModel]
mProvider = new OfflineContentProviderGlue(provider, offTheRecord);
mShareController = shareController;
......@@ -134,7 +137,8 @@ class DateOrderedListMediator {
mSource = new OfflineItemSource(mProvider);
mOffTheRecordFilter = new OffTheRecordOfflineItemFilter(offTheRecord, mSource);
mDeleteUndoFilter = new DeleteUndoOfflineItemFilter(mOffTheRecordFilter);
mInvalidStateFilter = new InvalidStateOfflineItemFilter(mOffTheRecordFilter);
mDeleteUndoFilter = new DeleteUndoOfflineItemFilter(mInvalidStateFilter);
mTypeFilter = new TypeOfflineItemFilter(mDeleteUndoFilter);
mSearchFilter = new SearchOfflineItemFilter(mTypeFilter);
mListMutator = new DateOrderedListMutator(mSearchFilter, mModel);
......
......@@ -485,6 +485,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/download/home/filter/FilterProperties.java",
"java/src/org/chromium/chrome/browser/download/home/filter/FilterView.java",
"java/src/org/chromium/chrome/browser/download/home/filter/FilterViewBinder.java",
"java/src/org/chromium/chrome/browser/download/home/filter/InvalidStateOfflineItemFilter.java",
"java/src/org/chromium/chrome/browser/download/home/filter/OfflineItemFilter.java",
"java/src/org/chromium/chrome/browser/download/home/filter/OfflineItemFilterObserver.java",
"java/src/org/chromium/chrome/browser/download/home/filter/OfflineItemFilterSource.java",
......
......@@ -39,9 +39,13 @@ public class OfflineItemFilterTest {
}
public void setFilters(Set<OfflineItem> items) {
setFiltersSilently(items);
onFilterChanged();
}
public void setFiltersSilently(Set<OfflineItem> items) {
mFilteredItems.clear();
mFilteredItems.addAll(items);
onFilterChanged();
}
// OfflineItemFilter implementation.
......@@ -163,6 +167,7 @@ public class OfflineItemFilterTest {
OfflineItem newItem2 = new OfflineItem();
sourceItems.remove(item2);
sourceItems.add(newItem2);
filter.setFiltersSilently(CollectionUtil.newHashSet(item1, newItem2, item4));
filter.onItemUpdated(item2, newItem2);
verify(mObserver, never()).onItemUpdated(item2, newItem2);
Assert.assertEquals(CollectionUtil.newHashSet(item3), filter.getItems());
......@@ -188,4 +193,41 @@ public class OfflineItemFilterTest {
verify(mObserver, times(1)).onItemUpdated(item3, newItem3);
Assert.assertEquals(CollectionUtil.newHashSet(newItem3), filter.getItems());
}
@Test
public void testUpdateFiltering() {
OfflineItem item1 = new OfflineItem();
OfflineItem item2 = new OfflineItem();
Collection<OfflineItem> sourceItems = CollectionUtil.newHashSet(item1, item2);
when(mSource.getItems()).thenReturn(sourceItems);
OfflineItemFilterImpl filter = new OfflineItemFilterImpl(mSource);
filter.addObserver(mObserver);
verify(mSource, times(1)).addObserver(filter);
verify(mSource, times(1)).getItems();
Assert.assertEquals(sourceItems, filter.getItems());
// Filter items.
filter.setFilters(CollectionUtil.newHashSet(item2));
verify(mObserver, times(1)).onItemsRemoved(CollectionUtil.newHashSet(item2));
Assert.assertEquals(CollectionUtil.newHashSet(item1), filter.getItems());
// Updated item goes from unfiltered to filtered.
OfflineItem newItem1 = new OfflineItem();
filter.setFiltersSilently(CollectionUtil.newHashSet(newItem1, item2));
sourceItems.remove(item1);
sourceItems.add(newItem1);
filter.onItemUpdated(item1, newItem1);
verify(mObserver, times(1)).onItemsRemoved(CollectionUtil.newHashSet(item1));
Assert.assertTrue(filter.getItems().isEmpty());
// Updated item goes from filtered to unfiltered.
OfflineItem newItem2 = new OfflineItem();
filter.setFiltersSilently(CollectionUtil.newHashSet(newItem1));
sourceItems.remove(item2);
sourceItems.add(newItem2);
filter.onItemUpdated(item2, newItem2);
verify(mObserver, times(1)).onItemsAdded(CollectionUtil.newHashSet(newItem2));
Assert.assertEquals(CollectionUtil.newHashSet(newItem2), filter.getItems());
}
}
\ No newline at end of file
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