Skip to content

GitLab

T
tangled
Commit 7865e3c4 authored by Shakti Sahu's avatar Shakti Sahu Committed by Commit Bot
Browse files
Options

Fixed crash in recycler view due to duplicate stable ids in download home 

Sometimes we incorrectly create two Just Now sections in download home.
This leads to a crash in RecyclerView as stable IDs should be unique.
This happens especially when there are two downloads very close to midnight,
one before midnight and one after midnight. This CL fixes this issue
and places both of them in one Just Now section.

Change-Id: I5af6560c29a7413f86a6245719859197d72268dd
Bug: 1020079
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024068Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735708}
parent 7d2d7ac4
Hide whitespace changes
Inline Side-by-side
Showing with 88 additions and 27 deletions
chrome/android/java/src/org/chromium/chrome/browser/download/home/JustNowProvider.java
View file @ 7865e3c4
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.download.home;
import androidx.annotation.VisibleForTesting;
import org.chromium.components.offline_items_collection.ContentId;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemState;
......@@ -24,7 +26,7 @@ public class JustNowProvider {
/** Constructor. */
public JustNowProvider(DownloadManagerUiConfig config) {
mThresholdDate = new Date(new Date().getTime() - config.justNowThresholdSeconds * 1000);
mThresholdDate = new Date(now().getTime() - config.justNowThresholdSeconds * 1000);
}
/**
......@@ -41,4 +43,9 @@ public class JustNowProvider {
|| (item.state == OfflineItemState.INTERRUPTED && item.isResumable)
|| new Date(item.completionTimeMs).after(mThresholdDate);
}
}
@VisibleForTesting
protected Date now() {
return new Date();
}
}
\ No newline at end of file
chrome/android/java/src/org/chromium/chrome/browser/download/home/list/mutator/DateLabelAdder.java
View file @ 7865e3c4
......@@ -54,8 +54,11 @@ public class DateLabelAdder implements ListConsumer {
if (!(listItem instanceof OfflineItemListItem)) continue;
OfflineItem offlineItem = ((OfflineItemListItem) listItem).item;
if (startOfNewDay(offlineItem, previousItem)
|| justNowSectionsDiffer(offlineItem, previousItem)) {
boolean startOfNewDay = startOfNewDay(offlineItem, previousItem);
boolean isJustNow =
mJustNowProvider != null && mJustNowProvider.isJustNowItem(offlineItem);
if (isJustNow) startOfNewDay = false;
if (startOfNewDay || justNowSectionsDiffer(offlineItem, previousItem)) {
addDateHeader(listItems, offlineItem, i);
}
......
chrome/android/junit/src/org/chromium/chrome/browser/download/home/list/DateOrderedListMutatorTest.java
View file @ 7865e3c4
......@@ -145,7 +145,7 @@ public class DateOrderedListMutatorTest {
DateOrderedListMutator list = createMutatorWithJustNowProvider();
Assert.assertEquals(2, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), item1);
}
......@@ -159,20 +159,56 @@ public class DateOrderedListMutatorTest {
*/
@Test
public void testMultipleSectionsInJustNowSection() {
OfflineItem item1 = buildItem("1", buildCalendar(2018, 1, 1, 1), OfflineItemFilter.VIDEO);
OfflineItem item2 = buildItem("2", buildCalendar(2018, 1, 1, 1), OfflineItemFilter.AUDIO);
Calendar calendar = buildCalendar(2018, 1, 1, 1);
OfflineItem item1 = buildItem("1", calendar, OfflineItemFilter.VIDEO);
OfflineItem item2 = buildItem("2", calendar, OfflineItemFilter.AUDIO);
item1.state = OfflineItemState.IN_PROGRESS;
item2.state = OfflineItemState.IN_PROGRESS;
item2.state = OfflineItemState.COMPLETE;
item2.completionTimeMs = item2.creationTimeMs;
when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1, item2));
DateOrderedListMutator list = createMutatorWithJustNowProvider();
DateOrderedListMutator list =
createMutatorWithJustNowProvider(buildJustNowProvider(calendar.getTime()));
Assert.assertEquals(3, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), item1);
assertOfflineItem(mModel.get(2), buildCalendar(2018, 1, 1, 1), item2);
}
/**
* Action List
* 1. Set(item1 @ 0:10 1/2/2018 [ DATE Just Now,
* Video COMPLETE,
* item2 @ 23:55 1/1/2018 [ DATE Just Now,
* Video COMPLETE,
*
* item3 @ 10:00 1/1/2018 DATE 1/1/2018
* Audio COMPLETE) item3 @ 1:00 1/1/2018 ]
*/
@Test
public void testRecentItemBeforeMidnightShowsInJustNowSection() {
Calendar calendar1 = CalendarFactory.get();
calendar1.set(2018, 1, 2, 0, 10);
Calendar calendar2 = CalendarFactory.get();
calendar2.set(2018, 1, 1, 23, 50);
OfflineItem item1 = buildItem("1", calendar1, OfflineItemFilter.VIDEO);
OfflineItem item2 = buildItem("2", calendar2, OfflineItemFilter.AUDIO);
OfflineItem item3 = buildItem("3", buildCalendar(2018, 1, 1, 10), OfflineItemFilter.AUDIO);
item1.completionTimeMs = item1.creationTimeMs;
item2.completionTimeMs = item2.creationTimeMs;
when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1, item2, item3));
Calendar now = CalendarFactory.get();
now.set(2018, 1, 2, 0, 15);
createMutatorWithJustNowProvider(buildJustNowProvider(now.getTime()));
Assert.assertEquals(5, mModel.size());
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), calendar1, item1);
assertOfflineItem(mModel.get(2), calendar2, item2);
assertSectionHeader(mModel.get(3), buildCalendar(2018, 1, 1, 0), true);
assertOfflineItem(mModel.get(4), buildCalendar(2018, 1, 1, 10), item3);
}
/**
* Action List
* 1. Set(item1 @ 1:00 1/1/2018 [ DATE Just Now,
......@@ -197,7 +233,7 @@ public class DateOrderedListMutatorTest {
mModel.addObserver(mObserver);
Assert.assertEquals(2, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), item1);
// Resume the download.
......@@ -207,7 +243,7 @@ public class DateOrderedListMutatorTest {
list.onItemUpdated(item1, update1);
Assert.assertEquals(2, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), update1);
// Complete the download.
......@@ -218,7 +254,7 @@ public class DateOrderedListMutatorTest {
list.onItemUpdated(update1, update2);
Assert.assertEquals(2, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), update2);
// Too much time has passed since completion of the download.
......@@ -229,7 +265,7 @@ public class DateOrderedListMutatorTest {
list.onItemUpdated(update2, update3);
Assert.assertEquals(2, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 1, 1, 1), update3);
}
......@@ -250,7 +286,7 @@ public class DateOrderedListMutatorTest {
DateOrderedListMutator list = createMutatorWithJustNowProvider();
Assert.assertEquals(4, mModel.size());
assertJustNowSection(mModel.get(0), false);
assertJustNowSection(mModel.get(0));
assertOfflineItem(mModel.get(1), buildCalendar(2018, 2, 1, 1), item1);
assertSectionHeader(mModel.get(2), buildCalendar(2018, 1, 1, 0), true);
assertOfflineItem(mModel.get(3), buildCalendar(2018, 1, 1, 1), item2);
......@@ -871,11 +907,25 @@ public class DateOrderedListMutatorTest {
return item;
}
private DownloadManagerUiConfig createConfig() {
return new DownloadManagerUiConfig.Builder()
.setUseNewDownloadPath(true)
.setSupportsGrouping(false)
.build();
}
private JustNowProvider buildJustNowProvider(Date overrideNow) {
JustNowProvider justNowProvider = new JustNowProvider(createConfig()) {
@Override
protected Date now() {
return overrideNow;
}
};
return justNowProvider;
}
private DateOrderedListMutator createMutatorWithoutJustNowProvider() {
DownloadManagerUiConfig config = new DownloadManagerUiConfig.Builder()
.setUseNewDownloadPath(true)
.setSupportsGrouping(false)
.build();
DownloadManagerUiConfig config = createConfig();
JustNowProvider justNowProvider = new JustNowProvider(config) {
@Override
public boolean isJustNowItem(OfflineItem item) {
......@@ -889,14 +939,15 @@ public class DateOrderedListMutatorTest {
}
private DateOrderedListMutator createMutatorWithJustNowProvider() {
DownloadManagerUiConfig config = new DownloadManagerUiConfig.Builder()
.setUseNewDownloadPath(true)
.setSupportsGrouping(false)
.build();
JustNowProvider justNowProvider = new JustNowProvider(config);
JustNowProvider justNowProvider = new JustNowProvider(createConfig());
return createMutatorWithJustNowProvider(justNowProvider);
}
private DateOrderedListMutator createMutatorWithJustNowProvider(
JustNowProvider justNowProvider) {
DateOrderedListMutator mutator =
new DateOrderedListMutator(mSource, mModel, justNowProvider);
new ListMutationController(config, justNowProvider, mutator, mModel);
new ListMutationController(createConfig(), justNowProvider, mutator, mModel);
return mutator;
}
......@@ -923,11 +974,11 @@ public class DateOrderedListMutatorTest {
Assert.assertEquals(sectionHeader.showTopDivider, showDivider);
}
private static void assertJustNowSection(ListItem item, boolean showDivider) {
private static void assertJustNowSection(ListItem item) {
Assert.assertTrue(item instanceof SectionHeaderListItem);
SectionHeaderListItem sectionHeader = (SectionHeaderListItem) item;
Assert.assertTrue(sectionHeader.isJustNow);
Assert.assertEquals(sectionHeader.showTopDivider, showDivider);
Assert.assertEquals(false, sectionHeader.showTopDivider);
Assert.assertEquals(StableIds.JUST_NOW_SECTION, item.stableId);
}
}
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