Skip to content

Commit

Permalink
[ios] Move OverscrollActionsControllerDelegate out of BVC
Browse files Browse the repository at this point in the history
This CL moves the implementation of OverscrollActionsControllerDelegate
out of BVC. In particular, the logic has been moved inside
BrowserCoordinator.

Change-Id: I6c98c5c19cc238bb192b27d6696d54bac4414b35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4614431
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Federica Germinario <fedegermi@google.com>
Reviewed-by: Asami Doi <asamidoi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1159481}
  • Loading branch information
Federica Germinario authored and Chromium LUCI CQ committed Jun 19, 2023
1 parent bbad56a commit da261de
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 161 deletions.
192 changes: 131 additions & 61 deletions ios/chrome/browser/ui/browser_view/browser_coordinator.mm

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
// YES from viewDidAppear to viewWillDisappear.
@property(nonatomic, readonly) BOOL viewVisible;

// Height of the header view.
@property(nonatomic, readonly) CGFloat headerHeight;

// Dismisses all presented views, excluding the omnibox if `dismissOmnibox` is
// NO, then calls `completion`.
- (void)clearPresentedStateWithCompletion:(ProceduralBlock)completion
Expand Down
4 changes: 0 additions & 4 deletions ios/chrome/browser/ui/browser_view/browser_view_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
@protocol ApplicationCommands;
@class BookmarksCoordinator;
@class BrowserContainerViewController;
@protocol BrowserCoordinatorCommands;
@class BubblePresenter;
@protocol CRWResponderInputView;
@protocol DefaultPromoNonModalPresentationDelegate;
Expand Down Expand Up @@ -53,7 +52,6 @@ class TabUsageRecorderBrowserAgent;
class UrlLoadingBrowserAgent;
class UrlLoadingNotifierBrowserAgent;
@protocol VoiceSearchController;
class WebNavigationBrowserAgent;
class WebStateUpdateBrowserAgent;

