Refactor TabStripLegacyCoordinator and TabStripController

This CL:
1- Updates the tabStripLegacyCoordinator to use Browser
   instead of TabModel.
2- Update TabStripController to use Browser instead of tabModel:
  - Change methods & variables to be more suitable to the usage of
    webStateList.
  - Change the type of indices related to the WebStateList to int.
  - Get the dispatchers from the browser instead of getting them
    from the initializer.
  - Remove initializeTabArrayWithNoModel, as there was no way to
    provide the controller no TabModel earlier, which means that
    was a dead code.
  - Updates the test to use TestBrowser instead of the fake tabModel.

Bug: 1049918, 1048683
Change-Id: I4183ce90df9dafd0182c4fa9f03834f21f507cf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2043077
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742894}

ios/chrome/browser/ui/browser_view/browser_view_controller.mm

@@ -2021,11 +2021,9 @@ NSString* const kBrowserViewControllerSnackbarCategory =
         static_cast<id<LoadQueryCommands>>(self.commandDispatcher));
 
   if (IsIPadIdiom()) {
-    self.tabStripCoordinator =
-        [[TabStripLegacyCoordinator alloc] initWithBaseViewController:self];
-    self.tabStripCoordinator.browserState = self.browserState;
-    self.tabStripCoordinator.dispatcher = self.commandDispatcher;
-    self.tabStripCoordinator.tabModel = self.tabModel;
+    self.tabStripCoordinator = [[TabStripLegacyCoordinator alloc]
+        initWithBaseViewController:self
+                           browser:self.browser];
     self.tabStripCoordinator.presentationProvider = self;
     self.tabStripCoordinator.animationWaitDuration =
         kLegacyFullscreenControllerToolbarAnimationDuration;

ios/chrome/browser/ui/tabs/BUILD.gn

@@ -41,6 +41,7 @@ source_set("tabs") {
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/drag_and_drop",
+    "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/snapshots",
     "//ios/chrome/browser/ui:feature_flags",
     "//ios/chrome/browser/ui/bubble",
@@ -80,6 +81,7 @@ source_set("coordinator") {
     ":tabs",
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/ui/bubble",
     "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
@@ -95,8 +97,11 @@ source_set("unit_tests") {
     ":tabs",
     "//base",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/main:public",
+    "//ios/chrome/browser/main:test_support",
     "//ios/chrome/browser/sessions:test_support",
     "//ios/chrome/browser/tabs",
+    "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/ui/util",
     "//ios/chrome/browser/web_state_list",
     "//ios/chrome/browser/web_state_list:test_support",

ios/chrome/browser/ui/tabs/tab_strip_controller.h

@@ -9,11 +9,9 @@
 
 #import "ios/chrome/browser/ui/tabs/requirements/tab_strip_constants.h"
 
-@protocol ApplicationCommands;
-@protocol BrowserCommands;
 @protocol PopupMenuLongPressDelegate;
-@class TabModel;
 @protocol TabStripPresentation;
+class Browser;
 
 // Controller class for the tabstrip.  Manages displaying tabs and keeping the
 // display in sync with the TabModel.  This controller is only instantiated on
@@ -24,8 +22,6 @@
 @property(nonatomic, assign) BOOL highlightsSelectedTab;
 @property(nonatomic, readonly, retain) UIView* view;
 
-@property(nonatomic, readonly, weak) id<BrowserCommands, ApplicationCommands>
-    dispatcher;
 // Delegate for the long press gesture recognizer triggering popup menu.
 @property(nonatomic, weak) id<PopupMenuLongPressDelegate> longPressDelegate;
 
@@ -37,11 +33,8 @@
 @property(nonatomic, assign) id<TabStripPresentation> presentationProvider;
 
 // Designated initializer, |dispatcher| is not retained.
-- (instancetype)initWithTabModel:(TabModel*)tabModel
-                           style:(TabStripStyle)style
-                      dispatcher:
-                          (id<ApplicationCommands, BrowserCommands>)dispatcher
-    NS_DESIGNATED_INITIALIZER;
+- (instancetype)initWithBrowser:(Browser*)browser
+                          style:(TabStripStyle)style NS_DESIGNATED_INITIALIZER;
 
 - (instancetype)init NS_UNAVAILABLE;
 

ios/chrome/browser/ui/tabs/tab_strip_controller_unittest.mm

@@ -6,15 +6,16 @@
 
 #include <memory>
 
-#include "base/strings/sys_string_conversions.h"
-#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#include "base/strings/utf_string_conversions.h"
+#import "ios/chrome/browser/main/test_browser.h"
 #import "ios/chrome/browser/sessions/test_session_service.h"
-#import "ios/chrome/browser/tabs/tab_model.h"
+#import "ios/chrome/browser/ui/commands/application_commands.h"
+#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
+#import "ios/chrome/browser/ui/commands/popup_menu_commands.h"
 #import "ios/chrome/browser/ui/tabs/tab_strip_controller.h"
 #import "ios/chrome/browser/ui/tabs/tab_strip_view.h"
 #import "ios/chrome/browser/ui/tabs/tab_view.h"
 #include "ios/chrome/browser/ui/util/ui_util.h"
-#import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/chrome/browser/web_state_list/web_state_opener.h"
 #import "ios/web/public/navigation/navigation_item.h"
@@ -31,65 +32,6 @@
 #error "This file requires ARC support."
 #endif
 
-@interface TabStripControllerTestTabModel : NSObject
-
-@property(nonatomic, assign) ChromeBrowserState* browserState;
-
-@end
-
-@implementation TabStripControllerTestTabModel {
-  FakeWebStateListDelegate _webStateListDelegate;
-  std::unique_ptr<WebStateList> _webStateList;
-  std::unique_ptr<web::NavigationItem> _visibleNavigationItem;
-}
-
-@synthesize browserState = _browserState;
-
-- (instancetype)init {
-  if ((self = [super init])) {
-    _webStateList = std::make_unique<WebStateList>(&_webStateListDelegate);
-    _visibleNavigationItem = web::NavigationItem::Create();
-  }
-  return self;
-}
-
-- (void)browserStateDestroyed {
-  _webStateList->CloseAllWebStates(WebStateList::CLOSE_NO_FLAGS);
-  _browserState = nullptr;
-}
-
-- (void)addWebStateForTestingWithTitle:(NSString*)title {
-  auto testWebState = std::make_unique<web::TestWebState>();
-  testWebState->SetTitle(base::SysNSStringToUTF16(title));
-  auto testNavigationManager = std::make_unique<web::TestNavigationManager>();
-  testNavigationManager->SetVisibleItem(_visibleNavigationItem.get());
-  testWebState->SetNavigationManager(std::move(testNavigationManager));
-  _webStateList->InsertWebState(0, std::move(testWebState),
-                                WebStateList::INSERT_NO_FLAGS,
-                                WebStateOpener());
-}
-
-- (BOOL)isEmpty {
-  return _webStateList->empty();
-}
-
-- (NSUInteger)count {
-  return static_cast<NSUInteger>(_webStateList->count());
-}
-
-- (void)closeTabAtIndex:(NSUInteger)index {
-  DCHECK(index < static_cast<NSUInteger>(INT_MAX));
-  DCHECK(static_cast<int>(index) < _webStateList->count());
-  _webStateList->CloseWebStateAt(static_cast<int>(index),
-                                 WebStateList::CLOSE_NO_FLAGS);
-}
-
-- (WebStateList*)webStateList {
-  return _webStateList.get();
-}
-
-@end
-
 namespace {
 
 class TabStripControllerTest : public PlatformTest {
@@ -98,40 +40,52 @@ class TabStripControllerTest : public PlatformTest {
     if (!IsIPadIdiom())
       return;
 
-    // Need a ChromeBrowserState for the tab model.
-    TestChromeBrowserState::Builder test_cbs_builder;
-    chrome_browser_state_ = test_cbs_builder.Build();
-
-    // Setup mock TabModel.
-    tab_model_ = [[TabStripControllerTestTabModel alloc] init];
-    tab_model_.browserState = chrome_browser_state_.get();
-
-    // Populate the TabModel.
-    [tab_model_ addWebStateForTestingWithTitle:@"Tab Title 1"];
-    [tab_model_ addWebStateForTestingWithTitle:@"Tab Title 2"];
-
-    controller_ = [[TabStripController alloc]
-        initWithTabModel:static_cast<TabModel*>(tab_model_)
-                   style:NORMAL
-              dispatcher:nil];
-
-    // Force the view to load.
-    UIWindow* window = [[UIWindow alloc] initWithFrame:CGRectZero];
-    [window addSubview:[controller_ view]];
-    window_ = window;
+    visible_navigation_item_ = web::NavigationItem::Create();
+    mock_application_commands_handler_ =
+        OCMStrictProtocolMock(@protocol(ApplicationCommands));
+    mock_popup_menu_commands_handler_ =
+        OCMStrictProtocolMock(@protocol(PopupMenuCommands));
+    mock_application_settings_commands_handler_ =
+        OCMStrictProtocolMock(@protocol(ApplicationSettingsCommands));
+
+    [browser_.GetCommandDispatcher()
+        startDispatchingToTarget:mock_application_commands_handler_
+                     forProtocol:@protocol(ApplicationCommands)];
+    [browser_.GetCommandDispatcher()
+        startDispatchingToTarget:mock_application_settings_commands_handler_
+                     forProtocol:@protocol(ApplicationSettingsCommands)];
+    [browser_.GetCommandDispatcher()
+        startDispatchingToTarget:mock_popup_menu_commands_handler_
+                     forProtocol:@protocol(PopupMenuCommands)];
+
+    controller_ = [[TabStripController alloc] initWithBrowser:&browser_
+                                                        style:NORMAL];
   }
 
   void TearDown() override {
     if (!IsIPadIdiom())
       return;
-
-    [tab_model_ browserStateDestroyed];
     [controller_ disconnect];
   }
 
+  void AddWebStateForTesting(std::string title) {
+    auto test_web_state = std::make_unique<web::TestWebState>();
+    test_web_state->SetTitle(base::UTF8ToUTF16(title));
+    auto test_navigation_manager =
+        std::make_unique<web::TestNavigationManager>();
+    test_navigation_manager->SetVisibleItem(visible_navigation_item_.get());
+    test_web_state->SetNavigationManager(std::move(test_navigation_manager));
+    browser_.GetWebStateList()->InsertWebState(
+        /*index=*/0, std::move(test_web_state), WebStateList::INSERT_NO_FLAGS,
+        WebStateOpener());
+  }
+
   web::WebTaskEnvironment task_environment_;
-  std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
-  TabStripControllerTestTabModel* tab_model_;
+  std::unique_ptr<web::NavigationItem> visible_navigation_item_;
+  TestBrowser browser_;
+  id mock_application_commands_handler_;
+  id mock_popup_menu_commands_handler_;
+  id mock_application_settings_commands_handler_;
   TabStripController* controller_;
   UIWindow* window_;
 };
@@ -139,6 +93,12 @@ class TabStripControllerTest : public PlatformTest {
 TEST_F(TabStripControllerTest, LoadAndDisplay) {
   if (!IsIPadIdiom())
     return;
+  AddWebStateForTesting("Tab Title 1");
+  AddWebStateForTesting("Tab Title 2");
+  // Force the view to load.
+  UIWindow* window = [[UIWindow alloc] initWithFrame:CGRectZero];
+  [window addSubview:[controller_ view]];
+  window_ = window;
 
   // There should be two TabViews and one new tab button nested within the
   // parent view (which contains exactly one scroll view).

ios/chrome/browser/ui/tabs/tab_strip_legacy_coordinator.h

@@ -10,29 +10,25 @@
 #import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h"
 #import "ios/chrome/browser/ui/tabs/requirements/tab_strip_highlighting.h"
 
-@protocol ApplicationCommands;
-@protocol BrowserCommands;
-class ChromeBrowserState;
 @protocol PopupMenuLongPressDelegate;
-@class TabModel;
 @protocol TabStripPresentation;
 
 // A legacy coordinator that presents the public interface for the tablet tab
 // strip feature.
 @interface TabStripLegacyCoordinator : ChromeCoordinator<TabStripHighlighting>
 
-// BrowserState for this coordinator.
-@property(nonatomic, assign) ChromeBrowserState* browserState;
+// Use Browser based init. -initWithBaseViewController:browser:
+- (instancetype)initWithBaseViewController:(UIViewController*)viewController
+    NS_UNAVAILABLE;
 
-// Dispatcher for sending commands.
-@property(nonatomic, weak) id dispatcher;
+// Use Browser based init. -initWithBaseViewController:browser:
+- (instancetype)initWithBaseViewController:(UIViewController*)viewController
+                              browserState:(ChromeBrowserState*)browserState
+    NS_UNAVAILABLE;
 
 // Delegate for the long press gesture recognizer triggering popup menu.
 @property(nonatomic, weak) id<PopupMenuLongPressDelegate> longPressDelegate;
 
-// Model layer for the tab strip.
-@property(nonatomic, weak) TabModel* tabModel;
-
 // Provides methods for presenting the tab strip and checking the visibility
 // of the tab strip in the containing object.
 @property(nonatomic, assign) id<TabStripPresentation> presentationProvider;

ios/chrome/browser/ui/tabs/tab_strip_legacy_coordinator.mm

@@ -3,8 +3,8 @@
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/tabs/tab_strip_legacy_coordinator.h"
-
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/ui/tabs/requirements/tab_strip_presentation.h"
 #import "ios/chrome/browser/ui/tabs/tab_strip_controller.h"
 
@@ -18,25 +18,12 @@
 @end
 
 @implementation TabStripLegacyCoordinator
-@synthesize browserState = _browserState;
-@synthesize dispatcher = _dispatcher;
 @synthesize longPressDelegate = _longPressDelegate;
-@synthesize tabModel = _tabModel;
 @synthesize presentationProvider = _presentationProvider;
 @synthesize started = _started;
 @synthesize tabStripController = _tabStripController;
 @synthesize animationWaitDuration = _animationWaitDuration;
 
-- (void)setDispatcher:(id<BrowserCommands, ApplicationCommands>)dispatcher {
-  DCHECK(!self.started);
-  _dispatcher = dispatcher;
-}
-
-- (void)setTabModel:(TabModel*)tabModel {
-  DCHECK(!self.started);
-  _tabModel = tabModel;
-}
-
 - (void)setLongPressDelegate:(id<PopupMenuLongPressDelegate>)longPressDelegate {
   _longPressDelegate = longPressDelegate;
   self.tabStripController.longPressDelegate = longPressDelegate;
@@ -69,16 +56,12 @@
 #pragma mark - ChromeCoordinator
 
 - (void)start {
-  DCHECK(self.browserState);
-  DCHECK(self.tabModel);
-  DCHECK(self.dispatcher);
+  DCHECK(self.browser);
   DCHECK(self.presentationProvider);
   TabStripStyle style =
-      self.browserState->IsOffTheRecord() ? INCOGNITO : NORMAL;
+      self.browser->GetBrowserState()->IsOffTheRecord() ? INCOGNITO : NORMAL;
   self.tabStripController =
-      [[TabStripController alloc] initWithTabModel:self.tabModel
-                                             style:style
-                                        dispatcher:self.dispatcher];
+      [[TabStripController alloc] initWithBrowser:self.browser style:style];
   self.tabStripController.presentationProvider = self.presentationProvider;
   self.tabStripController.animationWaitDuration = self.animationWaitDuration;
   self.tabStripController.longPressDelegate = self.longPressDelegate;
@@ -90,8 +73,6 @@
   self.started = NO;
   [self.tabStripController disconnect];
   self.tabStripController = nil;
-  self.dispatcher = nil;
-  self.tabModel = nil;
   self.presentationProvider = nil;
 }