Optimize DateOrderedListModel by batching notification calls

Add support for a BatchedListObservable.  This is an implementation of
ListObservable that incorporates the Android support library's
BatchListUpdateCallback.  This class has the following behavior:
- It batches up all calls to insert, remove, or change based on whether or not the changes occur to
adjacent elements.
- It automatically flushes the updates when a non-adjacent or different
type of operation occurs.
- It supports manual flushing for when the batch operation is done by
the caller.

BUG=842345

Change-Id: I87dcd03ad31952582308a690fb3a3dab3a4fc26b
Reviewed-on: https://chromium-review.googlesource.com/1070885
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561552}

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/BatchListObservable.java

+// 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.list;
+
+import android.support.v7.util.BatchingListUpdateCallback;
+import android.support.v7.util.ListUpdateCallback;
+
+import org.chromium.chrome.browser.modelutil.ListObservable;
+
+/**
+ * Helper class to batch updates to ListObservable before notifying observers.
+ * @see BatchingListUpdateCallback
+ */
+public abstract class BatchListObservable extends ListObservable {
+    final BatchingListUpdateCallback mBatchingCallback;
+
+    /** Creates a new BatchListObservable instance. */
+    public BatchListObservable() {
+        mBatchingCallback = new BatchingListUpdateCallback(new ListUpdateCallback() {
+            @Override
+            public void onInserted(int position, int count) {
+                super_notifyItemRangeInserted(position, count);
+            }
+
+            @Override
+            public void onRemoved(int position, int count) {
+                super_notifyItemRangeRemoved(position, count);
+            }
+
+            @Override
+            public void onMoved(int fromPosition, int toPosition) {
+                assert false : "ListUpdateCallback#onMoved() is not supported by ListObservable.";
+            }
+
+            @Override
+            public void onChanged(int position, int count, Object payload) {
+                super_notifyItemRangeChanged(position, count, payload);
+            }
+        });
+    }
+
+    /**
+     * Dispatches any outstanding batched updates to observers.
+     * @see BatchingListUpdateCallback#dispatchLastEvent()
+     */
+    public void dispatchLastEvent() {
+        mBatchingCallback.dispatchLastEvent();
+    }
+
+    @Override
+    protected void notifyItemRangeInserted(int index, int count) {
+        mBatchingCallback.onInserted(index, count);
+    }
+
+    @Override
+    protected void notifyItemRangeRemoved(int index, int count) {
+        mBatchingCallback.onRemoved(index, count);
+    }
+
+    @Override
+    protected void notifyItemRangeChanged(int index, int count, Object payload) {
+        mBatchingCallback.onChanged(index, count, payload);
+    }
+
+    private void super_notifyItemRangeInserted(int index, int count) {
+        super.notifyItemRangeInserted(index, count);
+    }
+
+    private void super_notifyItemRangeRemoved(int index, int count) {
+        super.notifyItemRangeRemoved(index, count);
+    }
+
+    private void super_notifyItemRangeChanged(int index, int count, Object payload) {
+        super.notifyItemRangeChanged(index, count, payload);
+    }
+}
\ No newline at end of file

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/DateOrderedListModel.java

@@ -18,7 +18,7 @@ import java.util.List;
  * {@link DateOrderedList}.  This will be updated in the near future to support large batch updates
  * to minimize the hit on {@link ListObserver}s.
  */
-public class DateOrderedListModel extends ListObservable {
+public class DateOrderedListModel extends BatchListObservable {
     /**
      * Represents an item meant to be shown in the UI.  If {@code ListItem#item} is {@code null},
      * this item represents a date separate for a new day's worth of items.  Otherwise it represents

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/DateOrderedListMutator.java

@@ -10,8 +10,11 @@ import org.chromium.chrome.browser.download.home.filter.OfflineItemFilterSource;
 import org.chromium.components.offline_items_collection.ContentId;
 import org.chromium.components.offline_items_collection.OfflineItem;
 
+import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * A class responsible for turning a {@link Collection} of {@link OfflineItem}s into a list meant
@@ -22,9 +25,9 @@ import java.util.Collection;
  *
  * TODO(dtrainor): This should be optimized in the near future.  There are a few key things that can
  * be changed:
- * - Batch observer calls if large adds/removes make a bunch of adjacent changes.
- * - Perform O(N) add/remove calls by effectively merging and unmerging the lists instead of looped
- *   inserts.
+ * - Do a single iterating across each list to merge/unmerge them.  This requires sorting and
+ *   tracking the current position across both as iterating (see {@link #onItemsRemoved(Collection)}
+ *   for an example since that is close to doing what we want - minus the contains() call).
  */
 public class DateOrderedListMutator implements OfflineItemFilterObserver {
     private static final int INVALID_INDEX = -1;
@@ -45,7 +48,15 @@ public class DateOrderedListMutator implements OfflineItemFilterObserver {
     // OfflineItemFilterObserver implementation.
     @Override
     public void onItemsAdded(Collection<OfflineItem> items) {
-        for (OfflineItem item : items) {
+        List<OfflineItem> sorted = new ArrayList<>(items);
+        Collections.sort(sorted, (lhs, rhs) -> {
+            long delta = rhs.creationTimeMs - lhs.creationTimeMs;
+            if (delta > 0) return 1;
+            if (delta < 0) return -1;
+            return 0;
+        });
+
+        for (OfflineItem item : sorted) {
             int index = getBestIndexFor(item);
             mModel.addItem(index, new DateOrderedListModel.ListItem(item));
 
@@ -59,6 +70,8 @@ public class DateOrderedListMutator implements OfflineItemFilterObserver {
                 mModel.addItem(index, new DateOrderedListModel.ListItem(startOfDay));
             }
         }
+
+        mModel.dispatchLastEvent();
     }
 
     @Override
@@ -74,6 +87,8 @@ public class DateOrderedListMutator implements OfflineItemFilterObserver {
 
             if (removeHeader || removeItem) mModel.removeItem(i);
         }
+
+        mModel.dispatchLastEvent();
     }
 
     @Override
@@ -90,6 +105,8 @@ public class DateOrderedListMutator implements OfflineItemFilterObserver {
         } else {
             mModel.setItem(i, new DateOrderedListModel.ListItem(item));
         }
+
+        mModel.dispatchLastEvent();
     }
 
     private int indexOfItem(ContentId id) {

chrome/android/java_sources.gni

@@ -446,6 +446,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/download/home/filter/chips/ChipView.java",
   "java/src/org/chromium/chrome/browser/download/home/list/CalendarFactory.java",
   "java/src/org/chromium/chrome/browser/download/home/list/CalendarUtils.java",
+  "java/src/org/chromium/chrome/browser/download/home/list/BatchListObservable.java",
   "java/src/org/chromium/chrome/browser/download/home/list/DateOrderedListModel.java",
   "java/src/org/chromium/chrome/browser/download/home/list/DateOrderedListMutator.java",
   "java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorFactory.java",

chrome/android/junit/src/org/chromium/chrome/browser/download/home/list/DateOrderedListMutatorTest.java

@@ -200,7 +200,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1));
         list.onItemsAdded(CollectionUtil.newArrayList(item1));
 
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 0, 1);
+        verify(mObserver, times(1)).onItemRangeInserted(mModel, 0, 2);
         Assert.assertEquals(2, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 1, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 1, 4), item1);
@@ -238,7 +238,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1, item2, item3));
         list.onItemsAdded(CollectionUtil.newArrayList(item3));
 
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 0, 1);
+        verify(mObserver, times(1)).onItemRangeInserted(mModel, 0, 2);
         Assert.assertEquals(5, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 1, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 1, 2), item3);
@@ -285,7 +285,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1, item2, item3));
         list.onItemsAdded(CollectionUtil.newArrayList(item3));
 
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 3, 1);
+        verify(mObserver, times(1)).onItemRangeInserted(mModel, 3, 2);
         Assert.assertEquals(5, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 2, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 2, 2), item1);
@@ -311,8 +311,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(Collections.emptySet());
         list.onItemsRemoved(CollectionUtil.newArrayList(item1));
 
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 1, 1);
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 0, 1);
+        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 0, 2);
         Assert.assertEquals(0, mModel.getItemCount());
     }
 
@@ -389,8 +388,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item1));
         list.onItemsRemoved(CollectionUtil.newArrayList(item2));
 
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 3, 1);
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 2, 1);
+        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 2, 2);
         Assert.assertEquals(2, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 2, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 2, 2), item1);
@@ -422,10 +420,7 @@ public class DateOrderedListMutatorTest {
                 .thenReturn(CollectionUtil.newArrayList(item1, item2, item3, item4));
         list.onItemsAdded(CollectionUtil.newArrayList(item1, item2, item3, item4));
 
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 0, 1);
-        verify(mObserver, times(1)).onItemRangeInserted(mModel, 2, 1);
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 3, 1);
-        verify(mObserver, times(1)).onItemRangeInserted(mModel, 4, 1);
+        verify(mObserver, times(1)).onItemRangeInserted(mModel, 0, 6);
         Assert.assertEquals(6, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 1, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 1, 4), item1);
@@ -463,9 +458,7 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(item2));
         list.onItemsRemoved(CollectionUtil.newArrayList(item1, item3, item4));
 
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 5, 1);
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 4, 1);
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 3, 1);
+        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 3, 3);
         verify(mObserver, times(1)).onItemRangeRemoved(mModel, 1, 1);
         Assert.assertEquals(2, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 1, 0), null);
@@ -554,9 +547,8 @@ public class DateOrderedListMutatorTest {
         when(mSource.getItems()).thenReturn(CollectionUtil.newArrayList(newItem1));
         list.onItemUpdated(item1, newItem1);
 
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 1, 1);
-        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 0, 1);
-        verify(mObserver, times(2)).onItemRangeInserted(mModel, 0, 1);
+        verify(mObserver, times(1)).onItemRangeRemoved(mModel, 0, 2);
+        verify(mObserver, times(1)).onItemRangeInserted(mModel, 0, 2);
         Assert.assertEquals(2, mModel.getItemCount());
         assertListItemEquals(mModel.getItemAt(0), buildCalendar(2018, 1, 2, 0), null);
         assertListItemEquals(mModel.getItemAt(1), buildCalendar(2018, 1, 2, 6), newItem1);