[ios][Thumb Strip] Switch BVCs when switching tab grid pages

When using the thumb strip, when the user is in the tab grid, they can
swipe the BVC up from the bottom of the screen to go back to it at any
time. Because of this, when switching tab grid pages, the BVC at the
bottom of the screen has to switch as well.

In general, the idea is that the BVCContainerViewController will always
be present. In the tab grid, if the user is on a page with a tab, it
the container will have the appropriate BVC. If they are on a page
without a tab (Recent Tabs or incognito with no open incognito tabs), it
will have no BVC but still be onscreen, ready to be filled.

All of the tab grid/scene controller presentation methods now take an
additional argument of whether to dismiss the tab grid after activating
the given Browser/UIViewController. This allows the tab grid to request
changes to only the active BVC without leading to dismissal of the tab
grid.

Bug: 1094335
Change-Id: I876ccaf01416caccc3bb85cccc5701527a45840a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577462
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837059}

ios/chrome/browser/ui/main/bvc_container_view_controller.mm

@@ -3,11 +3,11 @@
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/main/bvc_container_view_controller.h"
-#import "ios/chrome/browser/ui/thumb_strip/thumb_strip_feature.h"
 
 #include <ostream>
 
 #include "base/check_op.h"
+#import "ios/chrome/browser/ui/thumb_strip/thumb_strip_feature.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -22,7 +22,10 @@
 }
 
 - (void)setCurrentBVC:(UIViewController*)bvc {
-  DCHECK(bvc);
+  // When the thumb strip is enabled, the BVC container stays around all the
+  // time. When on a tab grid page with no tabs or the recent tab page, the
+  // currentBVC will be set to nil.
+  DCHECK(bvc || IsThumbStripEnabled());
   if (self.currentBVC == bvc) {
     return;
   }
@@ -38,15 +41,22 @@
   DCHECK_EQ(0U, self.view.subviews.count);
 
   // Add the new active view controller.
-  [self addChildViewController:bvc];
-  bvc.view.frame = self.view.bounds;
-  [self.view addSubview:bvc.view];
-  [bvc didMoveToParentViewController:self];
-
-  if (IsThumbStripEnabled()) {
-    // The background needs to be clear to allow the thumb strip to be seen
-    // during the enter/exit thumb strip animation.
-    self.currentBVC.view.backgroundColor = [UIColor clearColor];
+  if (bvc) {
+    [self addChildViewController:bvc];
+    // If the BVC's view has a transform, then its frame isn't accurate.
+    // Instead, remove the transform, set the frame, then reapply the transform.
+    CGAffineTransform oldTransform = bvc.view.transform;
+    bvc.view.transform = CGAffineTransformIdentity;
+    bvc.view.frame = self.view.bounds;
+    bvc.view.transform = oldTransform;
+    [self.view addSubview:bvc.view];
+    [bvc didMoveToParentViewController:self];
+
+    if (IsThumbStripEnabled()) {
+      // The background needs to be clear to allow the thumb strip to be seen
+      // during the enter/exit thumb strip animation.
+      self.currentBVC.view.backgroundColor = [UIColor clearColor];
+    }
   }
 
   DCHECK(self.currentBVC == bvc);

ios/chrome/browser/ui/tab_switcher/tab_grid/grid/grid_view_controller.mm

@@ -768,7 +768,9 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
       break;
   }
   auto completionBlock = ^(BOOL completed, BOOL finished) {
-    completion(completed, finished);
+    if (completion) {
+      completion(completed, finished);
+    }
     self.collectionView.scrollEnabled = YES;
     self.currentLayout = nextLayout;
   };

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_coordinator.h

@@ -61,10 +61,14 @@ class Browser;
 // Displays the TabGrid.
 - (void)showTabGrid;
 
-// Displays the given view controller, replacing any TabSwitchers or other view
-// controllers that may currently be visible.  Runs the given |completion| block
-// after the view controller is visible.
+// Displays the given view controller. If |closeTabGrid| is yes, any
+// TabSwitchers or other view controllers that may currently be visible will be
+// replaced. Otherwise, the view controller is added to the current container.
+// Runs the given |completion| block after the view controller is visible.
+// |shouldCloseTabGrid| is only used for the thumb strip, where the
+// tab container view controller is never dismissed.
 - (void)showTabViewController:(UIViewController*)viewController
+           shouldCloseTabGrid:(BOOL)shouldCloseTabGrid
                    completion:(ProceduralBlock)completion;
 
 // Sets the |page| as the active (visible) one. The active page must not be the

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_coordinator.mm

@@ -228,11 +228,9 @@
 }
 
 - (void)showTabViewController:(UIViewController*)viewController
