Reload the ReadingListCollection only if changes occurred

The Reading List Collection View does not need to be reloaded for every change.
This CL reloads it only if some changes occurred.

BUG=677924

Review-Url: https://codereview.chromium.org/2609173002
Cr-Commit-Position: refs/heads/master@{#441928}

ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h

@@ -21,6 +21,9 @@ class GURL;
 @property(nonatomic, copy) NSString* detailText;
 // The URL of the Reading List entry.
 @property(nonatomic, readonly) const GURL& url;
+// Status of the offline version.
+@property(nonatomic, assign)
+    ReadingListEntry::DistillationState distillationState;
 
 // Designated initializer. The |provider| will be used for loading favicon
 // attributes. The |delegate| is used to inform about changes to this item. The

ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm

@@ -47,7 +47,6 @@ const CGFloat kDistillationIndicatorSize = 18;
 
 @interface ReadingListCollectionViewItem ()<ReadingListCellDelegate> {
   GURL _url;
-  ReadingListEntry::DistillationState _distillationState;
 }
 // Attributes provider used to retrieve favicons.
 @property(nonatomic, strong)
@@ -66,6 +65,7 @@ const CGFloat kDistillationIndicatorSize = 18;
 @synthesize detailText = _detailText;
 @synthesize url = _url;
 @synthesize displayedCell = _displayedCell;
+@synthesize distillationState = _distillationState;
 
 - (instancetype)initWithType:(NSInteger)type
           attributesProvider:(FaviconAttributesProvider*)provider
@@ -96,6 +96,14 @@ const CGFloat kDistillationIndicatorSize = 18;
   return self;
 }
 
+#pragma mark - property
+
+- (void)setDistillationState:
+    (ReadingListEntry::DistillationState)distillationState {
+  self.displayedCell.distillationState = distillationState;
+  _distillationState = distillationState;
+}
+
 #pragma mark - CollectionViewTextItem
 
 - (void)configureCell:(ReadingListCell*)cell {
@@ -128,6 +136,18 @@ const CGFloat kDistillationIndicatorSize = 18;
                                     self.text, self.detailText];
 }
 
+- (BOOL)isEqual:(id)other {
+  if (other == self)
+    return YES;
+  if (!other || ![other isKindOfClass:[self class]])
+    return NO;
+  ReadingListCollectionViewItem* otherItem =
+      static_cast<ReadingListCollectionViewItem*>(other);
+  return [self.text isEqualToString:otherItem.text] &&
+         [self.detailText isEqualToString:otherItem.detailText] &&
+         self.distillationState == otherItem.distillationState;
+}
+
 @end
 
 #pragma mark - ReadingListCell
@@ -220,7 +240,7 @@ const CGFloat kDistillationIndicatorSize = 18;
   }
 }
 
-#pragma mark UICollectionViewCell
+#pragma mark - UICollectionViewCell
 
 - (void)prepareForReuse {
   [self.delegate readingListCellWillPrepareForReload:self];

ios/chrome/browser/ui/reading_list/reading_list_view_controller.mm

@@ -85,6 +85,11 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   AlertCoordinator* _actionSheet;
   std::unique_ptr<ReadingListModelBridge> _modelBridge;
   UIView* _emptyCollectionBackground;
+
+  // Whether the model modifications should be taken into account.
+  BOOL _shouldMonitorModel;
+  // Whether the model has pending modifications.
+  BOOL _modelHasBeenModified;
 }
 
 // Lazily instantiated.
@@ -101,9 +106,21 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
 // of the map key.
 - (void)loadItemsFromMap:(const ItemsMapByDate&)map
                toSection:(SectionIdentifier)sectionIdentifier;
+// Whether the model has changed.
+- (BOOL)hasModelChanged;
+// Returns whether there is a difference between the elements contained in the
+// |sectionIdentifier| and those in the |map|.
+- (BOOL)section:(SectionIdentifier)sectionIdentifier
+    isDifferentOfMap:(ItemsMapByDate&)map;
+// Reloads the data if a changed occured during editing
+- (void)applyPendingUpdates;
 // Convenience method to create cell items for reading list entries.
 - (ReadingListCollectionViewItem*)cellItemForReadingListEntry:
     (const ReadingListEntry&)entry;
