[iOS] Modernize Recent Tabs coordinator

Update the RecentTabsCooordinator to use the preferred initializer.

Most of the change volume comes from adopting the preferred dispatcher/
handler style and usage. This is especially painful in the unittest,
since the dispatcher in the test browser needs to conform to several
protocols to pass the runtime checks used by HandlerForProtocol().

This probably means that the large command protocols, especially those
that conform to several other protocols, need to be made more specific.

Bug: 1029346
Change-Id: I69baf57ce162799b5af4ec6fdfe19e7bdc787d9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944468Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721412}

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

@@ -482,10 +482,8 @@
 
   self.recentTabsCoordinator = [[RecentTabsCoordinator alloc]
       initWithBaseViewController:self.viewController
-                    browserState:self.browserState];
+                         browser:self.browser];
   self.recentTabsCoordinator.loadStrategy = UrlLoadStrategy::NORMAL;
-  self.recentTabsCoordinator.dispatcher = self.applicationCommandHandler;
-  self.recentTabsCoordinator.webStateList = self.browser->GetWebStateList();
   [self.recentTabsCoordinator start];
 }
 

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

@@ -23,7 +23,8 @@ source_set("recent_tabs") {
     "//components/sync",
     "//ios/chrome/app/strings",
     "//ios/chrome/browser/browser_state",
-    "//ios/chrome/browser/favicon:favicon",
+    "//ios/chrome/browser/favicon",
+    "//ios/chrome/browser/main",
     "//ios/chrome/browser/sessions",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/sync",
@@ -116,10 +117,14 @@ source_set("unit_tests") {
     "//components/sync_sessions",
     "//components/sync_user_events",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/main:test_support",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/sync",
     "//ios/chrome/browser/sync:test_support",
     "//ios/chrome/browser/ui:feature_flags",
+    "//ios/chrome/browser/ui/commands",
+    "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/browser/web_state_list:test_support",
     "//ios/chrome/test:test_support",
     "//ios/web/public/test",
     "//testing/gtest",

ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator.h

@@ -9,19 +9,18 @@
 
 #import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h"
 
-@protocol ApplicationCommands;
-
 enum class UrlLoadStrategy;
-class WebStateList;
 
 // Coordinator that presents Recent Tabs.
 @interface RecentTabsCoordinator : ChromeCoordinator
-// The dispatcher for this Coordinator.
-@property(nonatomic, weak) id<ApplicationCommands> dispatcher;
 // Opaque instructions on how to open urls.
 @property(nonatomic) UrlLoadStrategy loadStrategy;
-// WebStateList managed by this Coordinator.
-@property(nonatomic, assign) WebStateList* webStateList;
+
+// Use initWithBaseViewController:browser:
+- (instancetype)initWithBaseViewController:(UIViewController*)viewController
+                              browserState:
+                                  (ios::ChromeBrowserState*)browserState
+    NS_UNAVAILABLE;
 
 @end
 

ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator.mm

@@ -7,7 +7,9 @@
 #include "base/ios/block_types.h"
 #include "base/mac/foundation_util.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/main/browser.h"
 #include "ios/chrome/browser/ui/commands/application_commands.h"
+#include "ios/chrome/browser/ui/commands/command_dispatcher.h"
 #import "ios/chrome/browser/ui/recent_tabs/recent_tabs_mediator.h"
 #import "ios/chrome/browser/ui/recent_tabs/recent_tabs_presentation_delegate.h"
 #import "ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.h"
@@ -37,7 +39,6 @@
 
 @implementation RecentTabsCoordinator
 @synthesize completion = _completion;
-@synthesize dispatcher = _dispatcher;
 @synthesize mediator = _mediator;
 @synthesize recentTabsNavigationController = _recentTabsNavigationController;
 @synthesize recentTabsTransitioningDelegate = _recentTabsTransitioningDelegate;
@@ -48,9 +49,12 @@
       [[RecentTabsTableViewController alloc] init];
   recentTabsTableViewController.browserState = self.browserState;
   recentTabsTableViewController.loadStrategy = self.loadStrategy;
-  recentTabsTableViewController.dispatcher = self.dispatcher;
+  CommandDispatcher* dispatcher = self.browser->GetCommandDispatcher();
+  id<ApplicationCommands> handler =
+      HandlerForProtocol(dispatcher, ApplicationCommands);
+  recentTabsTableViewController.handler = handler;
   recentTabsTableViewController.presentationDelegate = self;
-  recentTabsTableViewController.webStateList = self.webStateList;
+  recentTabsTableViewController.webStateList = self.browser->GetWebStateList();
 
   // Adds the "Done" button and hooks it up to |stop|.
   UIBarButtonItem* dismissButton = [[UIBarButtonItem alloc]
@@ -141,9 +145,12 @@
 
 - (void)showHistoryFromRecentTabs {
   // Dismiss recent tabs before presenting history.
+  CommandDispatcher* dispatcher = self.browser->GetCommandDispatcher();
+  id<ApplicationCommands> handler =
+      HandlerForProtocol(dispatcher, ApplicationCommands);
   __weak RecentTabsCoordinator* weakSelf = self;
   self.completion = ^{
-    [weakSelf.dispatcher showHistory];
+    [handler showHistory];
     weakSelf.completion = nil;
   };
   [self stop];

ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm

@@ -18,12 +18,17 @@
 #include "components/sync_sessions/session_sync_service.h"
 #include "components/sync_user_events/global_id_mapper.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#include "ios/chrome/browser/main/test_browser.h"
 #include "ios/chrome/browser/sync/profile_sync_service_factory.h"
 #include "ios/chrome/browser/sync/session_sync_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service.h"
 #include "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service_mock.h"
+#include "ios/chrome/browser/ui/commands/application_commands.h"
+#include "ios/chrome/browser/ui/commands/command_dispatcher.h"
 #import "ios/chrome/browser/ui/recent_tabs/sessions_sync_user_state.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"
 #include "ios/chrome/test/block_cleanup_test.h"
 #include "ios/chrome/test/ios_chrome_scoped_testing_local_state.h"
 #include "ios/web/public/test/web_task_environment.h"
@@ -120,7 +125,8 @@ class RecentTabsTableCoordinatorTest : public BlockCleanupTest {
  public:
   RecentTabsTableCoordinatorTest()
       : no_error_(GoogleServiceAuthError::NONE),
-        fake_controller_delegate_(syncer::SESSIONS) {}
+        fake_controller_delegate_(syncer::SESSIONS),
+        web_state_list_(&web_state_list_delegate_) {}
 
  protected:
   void SetUp() override {
@@ -135,6 +141,9 @@ class RecentTabsTableCoordinatorTest : public BlockCleanupTest {
         base::BindRepeating(
             &BuildMockSessionSyncServiceForRecentTabsTableCoordinator));
     chrome_browser_state_ = test_cbs_builder.Build();
+
+    browser_ = std::make_unique<TestBrowser>(chrome_browser_state_.get(),
+                                             &web_state_list_);
   }
 
   void TearDown() override {
@@ -195,7 +204,24 @@ class RecentTabsTableCoordinatorTest : public BlockCleanupTest {
   void CreateController() {
     coordinator_ = [[RecentTabsCoordinator alloc]
         initWithBaseViewController:nil
-                      browserState:chrome_browser_state_.get()];
+                           browser:browser_.get()];
+    mock_application_commands_handler_ =
+        [OCMockObject mockForProtocol:@protocol(ApplicationCommands)];
+    mock_application_settings_commands_handler_ =
+        [OCMockObject mockForProtocol:@protocol(ApplicationSettingsCommands)];
+    mock_browsing_data_commands_handler_ =
+        [OCMockObject mockForProtocol:@protocol(BrowsingDataCommands)];
+
+    [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_browsing_data_commands_handler_
+                     forProtocol:@protocol(BrowsingDataCommands)];
+
     [coordinator_ start];
   }
 
@@ -206,12 +232,18 @@ class RecentTabsTableCoordinatorTest : public BlockCleanupTest {
   signin::IdentityTestEnvironment identity_test_env_;
 
   syncer::FakeModelTypeControllerDelegate fake_controller_delegate_;
+  FakeWebStateListDelegate web_state_list_delegate_;
+  WebStateList web_state_list_;
   testing::NiceMock<OpenTabsUIDelegateMock> open_tabs_ui_delegate_;
   testing::NiceMock<GlobalIdMapperMock> global_id_mapper_;
   std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
+  std::unique_ptr<Browser> browser_;
 
   // Must be declared *after* |chrome_browser_state_| so it can outlive it.
   RecentTabsCoordinator* coordinator_;
+  id<ApplicationCommands> mock_application_commands_handler_;
+  id<ApplicationSettingsCommands> mock_application_settings_commands_handler_;
+  id<BrowsingDataCommands> mock_browsing_data_commands_handler_;
 };
 
 TEST_F(RecentTabsTableCoordinatorTest, TestConstructorDestructor) {

ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.h

@@ -25,8 +25,8 @@ class WebStateList;
                                  UIAdaptivePresentationControllerDelegate>
 // The coordinator's BrowserState.
 @property(nonatomic, assign) ios::ChromeBrowserState* browserState;
-// The dispatcher used by this ViewController.
-@property(nonatomic, weak) id<ApplicationCommands> dispatcher;
+// The command handler used by this ViewController.
+@property(nonatomic, weak) id<ApplicationCommands> handler;
 // Opaque instructions on how to open urls.
 @property(nonatomic) UrlLoadStrategy loadStrategy;
 // Disposition for tabs restored by this object. Defaults to CURRENT_TAB.

ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm

@@ -1139,27 +1139,26 @@ const int kRecentlyClosedTabsSectionIndex = 0;
 #pragma mark - SyncPresenter
 
 - (void)showReauthenticateSignin {
-  [self.dispatcher
-              showSignin:
-                  [[ShowSigninCommand alloc]
-                      initWithOperation:AUTHENTICATION_OPERATION_REAUTHENTICATE
-                            accessPoint:signin_metrics::AccessPoint::
-                                            ACCESS_POINT_UNKNOWN]
-      baseViewController:self];
+  [self.handler showSignin:[[ShowSigninCommand alloc]
+                               initWithOperation:
+                                   AUTHENTICATION_OPERATION_REAUTHENTICATE
+                                     accessPoint:signin_metrics::AccessPoint::
+                                                     ACCESS_POINT_UNKNOWN]
+        baseViewController:self];
 }
 
 - (void)showSyncPassphraseSettings {
-  [self.dispatcher showSyncPassphraseSettingsFromViewController:self];
+  [self.handler showSyncPassphraseSettingsFromViewController:self];
 }
 
 - (void)showGoogleServicesSettings {
-  [self.dispatcher showGoogleServicesSettingsFromViewController:self];
+  [self.handler showGoogleServicesSettingsFromViewController:self];
 }
 
 #pragma mark - SigninPresenter
 
 - (void)showSignin:(ShowSigninCommand*)command {
-  [self.dispatcher showSignin:command baseViewController:self];
+  [self.handler showSignin:command baseViewController:self];
 }
 
 #pragma mark - UIAdaptivePresentationControllerDelegate

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

@@ -185,8 +185,8 @@
       self.remoteTabsMediator;
   baseViewController.remoteTabsViewController.delegate =
       self.remoteTabsMediator;
-  baseViewController.remoteTabsViewController.dispatcher =
-      static_cast<id<ApplicationCommands>>(self.dispatcher);
+  baseViewController.remoteTabsViewController.handler =
+      HandlerForProtocol(self.dispatcher, ApplicationCommands);
   baseViewController.remoteTabsViewController.loadStrategy =
       UrlLoadStrategy::ALWAYS_NEW_FOREGROUND_TAB;
   baseViewController.remoteTabsViewController.restoredTabDisposition =