+           shouldCloseTabGrid:(BOOL)shouldCloseTabGrid
                    completion:(ProceduralBlock)completion {
-  DCHECK(viewController);
-
-  // Record when the tab switcher is dismissed.
-  base::RecordAction(base::UserMetricsAction("MobileTabGridExited"));
+  DCHECK(viewController || (IsThumbStripEnabled() && self.bvcContainer));
 
   // If thumb strip is enabled, this will always be true except during initial
   // setup before the BVC container has been created.
@@ -241,12 +239,17 @@
     self.baseViewController.childViewControllerForStatusBarStyle =
         viewController;
     [self.baseViewController setNeedsStatusBarAppearanceUpdate];
-    [self.thumbStripCoordinator.panHandler setState:ViewRevealState::Hidden
-                                           animated:YES];
+    if (shouldCloseTabGrid) {
+      [self.thumbStripCoordinator.panHandler setState:ViewRevealState::Hidden
+                                             animated:YES];
+      [self.delegate tabGridDismissTransitionDidEnd:self];
+
+      // Record when the tab switcher is dismissed.
+      base::RecordAction(base::UserMetricsAction("MobileTabGridExited"));
+    }
     if (completion) {
       completion();
     }
-    [self.delegate tabGridDismissTransitionDidEnd:self];
     return;
   }
 
@@ -448,19 +451,40 @@
 
 #pragma mark - TabPresentationDelegate
 