+// Fills the |unread_map| and the |read_map| with the corresponding
+// ReadingListCollectionViewItem from the readingListModel.
+- (void)fillUnreadMap:(ItemsMapByDate&)unread_map
+              readMap:(ItemsMapByDate&)read_map;
 // Returns whether there are elements in the section identified by
 // |sectionIdentifier|.
 - (BOOL)hasItemInSection:(SectionIdentifier)sectionIdentifier;
@@ -135,7 +152,8 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
 // Applies |action| to every cell in the section |identifier|.
 - (void)updateItemsInSectionIdentifier:(SectionIdentifier)identifier
                      usingEntryUpdater:(EntryUpdater)updater;
-// Applies |action| to every selected element of collection view.
+// Applies |action| to every selected element of collection view. The monitoring
+// of the model updates is stopped during this time.
 - (void)updateSelectedItemsWithEntryUpdater:(EntryUpdater)updater;
 // Logs the deletions histograms for the entry with |url|.
 - (void)logDeletionHistogramsForEntry:(const GURL&)url;
@@ -187,6 +205,9 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
     _readingListDownloadService = readingListDownloadService;
     _emptyCollectionBackground = [self emptyCollectionBackground];
 
+    _shouldMonitorModel = YES;
+    _modelHasBeenModified = NO;
+
     _modelBridge.reset(new ReadingListModelBridge(self, model));
   }
   return self;
@@ -312,22 +333,33 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
 }
 
 - (void)readingListModelDidApplyChanges:(const ReadingListModel*)model {
-  // Ignore model updates when the view controller is being edited or  doing
-  // batch updates.
-  if (model->IsPerformingBatchUpdates() || [self.editor isEditing]) {
+  if (!_shouldMonitorModel) {
+    return;
+  }
+
+  // If we are editing and monitoring the model updates, set a flag to reload
+  // the data at the end of the editing.
+  if ([self.editor isEditing]) {
+    _modelHasBeenModified = YES;
     return;
   }
 
-  [self reloadData];
+  // Ignore model updates when the view controller is doing batch updates.
+  if (model->IsPerformingBatchUpdates()) {
+    return;
+  }
+
+  if ([self hasModelChanged])
+    [self reloadData];
 }
 
 - (void)readingListModelCompletedBatchUpdates:(const ReadingListModel*)model {
-  // Ignore model updates when the view controller is being edited.
-  if ([self.editor isEditing]) {
+  if (!_shouldMonitorModel) {
     return;
   }
 
-  [self reloadData];
+  if ([self hasModelChanged])
+    [self reloadData];
 }
 
 #pragma mark - private methods
@@ -483,6 +515,16 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
 - (void)loadItems {
   ItemsMapByDate read_map;
   ItemsMapByDate unread_map;
+  [self fillUnreadMap:unread_map readMap:read_map];
+  [self loadItemsFromMap:unread_map toSection:SectionIdentifierUnread];
+  [self loadItemsFromMap:read_map toSection:SectionIdentifierRead];
+
+  BOOL hasRead = read_map.size() > 0;
+  [_toolbar setHasReadItem:hasRead];
+}
+
+- (void)fillUnreadMap:(ItemsMapByDate&)unread_map
+              readMap:(ItemsMapByDate&)read_map {
   for (const auto& url : self.readingListModel->Keys()) {
     const ReadingListEntry* entry = self.readingListModel->GetEntryByURL(url);
     ReadingListCollectionViewItem* item =
@@ -493,11 +535,6 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
       unread_map.insert(std::make_pair(entry->UpdateTime(), item));
     }
   }
-  [self loadItemsFromMap:unread_map toSection:SectionIdentifierUnread];
-  [self loadItemsFromMap:read_map toSection:SectionIdentifierRead];
-
-  BOOL hasRead = read_map.size() > 0;
-  [_toolbar setHasReadItem:hasRead];
 }
 
 - (void)reloadData {
@@ -507,6 +544,55 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   }
 }
 
+- (void)applyPendingUpdates {
+  if (_modelHasBeenModified) {
+    [self reloadData];
+  }
+}
+
+- (BOOL)hasModelChanged {
+  ItemsMapByDate read_map;
+  ItemsMapByDate unread_map;
+  [self fillUnreadMap:unread_map readMap:read_map];
+
+  if ([self section:SectionIdentifierRead isDifferentOfMap:read_map])
+    return YES;
+  if ([self section:SectionIdentifierUnread isDifferentOfMap:unread_map])
+    return YES;
+
+  return NO;
+}
+
+- (BOOL)section:(SectionIdentifier)sectionIdentifier
+    isDifferentOfMap:(ItemsMapByDate&)map {
+  if (![self.collectionViewModel
+          hasSectionForSectionIdentifier:sectionIdentifier]) {
+    return !map.empty();
+  }
+
+  NSArray* items =
+      [self.collectionViewModel itemsInSectionWithIdentifier:sectionIdentifier];
+  if ([items count] != map.size())
+    return NO;
+
+  NSInteger index = 0;
+  ItemsMapByDate::const_reverse_iterator iterator = map.rbegin();
+  for (; iterator != map.rend(); iterator++) {
+    ReadingListCollectionViewItem* oldItem =
+        base::mac::ObjCCastStrict<ReadingListCollectionViewItem>(items[index]);
+    ReadingListCollectionViewItem* newItem = iterator->second;
+    if (oldItem.url == newItem.url) {
+      oldItem.text = newItem.text;
+      oldItem.distillationState = newItem.distillationState;
+    }
+    if (![oldItem isEqual:newItem]) {
+      return YES;
+    }
+    index++;
+  }
+  return NO;
+}
+
 - (ReadingListCollectionViewItem*)cellItemForReadingListEntry:
     (const ReadingListEntry&)entry {
   GURL url = entry.URL();
@@ -759,7 +845,7 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   }
       completion:^(BOOL) {
         // Reload data to take into account possible sync events.
-        [self reloadData];
+        [self applyPendingUpdates];
       }];
   // As we modified the section in the batch update block, remove the section in
   // another block.
@@ -783,7 +869,7 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   }
       completion:^(BOOL) {
         // Reload data to take into account possible sync events.
-        [self reloadData];
+        [self applyPendingUpdates];
       }];
   // As we modified the section in the batch update block, remove the section in
   // another block.
@@ -804,6 +890,7 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
 }
 
 - (void)updateSelectedItemsWithEntryUpdater:(EntryUpdater)updater {
+  _shouldMonitorModel = NO;
   auto token = self.readingListModel->BeginBatchUpdates();
   for (NSIndexPath* index in self.collectionView.indexPathsForSelectedItems) {
     CollectionViewItem* cell = [self.collectionViewModel itemAtIndexPath:index];
@@ -812,6 +899,9 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
     if (updater)
       updater(readingListItem.url);
   }
+  // Leave the batch update while it is not monitored.
+  token.reset();
+  _shouldMonitorModel = YES;
 }
 
 - (void)logDeletionHistogramsForEntry:(const GURL&)url {
@@ -871,7 +961,7 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   }
       completion:^(BOOL) {
         // Reload data to take into account possible sync events.
-        [self reloadData];
+        [self applyPendingUpdates];
       }];
   // As we modified the section in the batch update block, remove the section in
   // another block.
@@ -918,7 +1008,7 @@ using ItemsMapByDate = std::multimap<int64_t, ReadingListCollectionViewItem*>;
   }
       completion:^(BOOL) {
         // Reload data to take into account possible sync events.
-        [self reloadData];
+        [self applyPendingUpdates];
       }];
   // As we modified the section in the batch update block, remove the section in
   // another block.