typedef struct {
Expand All @@ -72,7 +70,6 @@ typedef struct {
id<HelpCommands> helpHandler;
id<PopupMenuCommands> popupMenuCommandsHandler;
id<ApplicationCommands> applicationCommandsHandler;
id<BrowserCoordinatorCommands> browserCoordinatorCommandsHandler;
id<FindInPageCommands> findInPageCommandsHandler;
LayoutGuideCenter* layoutGuideCenter;
BOOL isOffTheRecord;
Expand All @@ -81,7 +78,6 @@ typedef struct {
UrlLoadingNotifierBrowserAgent* urlLoadingNotifierBrowserAgent;
id<VoiceSearchController> voiceSearchController;
TabUsageRecorderBrowserAgent* tabUsageRecorderBrowserAgent;
WebNavigationBrowserAgent* webNavigationBrowserAgent;
base::WeakPtr<WebStateList> webStateList;
SafeAreaProvider* safeAreaProvider;
WebStateUpdateBrowserAgent* webStateUpdateBrowserAgent;
Expand Down
83 changes: 0 additions & 83 deletions ios/chrome/browser/ui/browser_view/browser_view_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@
#import "ios/chrome/browser/metrics/tab_usage_recorder_browser_agent.h"
#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
#import "ios/chrome/browser/ntp/new_tab_page_util.h"
#import "ios/chrome/browser/overscroll_actions/overscroll_actions_tab_helper.h"
#import "ios/chrome/browser/reading_list/reading_list_browser_agent.h"
#import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/shared/model/url/chrome_url_constants.h"
#import "ios/chrome/browser/shared/model/web_state_list/web_state_list.h"
#import "ios/chrome/browser/shared/public/commands/application_commands.h"
#import "ios/chrome/browser/shared/public/commands/browser_coordinator_commands.h"
#import "ios/chrome/browser/shared/public/commands/find_in_page_commands.h"
#import "ios/chrome/browser/shared/public/commands/help_commands.h"
#import "ios/chrome/browser/shared/public/commands/popup_menu_commands.h"
Expand Down Expand Up @@ -260,9 +258,6 @@ @interface BrowserViewController () <LensPresentationDelegate,
// Used to report usage of a single Browser's tab.
TabUsageRecorderBrowserAgent* _tabUsageRecorderBrowserAgent;

// Used for common web navigation tasks.
WebNavigationBrowserAgent* _webNavigationBrowserAgent;

// Used for updates in web state.
WebStateUpdateBrowserAgent* _webStateUpdateBrowserAgent;

Expand Down Expand Up @@ -349,10 +344,6 @@ @interface BrowserViewController () <LensPresentationDelegate,
// Command handler for application commands.
@property(nonatomic, weak) id<ApplicationCommands> applicationCommandsHandler;

// Command handler for browser coordinator commands.
@property(nonatomic, weak) id<BrowserCoordinatorCommands>
browserCoordinatorCommandsHandler;

// Command handler for find in page commands.
@property(nonatomic, weak) id<FindInPageCommands> findInPageCommandsHandler;

Expand Down Expand Up @@ -437,15 +428,12 @@ @implementation BrowserViewController
self.helpHandler = dependencies.helpHandler;
self.popupMenuCommandsHandler = dependencies.popupMenuCommandsHandler;
self.applicationCommandsHandler = dependencies.applicationCommandsHandler;
self.browserCoordinatorCommandsHandler =
dependencies.browserCoordinatorCommandsHandler;
self.findInPageCommandsHandler = dependencies.findInPageCommandsHandler;
_isOffTheRecord = dependencies.isOffTheRecord;
_urlLoadingBrowserAgent = dependencies.urlLoadingBrowserAgent;
_urlLoadingNotifierBrowserAgent =
dependencies.urlLoadingNotifierBrowserAgent;
_tabUsageRecorderBrowserAgent = dependencies.tabUsageRecorderBrowserAgent;
_webNavigationBrowserAgent = dependencies.webNavigationBrowserAgent;
_layoutGuideCenter = dependencies.layoutGuideCenter;
_webStateList = dependencies.webStateList;
_voiceSearchController = dependencies.voiceSearchController;
Expand Down Expand Up @@ -2233,77 +2221,6 @@ - (void)popupDidCloseForPresenter:(OmniboxPopupPresenter*)presenter {
self.secondaryToolbarContainerView.accessibilityElementsHidden = NO;
}

#pragma mark - OverscrollActionsControllerDelegate methods.

- (void)overscrollActionNewTab:(OverscrollActionsController*)controller {
[self.applicationCommandsHandler
openURLInNewTab:[OpenNewTabCommand commandWithIncognito:_isOffTheRecord]];
}

- (void)overscrollActionCloseTab:(OverscrollActionsController*)controller {
[self.browserCoordinatorCommandsHandler closeCurrentTab];
}

- (void)overscrollActionRefresh:(OverscrollActionsController*)controller {
// Instruct the SnapshotTabHelper to ignore the next load event.
// Attempting to snapshot while the overscroll "bounce back" animation is
// occurring will cut the animation short.
DCHECK(self.currentWebState);
SnapshotTabHelper::FromWebState(self.currentWebState)->IgnoreNextLoad();
_webNavigationBrowserAgent->Reload();
}

- (BOOL)shouldAllowOverscrollActionsForOverscrollActionsController:
(OverscrollActionsController*)controller {
// When screeen size is not regular, overscroll actions should be enabled.
return !self.toolbarAccessoryPresenter.presenting &&
!IsRegularXRegularSizeClass(self);
}

- (UIView*)headerViewForOverscrollActionsController:
(OverscrollActionsController*)controller {
return self.toolbarCoordinator.primaryToolbarViewController.view;
}

- (UIView*)toolbarSnapshotViewForOverscrollActionsController:
(OverscrollActionsController*)controller {
return [self.toolbarCoordinator.primaryToolbarViewController.view
snapshotViewAfterScreenUpdates:NO];
}

- (CGFloat)headerInsetForOverscrollActionsController:
(OverscrollActionsController*)controller {
// The current WebState can be nil if the Browser's WebStateList is empty
// (e.g. after closing the last tab, etc).
web::WebState* currentWebState = self.currentWebState;
if (!currentWebState)
return 0.0;

OverscrollActionsTabHelper* activeTabHelper =
OverscrollActionsTabHelper::FromWebState(currentWebState);
if (controller == activeTabHelper->GetOverscrollActionsController()) {
return self.headerHeight;
} else
return 0;
}

- (CGFloat)headerHeightForOverscrollActionsController:
(OverscrollActionsController*)controller {
return self.headerHeight;
}

- (CGFloat)initialContentOffsetForOverscrollActionsController:
(OverscrollActionsController*)controller {
return ios::provider::IsFullscreenSmoothScrollingSupported()
? -[self headerInsetForOverscrollActionsController:controller]
: 0;
}

- (FullscreenController*)fullscreenControllerForOverscrollActionsController:
(OverscrollActionsController*)controller {
return self.fullscreenController;
}

#pragma mark - FullscreenUIElement methods

- (void)updateForFullscreenProgress:(CGFloat)progress {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,6 @@ void SetUp() override {

tab_usage_recorder_browser_agent_ =
TabUsageRecorderBrowserAgent::FromBrowser(browser_.get());
web_navigation_browser_agent_ =
WebNavigationBrowserAgent::FromBrowser(browser_.get());
page_placeholder_browser_agent_ =
PagePlaceholderBrowserAgent::FromBrowser(browser_.get());

Expand All @@ -280,15 +278,12 @@ void SetUp() override {
url_loading_notifier_browser_agent_;
dependencies.tabUsageRecorderBrowserAgent =
tab_usage_recorder_browser_agent_;
dependencies.webNavigationBrowserAgent = web_navigation_browser_agent_;
dependencies.layoutGuideCenter =
LayoutGuideCenterForBrowser(browser_.get());
dependencies.webStateList = browser_->GetWebStateList()->AsWeakPtr();
dependencies.safeAreaProvider = safe_area_provider_;
dependencies.pagePlaceholderBrowserAgent = page_placeholder_browser_agent_;
dependencies.applicationCommandsHandler = mockApplicationCommandHandler_;
dependencies.webStateUpdateBrowserAgent =
WebStateUpdateBrowserAgent::FromBrowser(browser_.get());

bvc_ = [[BrowserViewController alloc]
initWithBrowserContainerViewController:container_
Expand Down Expand Up @@ -390,7 +385,6 @@ void ExpectNewTabInsertionAnimation(bool animated, ProceduralBlock block) {
TabEventsMediator* tab_events_mediator_;
UrlLoadingNotifierBrowserAgent* url_loading_notifier_browser_agent_;
TabUsageRecorderBrowserAgent* tab_usage_recorder_browser_agent_;
WebNavigationBrowserAgent* web_navigation_browser_agent_;
SafeAreaProvider* safe_area_provider_;
PagePlaceholderBrowserAgent* page_placeholder_browser_agent_;
id mockApplicationCommandHandler_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,14 @@
#define IOS_CHROME_BROWSER_UI_BROWSER_VIEW_COMMON_TAB_HELPER_DELEGATE_H_

#import "ios/chrome/browser/passwords/password_controller_delegate.h"
#import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h"

// Protocol containing all of the tab helper delegate protocols needed to set
// up webstates after the UI is available.
// This protocol is scaffolding for refactoring these delegate responsibilities
// out of the BVC. The goal is to reduce the number of these delegate protocols
// that the BVC conforms to to zero.
@protocol CommonTabHelperDelegate <OverscrollActionsControllerDelegate,
// TODO(crbug.com/1272487): Factor
// PasswordControllerDelegate out of the BVC.
PasswordControllerDelegate>
// TODO(crbug.com/1272487): Factor PasswordControllerDelegate out of the BVC.
@protocol CommonTabHelperDelegate <PasswordControllerDelegate>
@end

#endif // IOS_CHROME_BROWSER_UI_BROWSER_VIEW_COMMON_TAB_HELPER_DELEGATE_H_
3 changes: 3 additions & 0 deletions ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class TabInsertionBrowserAgent;
class WebStateList;
@protocol NetExportTabHelperDelegate;
@protocol NewTabPageTabHelperDelegate;
@protocol OverscrollActionsControllerDelegate;
@protocol PriceNotificationsIPHPresenter;
@protocol SnapshotGeneratorDelegate;

Expand All @@ -47,6 +48,8 @@ class WebStateList;
@property(nonatomic, weak) id<RepostFormTabHelperDelegate> repostFormDelegate;
@property(nonatomic, weak) id<FollowIPHPresenter> followIPHPresenter;
@property(nonatomic, assign) TabInsertionBrowserAgent* tabInsertionBrowserAgent;
@property(nonatomic, weak) id<OverscrollActionsControllerDelegate>
overscrollActionsDelegate;
@property(nonatomic, weak) id<CommonTabHelperDelegate> delegate;
@property(nonatomic, weak) id<SnapshotGeneratorDelegate>
snapshotGeneratorDelegate;
Expand Down
5 changes: 3 additions & 2 deletions ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ - (void)installDependencyForWebState:(web::WebState*)webState {
bottomSheetTabHelper->SetAutofillBottomSheetHandler(
HandlerForProtocol(_commandDispatcher, AutofillBottomSheetCommands));

DCHECK(_delegate);
OverscrollActionsTabHelper::FromWebState(webState)->SetDelegate(_delegate);
DCHECK(_overscrollActionsDelegate);
OverscrollActionsTabHelper::FromWebState(webState)->SetDelegate(
_overscrollActionsDelegate);

// DownloadManagerTabHelper cannot function without its delegate.
DCHECK(_downloadManagerCoordinator);
Expand Down

0 comments on commit da261de

Please sign in to comment.