-- (void)showActiveTabInPage:(TabGridPage)page focusOmnibox:(BOOL)focusOmnibox {
+- (void)showActiveTabInPage:(TabGridPage)page
+               focusOmnibox:(BOOL)focusOmnibox
+               closeTabGrid:(BOOL)closeTabGrid {
   DCHECK(self.regularBrowser && self.incognitoBrowser);
+  DCHECK(closeTabGrid || IsThumbStripEnabled());
   Browser* activeBrowser = nullptr;
   switch (page) {
     case TabGridPageIncognitoTabs:
-      DCHECK_GT(self.incognitoBrowser->GetWebStateList()->count(), 0);
+      if (self.incognitoBrowser->GetWebStateList()->count() == 0) {
+        DCHECK(IsThumbStripEnabled());
+        [self showTabViewController:nil
+                 shouldCloseTabGrid:closeTabGrid
+                         completion:nil];
+        return;
+      }
       activeBrowser = self.incognitoBrowser;
       break;
     case TabGridPageRegularTabs:
-      DCHECK_GT(self.regularBrowser->GetWebStateList()->count(), 0);
+      if (self.regularBrowser->GetWebStateList()->count() == 0) {
+        DCHECK(IsThumbStripEnabled());
+        [self showTabViewController:nil
+                 shouldCloseTabGrid:closeTabGrid
+                         completion:nil];
+        return;
+      }
       activeBrowser = self.regularBrowser;
       break;
     case TabGridPageRemoteTabs:
+      if (IsThumbStripEnabled()) {
+        [self showTabViewController:nil
+                 shouldCloseTabGrid:closeTabGrid
+                         completion:nil];
+        return;
+      }
       NOTREACHED() << "It is invalid to have an active tab in remote tabs.";
       // This appears to come up in release -- see crbug.com/1069243.
       // Defensively early return instead of continuing.
@@ -469,8 +493,9 @@
   // Trigger the transition through the delegate. This will in turn call back
   // into this coordinator.
   [self.delegate tabGrid:self
-      shouldFinishWithBrowser:activeBrowser
-                 focusOmnibox:focusOmnibox];
+      shouldActivateBrowser:activeBrowser
+             dismissTabGrid:closeTabGrid
+               focusOmnibox:focusOmnibox];
 }
 
 - (void)showCloseAllConfirmationActionSheetWitTabGridMediator:
@@ -538,22 +563,25 @@
 
 - (void)showActiveRegularTabFromRecentTabs {
   [self.delegate tabGrid:self
-      shouldFinishWithBrowser:self.regularBrowser
-                 focusOmnibox:NO];
+      shouldActivateBrowser:self.regularBrowser
+             dismissTabGrid:YES
+               focusOmnibox:NO];
 }
 
 #pragma mark - HistoryPresentationDelegate
 
 - (void)showActiveRegularTabFromHistory {
   [self.delegate tabGrid:self
-      shouldFinishWithBrowser:self.regularBrowser
-                 focusOmnibox:NO];
+      shouldActivateBrowser:self.regularBrowser
+             dismissTabGrid:YES
+               focusOmnibox:NO];
 }
 
 - (void)showActiveIncognitoTabFromHistory {
   [self.delegate tabGrid:self
-      shouldFinishWithBrowser:self.incognitoBrowser
-                 focusOmnibox:NO];
+      shouldActivateBrowser:self.incognitoBrowser
+             dismissTabGrid:YES
+               focusOmnibox:NO];
 }
 
 - (void)openAllTabsFromSession:(const synced_sessions::DistantSession*)session {

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_coordinator_delegate.h

@@ -14,11 +14,14 @@ class Browser;
 // when the presentation and dismmiss animations finishes.
 @protocol TabGridCoordinatorDelegate
 
-// Informs the delegate the tab switcher should be dismissed with the given
-// active browser.
+// Informs the delegate the tab switcher that the given browser should be set to
+// active. If |dismissTabGrid| is YES, the tab grid itself should also be
+// dismissed. This should always be the case except when using the thumb strip,
+// where the tab grid is never dismissed
 - (void)tabGrid:(TabGridCoordinator*)tabGrid
-    shouldFinishWithBrowser:(Browser*)browser
-               focusOmnibox:(BOOL)focusOmnibox;
+    shouldActivateBrowser:(Browser*)browser
+           dismissTabGrid:(BOOL)dismissTabGrid
+             focusOmnibox:(BOOL)focusOmnibox;
 
 // Informs the delegate that the tab switcher is done and should be dismissed.
 - (void)tabGridDismissTransitionDidEnd:(TabGridCoordinator*)tabGrid;

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_coordinator_unittest.mm

@@ -46,8 +46,9 @@
 @implementation TestTabGridCoordinatorDelegate
 @synthesize didEndCalled = _didEndCalled;
 - (void)tabGrid:(TabGridCoordinator*)tabGrid
-    shouldFinishWithBrowser:(Browser*)browser
-               focusOmnibox:(BOOL)focusOmnibox {
+    shouldActivateBrowser:(Browser*)browser
+           dismissTabGrid:(BOOL)dismissTabGrid
+             focusOmnibox:(BOOL)focusOmnibox {
   // No-op.
 }
 
@@ -154,6 +155,7 @@ TEST_F(TabGridCoordinatorTest, InitialActiveViewController) {
 // TabSwitcher.
 TEST_F(TabGridCoordinatorTest, TabViewControllerBeforeTabSwitcher) {
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);
 
@@ -174,6 +176,7 @@ TEST_F(TabGridCoordinatorTest, TabViewControllerAfterTabSwitcher) {
   EXPECT_EQ(coordinator_.baseViewController, coordinator_.activeViewController);
 
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);
 
@@ -190,10 +193,12 @@ TEST_F(TabGridCoordinatorTest, TabViewControllerAfterTabSwitcher) {
 // Tests swapping between two TabViewControllers.
 TEST_F(TabGridCoordinatorTest, SwapTabViewControllers) {
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);
 
   [coordinator_ showTabViewController:incognito_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(incognito_tab_view_controller_, coordinator_.activeViewController);
 }
@@ -210,10 +215,12 @@ TEST_F(TabGridCoordinatorTest, ShowTabSwitcherTwice) {
 // Tests calling showTabViewController twice in a row with the same VC.
 TEST_F(TabGridCoordinatorTest, ShowTabViewControllerTwice) {
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);
 
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:nil];
   EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);
 }
@@ -229,6 +236,7 @@ TEST_F(TabGridCoordinatorTest, CompletionHandlers) {
   delegate_.didEndCalled = NO;
   __block BOOL completion_handler_was_called = NO;
   [coordinator_ showTabViewController:normal_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:^{
                              completion_handler_was_called = YES;
                            }];
@@ -242,6 +250,7 @@ TEST_F(TabGridCoordinatorTest, CompletionHandlers) {
   // view controller. Tests that the delegate 'didEnd' method is *not* called.
   delegate_.didEndCalled = NO;
   [coordinator_ showTabViewController:incognito_tab_view_controller_
+                   shouldCloseTabGrid:YES
                            completion:^{
                              completion_handler_was_called = YES;
                            }];

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_view_controller.h

@@ -26,8 +26,14 @@
 // from the tab grid.
 @protocol TabPresentationDelegate <NSObject>
 // Show the active tab in |page|, presented on top of the tab grid.  The
-// omnibox will be focused after the animation if |focusOmnibox| is YES.
-- (void)showActiveTabInPage:(TabGridPage)page focusOmnibox:(BOOL)focusOmnibox;
+// omnibox will be focused after the animation if |focusOmnibox| is YES. If
+// |closeTabGrid| is NO, then the tab grid will not be closed, and the active
+// tab will simply be displayed in its current position.
+// This last parameter is used for the thumb strip, where the
+// BVCContainerViewController is never dismissed.
+- (void)showActiveTabInPage:(TabGridPage)page
+               focusOmnibox:(BOOL)focusOmnibox
+               closeTabGrid:(BOOL)closeTabGrid;
 @end
 
 // View controller representing a tab switcher. The tab switcher has an

ios/chrome/browser/ui/tab_switcher/tab_grid/tab_grid_view_controller.mm

@@ -288,11 +288,13 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
     [self broadcastIncognitoContentVisibility];
     [self configureButtonsForActiveAndCurrentPage];
   }
+  [self arriveAtCurrentPage];
 }
 
 - (void)scrollViewDidEndScrollingAnimation:(UIScrollView*)scrollView {
   self.currentPage = GetPageFromScrollView(scrollView);
   self.scrollViewAnimatingContentOffset = NO;
+  [self arriveAtCurrentPage];
   [self broadcastIncognitoContentVisibility];
   [self configureButtonsForActiveAndCurrentPage];
 }
@@ -467,12 +469,15 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
                         (void (^)(BOOL completed, BOOL finished))completion {
   GridViewController* regularViewController =
       [self gridViewControllerForPage:TabGridPageRegularTabs];
-  [regularViewController willTransitionToLayout:nextState
-                                     completion:completion];
   GridViewController* incognitoViewController =
       [self gridViewControllerForPage:TabGridPageIncognitoTabs];
-  [incognitoViewController willTransitionToLayout:nextState
-                                       completion:completion];
+
+  // Each LayoutSwitcher method calls regular and icognito grid controller's
+  // corresponding method. Thus, attaching the completion to only one of the
+  // grid view controllers should suffice.
+  [regularViewController willTransitionToLayout:nextState
+                                     completion:completion];
+  [incognitoViewController willTransitionToLayout:nextState completion:nil];
 }
 
 - (void)didUpdateTransitionLayoutProgress:(CGFloat)progress {
@@ -726,6 +731,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
     // Important updates (e.g., button configurations, incognito visibility) are
     // made at the end of scrolling animations after |self.currentPage| is set.
     // Since this codepath has no animations, updates must be called manually.
+    [self arriveAtCurrentPage];
     [self broadcastIncognitoContentVisibility];
     [self configureButtonsForActiveAndCurrentPage];
   } else {
@@ -751,6 +757,19 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   }
 }
 
+// Updates the state when the scroll view stops scrolling at a given page,
+// whether the scroll is from dragging or programmatic.
+- (void)arriveAtCurrentPage {
+  if (!self.viewVisible) {
+    return;
+  }
+  if (IsThumbStripEnabled()) {
+    [self.tabPresentationDelegate showActiveTabInPage:self.currentPage
+                                         focusOmnibox:NO
+                                         closeTabGrid:NO];
+  }
+}
+
 - (UIViewController*)currentPageViewController {
   switch (self.currentPage) {
     case TabGridPageIncognitoTabs:
@@ -1287,7 +1306,8 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   }
   self.activePage = page;
   [self.tabPresentationDelegate showActiveTabInPage:page
-                                       focusOmnibox:focusOmnibox];
+                                       focusOmnibox:focusOmnibox
+                                       closeTabGrid:YES];
 }
 
 // Creates and shows a new regular tab.
@@ -1376,7 +1396,8 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   }
   self.activePage = self.currentPage;
   [self.tabPresentationDelegate showActiveTabInPage:self.currentPage
-                                       focusOmnibox:NO];
+                                       focusOmnibox:NO
+                                       closeTabGrid:YES];
   gridViewController.showsSelectionUpdates = YES;
 }
 
@@ -1493,7 +1514,8 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   // disabled. Ensure that action is only taken on a valid state.
   if (![[self gridViewControllerForPage:newActivePage] isGridEmpty]) {
     [self.tabPresentationDelegate showActiveTabInPage:newActivePage
-                                         focusOmnibox:NO];
+                                         focusOmnibox:NO
+                                         closeTabGrid:YES];
     // Record when users exit the tab grid to return to the current foreground
     // tab.
     base::RecordAction(base::UserMetricsAction("MobileTabGridDone"));