[ios] Wait for sync query to return before displaying history

ios/chrome/browser/ui/history/history_collection_view_controller.mm

Index: ios/chrome/browser/ui/history/history_collection_view_controller.mm
diff --git a/ios/chrome/browser/ui/history/history_collection_view_controller.mm b/ios/chrome/browser/ui/history/history_collection_view_controller.mm
index c2c010360832627a6b280f0861faedca1db39484..f0a2e1cd5b6ad8bc1b9551b95d6d3ee295be62da 100644
--- a/ios/chrome/browser/ui/history/history_collection_view_controller.mm
+++ b/ios/chrome/browser/ui/history/history_collection_view_controller.mm
@@ -21,6 +21,8 @@
 #include "ios/chrome/browser/chrome_url_constants.h"
 #import "ios/chrome/browser/signin/authentication_service.h"
 #include "ios/chrome/browser/signin/authentication_service_factory.h"
+#include "ios/chrome/browser/sync/sync_setup_service.h"
+#include "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #import "ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.h"
 #import "ios/chrome/browser/ui/collection_view/cells/activity_indicator_cell.h"
 #import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h"
@@ -107,6 +109,9 @@ const CGFloat kSeparatorInset = 10;
 - (void)removeSelectedItemsFromCollection;
 // Removes all items in the collection that are not included in entries.
 - (void)filterForHistoryEntries:(NSArray*)entries;
+// Adds loading indicator to the top of the history collection, if one is not
+// already present.
+- (void)addLoadingIndicator;
 // Displays context menu on cell pressed with gestureRecognizer.
 - (void)displayContextMenuInvokedByGestureRecognizer:
     (UILongPressGestureRecognizer*)gestureRecognizer;
@@ -315,15 +320,15 @@ const CGFloat kSeparatorInset = 10;
 - (void)historyServiceFacade:(HistoryServiceFacade*)facade
        didReceiveQueryResult:(HistoryServiceFacade::QueryResult)result {
   self.loading = NO;
-  // Remove loading indicator.
-  CollectionViewItem* headerItem = [self.collectionViewModel
-      itemAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]];
-  if ([headerItem.cellClass isSubclassOfClass:[ActivityIndicatorCell class]]) {
-    [self.collectionViewModel removeItemWithType:kItemTypeEnumZero
-                       fromSectionWithIdentifier:kSectionIdentifierEnumZero];
-    [self.collectionView
-        deleteItemsAtIndexPaths:@[ [NSIndexPath indexPathForItem:0
-                                                       inSection:0] ]];
+  // If history sync is enabled and there hasn't been a response from synced
+  // history, try fetching again.
+  SyncSetupService* syncSetupService =
+      SyncSetupServiceFactory::GetForBrowserState(_browserState);
+  if (syncSetupService->IsSyncEnabled() &&
+      syncSetupService->IsDataTypeEnabled(syncer::HISTORY_DELETE_DIRECTIVES) &&
+      !result.sync_returned) {
+    [self showHistoryMatchingQuery:_currentQuery];
+    return;
   }
 
   // If there are no results and no URLs have been loaded, report that no
@@ -526,24 +531,9 @@ const CGFloat kSeparatorInset = 10;
 - (void)fetchHistoryForQuery:(NSString*)query
                  priorToTime:(const base::Time&)time {
   self.loading = YES;
-  // Add loading indicator if nothing else is shown.
+  // Add loading indicator if no items are shown.
   if (!self.hasHistoryEntries && !self.isSearching) {
-    [self.collectionView performBatchUpdates:^{
-      NSIndexPath* indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
-      if ([self.collectionViewModel hasItemAtIndexPath:indexPath]) {
-        [self.collectionViewModel
-                   removeItemWithType:kItemTypeEnumZero
-            fromSectionWithIdentifier:kSectionIdentifierEnumZero];
-        [self.collectionView deleteItemsAtIndexPaths:@[ indexPath ]];
-      }
-      CollectionViewItem* loadingIndicatorItem = [[[CollectionViewItem alloc]
-          initWithType:kItemTypeEnumZero] autorelease];
-      loadingIndicatorItem.cellClass = [ActivityIndicatorCell class];
-      [self.collectionViewModel addItem:loadingIndicatorItem
-                toSectionWithIdentifier:kEntriesStatusSectionIdentifier];
-      [self.collectionView insertItemsAtIndexPaths:@[ indexPath ]];
-    }
-                                  completion:nil];
+    [self addLoadingIndicator];
   }
 
   BOOL fetchAllHistory = !query || [query isEqualToString:@""];
@@ -557,7 +547,6 @@ const CGFloat kSeparatorInset = 10;
   options.max_count = kMaxFetchCount;
   options.matching_algorithm =
       query_parser::MatchingAlgorithm::ALWAYS_PREFIX_SEARCH;
-  _historyServiceFacade->QueryOtherFormsOfBrowsingHistory();
   _historyServiceFacade->QueryHistory(queryString, options);
   // Also determine whether notice regarding other forms of browsing history
   // should be shown.
@@ -667,6 +656,31 @@ const CGFloat kSeparatorInset = 10;
   [self removeSelectedItemsFromCollection];
 }
 
+- (void)addLoadingIndicator {
+  NSIndexPath* indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
+  if ([self.collectionViewModel hasItemAtIndexPath:indexPath] &&
+      [[[self.collectionViewModel itemAtIndexPath:indexPath] cellClass]
+          isSubclassOfClass:[ActivityIndicatorCell class]]) {
+    // Do not add indicator a sectond time.

[lpromero, 2016/12/20 23:13:42]

second*

Can't you look instead in the model then ?

[Jackie Quinn, 2016/12/20 23:18:10]

Done. There is no ActivityIndicatorItem, only the cell.

[lpromero, 2016/12/20 23:26:23]

That's where giving a specific type to the item can come in handy I guess. Would
it be worth it here?

[Jackie Quinn, 2016/12/20 23:33:21]

If the sole purpose is checking for the type of item, I feel like an extra class
is overkill here when you can just check the cellClass. If you feel differently
let me know.

[lpromero, 2016/12/20 23:38:51]

I meant just the enum value you pass to the initializer -initWithType:, not the
type in the sense of compiler. I agree that we shouldn't need a subclass.

[Jackie Quinn, 2016/12/21 00:02:05]

Oh, I see. Yeah that sounds reasonable. Updated!

+    return;
+  }
+
+  [self.collectionView performBatchUpdates:^{
+    if ([self.collectionViewModel hasItemAtIndexPath:indexPath]) {
+      [self.collectionViewModel removeItemWithType:kItemTypeEnumZero
+                         fromSectionWithIdentifier:kSectionIdentifierEnumZero];
+      [self.collectionView deleteItemsAtIndexPaths:@[ indexPath ]];
+    }
+    CollectionViewItem* loadingIndicatorItem = [[[CollectionViewItem alloc]
+        initWithType:kItemTypeEnumZero] autorelease];
+    loadingIndicatorItem.cellClass = [ActivityIndicatorCell class];
+    [self.collectionViewModel addItem:loadingIndicatorItem
+              toSectionWithIdentifier:kEntriesStatusSectionIdentifier];
+    [self.collectionView insertItemsAtIndexPaths:@[ indexPath ]];
+  }
+                                completion:nil];
+}
+
 #pragma mark Context Menu
 
 - (void)displayContextMenuInvokedByGestureRecognizer: