Commit f500ddf2 authored by Nazerke's avatar Nazerke Committed by Commit Bot

[iOS][coordinator] Modernize Sad Tab Coordinator.

This CL modernizes the SadTabCoordinator
 - to use |browser| in the initializer
 - to remove the public property for  dispatcher and browserstate.
 - to use self.browser to get browserstate and dispatcher values.

This CL also updates the unit tests.

Bug: 1029346, 1048669
Change-Id: Ia830b1f39448005109eb1193fdb82c11e14dfb4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2093671Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Commit-Queue: Nazerke Kalidolda <nazerke@google.com>
Cr-Commit-Position: refs/heads/master@{#753179}
parent 7c3fa9f3
...@@ -2195,8 +2195,7 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2195,8 +2195,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
_sadTabCoordinator = [[SadTabCoordinator alloc] _sadTabCoordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:self.browserContainerViewController initWithBaseViewController:self.browserContainerViewController
browserState:self.browserState]; browser:self.browser];
_sadTabCoordinator.dispatcher = self.dispatcher;
_sadTabCoordinator.overscrollDelegate = self; _sadTabCoordinator.overscrollDelegate = self;
// If there are any existing SadTabHelpers in // If there are any existing SadTabHelpers in
......
...@@ -41,6 +41,7 @@ source_set("coordinator") { ...@@ -41,6 +41,7 @@ source_set("coordinator") {
":sad_tab", ":sad_tab",
"//components/ui_metrics", "//components/ui_metrics",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/main:public",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/fullscreen:coordinators", "//ios/chrome/browser/ui/fullscreen:coordinators",
...@@ -63,6 +64,7 @@ source_set("unit_tests") { ...@@ -63,6 +64,7 @@ source_set("unit_tests") {
deps = [ deps = [
"//components/strings:components_strings_grit", "//components/strings:components_strings_grit",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/sad_tab", "//ios/chrome/browser/ui/sad_tab",
"//ios/chrome/browser/ui/sad_tab:coordinator", "//ios/chrome/browser/ui/sad_tab:coordinator",
......
...@@ -8,14 +8,18 @@ ...@@ -8,14 +8,18 @@
#import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h" #import "ios/chrome/browser/ui/coordinators/chrome_coordinator.h"
#import "ios/chrome/browser/web/sad_tab_tab_helper_delegate.h" #import "ios/chrome/browser/web/sad_tab_tab_helper_delegate.h"
@protocol ApplicationCommands;
@protocol BrowserCommands;
@protocol OverscrollActionsControllerDelegate; @protocol OverscrollActionsControllerDelegate;
// Coordinator that displays a SadTab view. // Coordinator that displays a SadTab view.
@interface SadTabCoordinator : ChromeCoordinator<SadTabTabHelperDelegate> @interface SadTabCoordinator : ChromeCoordinator<SadTabTabHelperDelegate>
@property(nonatomic, weak) id<ApplicationCommands, BrowserCommands> dispatcher; // Unavailable, use -initWithBaseViewController:browser:.
- (instancetype)initWithBaseViewController:(UIViewController*)viewController
NS_UNAVAILABLE;
// Unavailable, use -initWithBaseViewController:browser:.
- (instancetype)initWithBaseViewController:(UIViewController*)viewController
browserState:(ChromeBrowserState*)browserState
NS_UNAVAILABLE;
// Required to support Overscroll Actions UI, which is displayed when Sad Tab is // Required to support Overscroll Actions UI, which is displayed when Sad Tab is
// pulled down. // pulled down.
......
...@@ -7,8 +7,10 @@ ...@@ -7,8 +7,10 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "components/ui_metrics/sadtab_metrics_types.h" #include "components/ui_metrics/sadtab_metrics_types.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/ui/commands/application_commands.h" #import "ios/chrome/browser/ui/commands/application_commands.h"
#import "ios/chrome/browser/ui/commands/browser_commands.h" #import "ios/chrome/browser/ui/commands/browser_commands.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/commands/open_new_tab_command.h" #import "ios/chrome/browser/ui/commands/open_new_tab_command.h"
#import "ios/chrome/browser/ui/fullscreen/chrome_coordinator+fullscreen_disabling.h" #import "ios/chrome/browser/ui/fullscreen/chrome_coordinator+fullscreen_disabling.h"
#import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h" #import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h"
...@@ -50,7 +52,8 @@ ...@@ -50,7 +52,8 @@
_viewController = [[SadTabViewController alloc] init]; _viewController = [[SadTabViewController alloc] init];
_viewController.delegate = self; _viewController.delegate = self;
_viewController.overscrollDelegate = self.overscrollDelegate; _viewController.overscrollDelegate = self.overscrollDelegate;
_viewController.offTheRecord = self.browserState->IsOffTheRecord(); _viewController.offTheRecord =
self.browser->GetBrowserState()->IsOffTheRecord();
_viewController.repeatedFailure = self.repeatedFailure; _viewController.repeatedFailure = self.repeatedFailure;
[self.baseViewController addChildViewController:_viewController]; [self.baseViewController addChildViewController:_viewController];
...@@ -85,17 +88,26 @@ ...@@ -85,17 +88,26 @@
- (void)sadTabViewControllerShowReportAnIssue: - (void)sadTabViewControllerShowReportAnIssue:
(SadTabViewController*)sadTabViewController { (SadTabViewController*)sadTabViewController {
[self.dispatcher showReportAnIssueFromViewController:self.baseViewController]; // TODO(crbug.com/1045047): Use HandlerForProtocol after commands protocol
// clean up.
[static_cast<id<ApplicationCommands>>(self.browser->GetCommandDispatcher())
showReportAnIssueFromViewController:self.baseViewController];
} }
- (void)sadTabViewController:(SadTabViewController*)sadTabViewController - (void)sadTabViewController:(SadTabViewController*)sadTabViewController
showSuggestionsPageWithURL:(const GURL&)URL { showSuggestionsPageWithURL:(const GURL&)URL {
OpenNewTabCommand* command = [OpenNewTabCommand commandWithURLFromChrome:URL]; OpenNewTabCommand* command = [OpenNewTabCommand commandWithURLFromChrome:URL];
[self.dispatcher openURLInNewTab:command]; // TODO(crbug.com/1045047): Use HandlerForProtocol after commands protocol
// clean up.
[static_cast<id<ApplicationCommands>>(self.browser->GetCommandDispatcher())
openURLInNewTab:command];
} }
- (void)sadTabViewControllerReload:(SadTabViewController*)sadTabViewController { - (void)sadTabViewControllerReload:(SadTabViewController*)sadTabViewController {
[self.dispatcher reload]; // TODO(crbug.com/1045047): Use HandlerForProtocol after commands protocol
// clean up.
[static_cast<id<BrowserCommands>>(self.browser->GetCommandDispatcher())
reload];
} }
#pragma mark - SadTabTabHelperDelegate #pragma mark - SadTabTabHelperDelegate
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#import "ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.h" #import "ios/chrome/browser/ui/sad_tab/sad_tab_coordinator.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/ui/commands/application_commands.h" #import "ios/chrome/browser/ui/commands/application_commands.h"
#import "ios/chrome/browser/ui/commands/browser_commands.h" #import "ios/chrome/browser/ui/commands/browser_commands.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.h" #import "ios/chrome/browser/ui/sad_tab/sad_tab_view_controller.h"
#import "ios/chrome/browser/ui/util/named_guide.h" #import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/chrome/common/ui/util/constraints_ui_util.h" #import "ios/chrome/common/ui/util/constraints_ui_util.h"
...@@ -26,21 +28,21 @@ class SadTabCoordinatorTest : public PlatformTest { ...@@ -26,21 +28,21 @@ class SadTabCoordinatorTest : public PlatformTest {
protected: protected:
SadTabCoordinatorTest() SadTabCoordinatorTest()
: base_view_controller_([[UIViewController alloc] init]), : base_view_controller_([[UIViewController alloc] init]),
browser_state_(TestChromeBrowserState::Builder().Build()) { browser_(std::make_unique<TestBrowser>()) {
UILayoutGuide* guide = [[NamedGuide alloc] initWithName:kContentAreaGuide]; UILayoutGuide* guide = [[NamedGuide alloc] initWithName:kContentAreaGuide];
[base_view_controller_.view addLayoutGuide:guide]; [base_view_controller_.view addLayoutGuide:guide];
AddSameConstraints(guide, base_view_controller_.view); AddSameConstraints(guide, base_view_controller_.view);
} }
web::WebTaskEnvironment task_environment_; web::WebTaskEnvironment task_environment_;
UIViewController* base_view_controller_; UIViewController* base_view_controller_;
std::unique_ptr<TestChromeBrowserState> browser_state_; std::unique_ptr<Browser> browser_;
}; };
// Tests starting coordinator. // Tests starting coordinator.
TEST_F(SadTabCoordinatorTest, Start) { TEST_F(SadTabCoordinatorTest, Start) {
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator start]; [coordinator start];
...@@ -60,7 +62,7 @@ TEST_F(SadTabCoordinatorTest, Start) { ...@@ -60,7 +62,7 @@ TEST_F(SadTabCoordinatorTest, Start) {
TEST_F(SadTabCoordinatorTest, Stop) { TEST_F(SadTabCoordinatorTest, Stop) {
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator start]; [coordinator start];
ASSERT_EQ(1U, base_view_controller_.childViewControllers.count); ASSERT_EQ(1U, base_view_controller_.childViewControllers.count);
...@@ -73,7 +75,7 @@ TEST_F(SadTabCoordinatorTest, Stop) { ...@@ -73,7 +75,7 @@ TEST_F(SadTabCoordinatorTest, Stop) {
TEST_F(SadTabCoordinatorTest, Dismiss) { TEST_F(SadTabCoordinatorTest, Dismiss) {
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator start]; [coordinator start];
ASSERT_EQ(1U, base_view_controller_.childViewControllers.count); ASSERT_EQ(1U, base_view_controller_.childViewControllers.count);
...@@ -87,7 +89,7 @@ TEST_F(SadTabCoordinatorTest, Dismiss) { ...@@ -87,7 +89,7 @@ TEST_F(SadTabCoordinatorTest, Dismiss) {
TEST_F(SadTabCoordinatorTest, Hide) { TEST_F(SadTabCoordinatorTest, Hide) {
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator start]; [coordinator start];
ASSERT_EQ(1U, base_view_controller_.childViewControllers.count); ASSERT_EQ(1U, base_view_controller_.childViewControllers.count);
...@@ -103,7 +105,7 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInNonIncognito) { ...@@ -103,7 +105,7 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInNonIncognito) {
web_state.WasShown(); web_state.WasShown();
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator sadTabTabHelper:nullptr [coordinator sadTabTabHelper:nullptr
presentSadTabForWebState:&web_state presentSadTabForWebState:&web_state
...@@ -125,11 +127,11 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInNonIncognito) { ...@@ -125,11 +127,11 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInNonIncognito) {
TEST_F(SadTabCoordinatorTest, FirstFailureInIncognito) { TEST_F(SadTabCoordinatorTest, FirstFailureInIncognito) {
web::TestWebState web_state; web::TestWebState web_state;
web_state.WasShown(); web_state.WasShown();
ChromeBrowserState* otr_browser_state = std::unique_ptr<Browser> otr_browser = std::make_unique<TestBrowser>(
browser_state_->GetOffTheRecordChromeBrowserState(); browser_->GetBrowserState()->GetOffTheRecordChromeBrowserState());
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:otr_browser_state]; browser:otr_browser.get()];
[coordinator sadTabTabHelper:nullptr [coordinator sadTabTabHelper:nullptr
presentSadTabForWebState:&web_state presentSadTabForWebState:&web_state
...@@ -149,11 +151,11 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInIncognito) { ...@@ -149,11 +151,11 @@ TEST_F(SadTabCoordinatorTest, FirstFailureInIncognito) {
// Tests SadTabViewController state for the repeated failure in incognito mode. // Tests SadTabViewController state for the repeated failure in incognito mode.
TEST_F(SadTabCoordinatorTest, ShowFirstFailureInIncognito) { TEST_F(SadTabCoordinatorTest, ShowFirstFailureInIncognito) {
ChromeBrowserState* otr_browser_state = std::unique_ptr<Browser> otr_browser = std::make_unique<TestBrowser>(
browser_state_->GetOffTheRecordChromeBrowserState(); browser_->GetBrowserState()->GetOffTheRecordChromeBrowserState());
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:otr_browser_state]; browser:otr_browser.get()];
[coordinator sadTabTabHelper:nullptr didShowForRepeatedFailure:YES]; [coordinator sadTabTabHelper:nullptr didShowForRepeatedFailure:YES];
...@@ -175,9 +177,14 @@ TEST_F(SadTabCoordinatorTest, FirstFailureAction) { ...@@ -175,9 +177,14 @@ TEST_F(SadTabCoordinatorTest, FirstFailureAction) {
web_state.WasShown(); web_state.WasShown();
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
coordinator.dispatcher = OCMStrictProtocolMock(@protocol(BrowserCommands));
OCMExpect([coordinator.dispatcher reload]); id mock_browser_commands_handler_ =
OCMStrictProtocolMock(@protocol(BrowserCommands));
[browser_->GetCommandDispatcher()
startDispatchingToTarget:mock_browser_commands_handler_
forProtocol:@protocol(BrowserCommands)];
OCMExpect([mock_browser_commands_handler_ reload]);
[coordinator sadTabTabHelper:nullptr [coordinator sadTabTabHelper:nullptr
presentSadTabForWebState:&web_state presentSadTabForWebState:&web_state
...@@ -192,7 +199,7 @@ TEST_F(SadTabCoordinatorTest, FirstFailureAction) { ...@@ -192,7 +199,7 @@ TEST_F(SadTabCoordinatorTest, FirstFailureAction) {
// Verify dispatcher's message. // Verify dispatcher's message.
[view_controller.actionButton [view_controller.actionButton
sendActionsForControlEvents:UIControlEventTouchUpInside]; sendActionsForControlEvents:UIControlEventTouchUpInside];
EXPECT_OCMOCK_VERIFY(coordinator.dispatcher); EXPECT_OCMOCK_VERIFY(mock_browser_commands_handler_);
[coordinator stop]; [coordinator stop];
} }
...@@ -202,10 +209,14 @@ TEST_F(SadTabCoordinatorTest, RepeatedFailureAction) { ...@@ -202,10 +209,14 @@ TEST_F(SadTabCoordinatorTest, RepeatedFailureAction) {
web_state.WasShown(); web_state.WasShown();
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
coordinator.dispatcher =
id mock_application_commands_handler_ =
OCMStrictProtocolMock(@protocol(ApplicationCommands)); OCMStrictProtocolMock(@protocol(ApplicationCommands));
OCMExpect([coordinator.dispatcher [browser_->GetCommandDispatcher()
startDispatchingToTarget:mock_application_commands_handler_
forProtocol:@protocol(ApplicationCommands)];
OCMExpect([mock_application_commands_handler_
showReportAnIssueFromViewController:base_view_controller_]); showReportAnIssueFromViewController:base_view_controller_]);
[coordinator sadTabTabHelper:nullptr [coordinator sadTabTabHelper:nullptr
...@@ -221,7 +232,7 @@ TEST_F(SadTabCoordinatorTest, RepeatedFailureAction) { ...@@ -221,7 +232,7 @@ TEST_F(SadTabCoordinatorTest, RepeatedFailureAction) {
// Verify dispatcher's message. // Verify dispatcher's message.
[view_controller.actionButton [view_controller.actionButton
sendActionsForControlEvents:UIControlEventTouchUpInside]; sendActionsForControlEvents:UIControlEventTouchUpInside];
EXPECT_OCMOCK_VERIFY(coordinator.dispatcher); EXPECT_OCMOCK_VERIFY(mock_application_commands_handler_);
[coordinator stop]; [coordinator stop];
} }
...@@ -230,7 +241,7 @@ TEST_F(SadTabCoordinatorTest, IgnoreSadTabFromHiddenWebState) { ...@@ -230,7 +241,7 @@ TEST_F(SadTabCoordinatorTest, IgnoreSadTabFromHiddenWebState) {
web::TestWebState web_state; web::TestWebState web_state;
SadTabCoordinator* coordinator = [[SadTabCoordinator alloc] SadTabCoordinator* coordinator = [[SadTabCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:browser_state_.get()]; browser:browser_.get()];
[coordinator sadTabTabHelper:nullptr [coordinator sadTabTabHelper:nullptr
presentSadTabForWebState:&web_state presentSadTabForWebState:&web_state
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment