[ios] DiscoverFeed layout and spinner enhancements

- Prevents the same FeedVC from being re-added to the VC hierarchy,
and if a new FeedVC is set, removes the old one from the VC hierarchy.

- Uses KVO to observe of Feed contentSize changes, so we can reload
the Collection view and display the Feed. Without this the Feed wouldn't
appear after it has been loaded. Credit to adamta@ crrev.com/c/2342580

- Sets a minimum height for the Feed Cell, this way the loading spinner
will always be displayed.


Bug: 1085419
Change-Id: I9d9ef7978a667a2eddc62c0b8e1aad5300185cc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347372
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796853}

ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_discover_item.mm

@@ -8,6 +8,11 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+// The minimum height for the Feed Cell.
+const CGFloat kMinimumFeedCellHeight = 50;
+}
+
 #pragma mark - ContentSuggestionsDiscoverItem
 
 @interface ContentSuggestionsDiscoverItem ()
@@ -36,7 +41,8 @@
 }
 
 - (CGFloat)cellHeightForWidth:(CGFloat)width {
-  return self.lastConfiguredCell ? [self.lastConfiguredCell feedHeight] : 100.0;
+  return self.lastConfiguredCell ? [self.lastConfiguredCell feedHeight]
+                                 : kMinimumFeedCellHeight;
 }
 
 @end
@@ -56,7 +62,6 @@
   if (_discoverFeed == discoverFeed) {
     return;
   }
-  [_discoverFeed.view removeFromSuperview];
   _discoverFeed = discoverFeed;
   if (discoverFeed) {
     UIView* discoverView = discoverFeed.view;
@@ -78,7 +83,7 @@
 - (CGFloat)feedHeight {
   UICollectionView* feedView;
   if (!self.discoverFeed) {
-    return 0;
+    return kMinimumFeedCellHeight;
   }
   // TODO(crbug.com/1092900): Make this more robust. Will require
   // the provider to expose the feed as a UICollectionViewController.
@@ -87,7 +92,9 @@
       feedView = static_cast<UICollectionView*>(view);
     }
   }
-  return feedView.contentSize.height;
+  // In order for the Feed loading spinner to be displayed, the minimum feed
+  // height must be kMinimumFeedCellHeight.
+  return fmax(feedView.contentSize.height, kMinimumFeedCellHeight);
 }
 
 @end

ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm

@@ -71,6 +71,8 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
 
 // The DiscoverFeedVC that might be displayed by this VC.
 @property(nonatomic, weak) UIViewController* discoverFeedVC;
+// The FeedView CollectionView contained by discoverFeedVC.
+@property(nonatomic, strong) UICollectionView* feedView;
 
 @end
 
@@ -396,15 +398,40 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
   if ([self.collectionUpdater
           isDiscoverItem:[self.collectionViewModel
                              itemTypeForIndexPath:indexPath]]) {
+    // TODO(crbug.com/1114792): Remove DiscoverItem logic once we stop
+    // containing the DiscoverFeed inside a cell.
     ContentSuggestionsDiscoverItem* discoverFeedItem =
         static_cast<ContentSuggestionsDiscoverItem*>(item);
-    self.discoverFeedVC = discoverFeedItem.discoverFeed;
-    if (self.discoverFeedVC) {
-      [self addChildViewController:self.discoverFeedVC];
-      UICollectionViewCell* cell = [super collectionView:collectionView
-                                  cellForItemAtIndexPath:indexPath];
-      [self.discoverFeedVC didMoveToParentViewController:self];
-      return cell;
+    UIViewController* newFeedViewController = discoverFeedItem.discoverFeed;
+
+    if (newFeedViewController != self.discoverFeedVC) {
+      // If previous VC is not nil, remove it from the view hierarchy.
+      if (self.discoverFeedVC) {
+        [self.discoverFeedVC willMoveToParentViewController:nil];
+        [self.discoverFeedVC.view removeFromSuperview];
+        [self.discoverFeedVC removeFromParentViewController];
+      }
+
+      // If new VC is not nil, add it to the view hierarchy.
+      if (newFeedViewController) {
+        [self addChildViewController:newFeedViewController];
+        UICollectionViewCell* cell = [super collectionView:collectionView
+                                    cellForItemAtIndexPath:indexPath];
+        [newFeedViewController didMoveToParentViewController:self];
+
+        // Observe its CollectionView for contentSize changes.
+        for (UIView* view in newFeedViewController.view.subviews) {
+          if ([view isKindOfClass:[UICollectionView class]]) {
+            self.feedView = static_cast<UICollectionView*>(view);
+          }
+        }
+        [self.feedView addObserver:self
+                        forKeyPath:@"contentSize"
+                           options:0
+                           context:nil];
+        self.discoverFeedVC = newFeedViewController;
+        return cell;
+      }
     }
   }
 
@@ -729,6 +756,26 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
   return YES;
 }
 
+#pragma mark - ContentSuggestionsConsumer
+
+- (void)setContentSuggestionsEnabled:(BOOL)enabled {
+  _contentSuggestionsEnabled = enabled;
+}
+
+#pragma mark - NSKeyValueObserving
+
+// TODO(crbug.com/1114792): Remove once we stop containing the DiscoverFeed
+// inside a cell.
+- (void)observeValueForKeyPath:(NSString*)keyPath
+                      ofObject:(id)object
+                        change:(NSDictionary*)change
+                       context:(void*)context {
+  if (object == self.feedView && [keyPath isEqualToString:@"contentSize"]) {
+    // Reload the CollectionView data to adjust to the new Feed height.
+    [self.collectionView reloadData];
+  }
+}
+
 #pragma mark - Private
 
 - (void)handleLongPress:(UILongPressGestureRecognizer*)gestureRecognizer {
@@ -819,10 +866,4 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
   [self.discoverFeedMenuHandler openDiscoverFeedMenu:menuButton];
 }
 
-#pragma mark - ContentSuggestionsConsumer
-
-- (void)setContentSuggestionsEnabled:(BOOL)enabled {
-  _contentSuggestionsEnabled = enabled;
-}
-
 @end