Commit 9d022abd authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

Have the PopupViewIOS be owned by the coordinator

This CL moves the ownership of the OmniboxPopupViewIOS to the
OmniboxPopupCoordinator.
The coordinator is now owned by the toolbar, where the popup view was
previous owned.
It also changes the PopupView so it communicates with the mediator and
has now no knowledge of the coordinator.

Bug: 788640
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I530562cc0e52ed8ede575fc689a5326faf978e81
Reviewed-on: https://chromium-review.googlesource.com/833878
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarStepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525338}
parent 2d8a8ffa
......@@ -34,6 +34,8 @@
// CALayer animations to fade the OmniboxPopupRows in.
- (void)updateMatches:(NSArray<id<AutocompleteSuggestion>>*)result
withAnimation:(BOOL)animation;
// Sets the text alignment of the popup content.
- (void)setTextAlignment:(NSTextAlignment)alignment;
@end
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_AUTOCOMPLETE_RESULT_CONSUMER_H_
......@@ -29,11 +29,11 @@ class WebState;
@class PageInfoBridge;
class OmniboxViewIOS;
@class OmniboxClearButtonBridge;
@class OmniboxPopupCoordinator;
@protocol OmniboxPopupPositioner;
@class LocationBarView;
class ScopedFullscreenDisabler;
class ToolbarModel;
class OmniboxPopupViewIOS;
// Concrete implementation of the LocationBarController interface.
class LocationBarControllerImpl : public LocationBarController,
......@@ -45,8 +45,8 @@ class LocationBarControllerImpl : public LocationBarController,
id<BrowserCommands> dispatcher);
~LocationBarControllerImpl() override;
// Creates a popup view and wires it to |edit_view_|.
std::unique_ptr<OmniboxPopupViewIOS> CreatePopupView(
// Creates a popup coordinator and wires it to |edit_view_|.
OmniboxPopupCoordinator* CreatePopupCoordinator(
id<OmniboxPopupPositioner> positioner);
// OmniboxEditController implementation
......
......@@ -27,6 +27,7 @@
#import "ios/chrome/browser/ui/omnibox/location_bar_view.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h"
#include "ios/chrome/browser/ui/ui_util.h"
#import "ios/chrome/browser/ui/uikit_ui_util.h"
#include "ios/chrome/grit/ios_strings.h"
......@@ -166,17 +167,21 @@ LocationBarControllerImpl::LocationBarControllerImpl(
LocationBarControllerImpl::~LocationBarControllerImpl() {}
std::unique_ptr<OmniboxPopupViewIOS> LocationBarControllerImpl::CreatePopupView(
OmniboxPopupCoordinator* LocationBarControllerImpl::CreatePopupCoordinator(
id<OmniboxPopupPositioner> positioner) {
std::unique_ptr<OmniboxPopupViewIOS> popup_view =
base::MakeUnique<OmniboxPopupViewIOS>(edit_view_->browser_state(),
edit_view_->model(),
edit_view_.get(), positioner);
base::MakeUnique<OmniboxPopupViewIOS>(edit_view_->model(),
edit_view_.get());
edit_view_->model()->set_popup_model(popup_view->model());
edit_view_->SetPopupProvider(popup_view.get());
return popup_view;
OmniboxPopupCoordinator* coordinator =
[[OmniboxPopupCoordinator alloc] initWithPopupView:std::move(popup_view)];
coordinator.browserState = browser_state_;
coordinator.positioner = positioner;
return coordinator;
}
void LocationBarControllerImpl::HideKeyboardAndEndEditing() {
......
......@@ -11,6 +11,8 @@
#import "ios/chrome/browser/ui/omnibox/autocomplete_result_consumer.h"
#import "ios/chrome/browser/ui/omnibox/image_retriever.h"
@class OmniboxPopupPresenter;
namespace image_fetcher {
class IOSImageDataFetcherWrapper;
} // namespace
......@@ -36,8 +38,19 @@ class OmniboxPopupMediatorDelegate {
- (void)updateMatches:(const AutocompleteResult&)result
withAnimation:(BOOL)animated;
// Sets the text alignment of the popup content.
- (void)setTextAlignment:(NSTextAlignment)alignment;
// Updates the popup with the |results|.
- (void)updateWithResults:(const AutocompleteResult&)results;
@property(nonatomic, weak) id<AutocompleteResultConsumer> consumer;
@property(nonatomic, assign, getter=isIncognito) BOOL incognito;
// Whether the popup is open.
@property(nonatomic, assign, getter=isOpen) BOOL open;
// Presenter for the popup, handling the positioning and the presentation
// animations.
@property(nonatomic, strong) OmniboxPopupPresenter* presenter;
@end
......
......@@ -11,6 +11,7 @@
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/autocomplete_result.h"
#import "ios/chrome/browser/ui/omnibox/autocomplete_match_formatter.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_presenter.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
......@@ -26,6 +27,8 @@
}
@synthesize consumer = _consumer;
@synthesize incognito = _incognito;
@synthesize open = _open;
@synthesize presenter = _presenter;
- (instancetype)initWithFetcher:
(std::unique_ptr<image_fetcher::IOSImageDataFetcherWrapper>)
......@@ -36,6 +39,7 @@
DCHECK(delegate);
_delegate = delegate;
_imageFetcher = std::move(imageFetcher);
_open = NO;
}
return self;
}
......@@ -66,6 +70,29 @@
return wrappedMatches;
}
- (void)updateWithResults:(const AutocompleteResult&)result {
if (!self.open && !result.empty()) {
// The popup is not currently open and there are results to display. Update
// and animate the cells
[self updateMatches:result withAnimation:YES];
} else {
// The popup is already displayed or there are no results to display. Update
// the cells without animating.
[self updateMatches:result withAnimation:NO];
}
self.open = !result.empty();
if (self.open) {
[self.presenter updateHeightAndAnimateAppearanceIfNecessary];
} else {
[self.presenter animateCollapse];
}
}
- (void)setTextAlignment:(NSTextAlignment)alignment {
[self.consumer setTextAlignment:alignment];
}
#pragma mark - AutocompleteResultConsumerDelegate
- (void)autocompleteResultConsumer:(id<AutocompleteResultConsumer>)sender
......
......@@ -26,9 +26,6 @@
- (instancetype)initWithNibName:(NSString*)nibNameOrNil
bundle:(NSBundle*)nibBundleOrNil NS_UNAVAILABLE;
// Set text alignment for popup cells.
- (void)setTextAlignment:(NSTextAlignment)alignment;
@end
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_VIEW_CONTROLLER_H_
......@@ -17,25 +17,18 @@
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_view_controller.h"
class OmniboxEditModel;
@class OmniboxPopupCoordinator;
@class OmniboxPopupMediator;
class OmniboxPopupModel;
class OmniboxPopupViewSuggestionsDelegate;
@protocol OmniboxPopupPositioner;
struct AutocompleteMatch;
namespace ios {
class ChromeBrowserState;
} // namespace ios
// iOS implementation of AutocompletePopupView.
class OmniboxPopupViewIOS : public OmniboxPopupView,
public OmniboxPopupMediatorDelegate,
public OmniboxPopupProvider {
public:
OmniboxPopupViewIOS(ios::ChromeBrowserState* browser_state,
OmniboxEditModel* edit_model,
OmniboxPopupViewSuggestionsDelegate* delegate,
id<OmniboxPopupPositioner> positioner);
OmniboxPopupViewIOS(OmniboxEditModel* edit_model,
OmniboxPopupViewSuggestionsDelegate* delegate);
~OmniboxPopupViewIOS() override;
// Popup model used for this.
......@@ -64,10 +57,14 @@ class OmniboxPopupViewIOS : public OmniboxPopupView,
void OnMatchSelectedForDeletion(const AutocompleteMatch& match) override;
void OnScroll() override;
void SetMediator(OmniboxPopupMediator* mediator) {
mediator_.reset(mediator);
}
private:
std::unique_ptr<OmniboxPopupModel> model_;
OmniboxPopupViewSuggestionsDelegate* delegate_; // weak
base::scoped_nsobject<OmniboxPopupCoordinator> coordinator_;
base::scoped_nsobject<OmniboxPopupMediator> mediator_;
};
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_VIEW_IOS_H_
......@@ -20,11 +20,8 @@
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/experimental_flags.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_mediator.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_presenter.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_view_controller.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_suggestions_delegate.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_util.h"
#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h"
#include "ios/chrome/browser/ui/ui_util.h"
#import "ios/chrome/browser/ui/uikit_ui_util.h"
#include "ios/chrome/grit/ios_theme_resources.h"
......@@ -39,23 +36,11 @@
using base::UserMetricsAction;
OmniboxPopupViewIOS::OmniboxPopupViewIOS(
ios::ChromeBrowserState* browser_state,
OmniboxEditModel* edit_model,
OmniboxPopupViewSuggestionsDelegate* delegate,
id<OmniboxPopupPositioner> positioner)
OmniboxPopupViewSuggestionsDelegate* delegate)
: model_(new OmniboxPopupModel(this, edit_model)), delegate_(delegate) {
DCHECK(delegate);
DCHECK(browser_state);
DCHECK(edit_model);
// TODO(crbug.com/788640): The coordinator should own the OmniboxPopupViewIOS,
// not the other way around.
coordinator_.reset([[OmniboxPopupCoordinator alloc] init]);
[coordinator_ setBrowserState:browser_state];
[coordinator_ setMediatorDelegate:this];
[coordinator_ setPositioner:positioner];
[coordinator_ start];
}
OmniboxPopupViewIOS::~OmniboxPopupViewIOS() {
......@@ -76,8 +61,8 @@ void OmniboxPopupViewIOS::UpdateEditViewIcon() {
void OmniboxPopupViewIOS::UpdatePopupAppearance() {
const AutocompleteResult& result = model_->result();
[coordinator_ updateWithResults:result];
if ([coordinator_ isOpen]) {
[mediator_ updateWithResults:result];
if ([mediator_ isOpen]) {
UpdateEditViewIcon();
}
......@@ -89,7 +74,7 @@ gfx::Rect OmniboxPopupViewIOS::GetTargetBounds() {
}
bool OmniboxPopupViewIOS::IsOpen() const {
return [coordinator_ isOpen];
return [mediator_ isOpen];
}
OmniboxPopupModel* OmniboxPopupViewIOS::model() const {
......@@ -99,11 +84,11 @@ OmniboxPopupModel* OmniboxPopupViewIOS::model() const {
#pragma mark - OmniboxPopupProvider
bool OmniboxPopupViewIOS::IsPopupOpen() {
return [coordinator_ isOpen];
return [mediator_ isOpen];
}
void OmniboxPopupViewIOS::SetTextAlignment(NSTextAlignment alignment) {
[coordinator_ setTextAlignment:alignment];
[mediator_ setTextAlignment:alignment];
}
#pragma mark - OmniboxPopupViewControllerDelegate
......
......@@ -7,9 +7,10 @@
#import <UIKit/UIKit.h>
class AutocompleteResult;
class OmniboxPopupMediatorDelegate;
#include <memory>
@protocol OmniboxPopupPositioner;
class OmniboxPopupViewIOS;
namespace ios {
class ChromeBrowserState;
......@@ -17,20 +18,18 @@ class ChromeBrowserState;
// Coordinator for the Omnibox Popup.
@interface OmniboxPopupCoordinator : NSObject
- (instancetype)initWithPopupView:
(std::unique_ptr<OmniboxPopupViewIOS>)popupView NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
// BrowserState.
@property(nonatomic, assign) ios::ChromeBrowserState* browserState;
// Positioner for the popup.
@property(nonatomic, weak) id<OmniboxPopupPositioner> positioner;
// Delegate for the popup mediator.
@property(nonatomic, assign) OmniboxPopupMediatorDelegate* mediatorDelegate;
// Whether the popup is open.
@property(nonatomic, assign, getter=isOpen) BOOL open;
- (void)start;
// Updates the popup with the |results|.
- (void)updateWithResults:(const AutocompleteResult&)results;
// Sets the text alignment of the popup content.
- (void)setTextAlignment:(NSTextAlignment)alignment;
- (void)stop;
@end
#endif // IOS_CHROME_BROWSER_UI_OMNIBOX_POPUP_OMNIBOX_POPUP_COORDINATOR_H_
......@@ -10,16 +10,18 @@
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_mediator.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_presenter.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_view_controller.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
@interface OmniboxPopupCoordinator ()
@interface OmniboxPopupCoordinator () {
std::unique_ptr<OmniboxPopupViewIOS> _popupView;
}
@property(nonatomic, strong) OmniboxPopupViewController* popupViewController;
@property(nonatomic, strong) OmniboxPopupMediator* mediator;
@property(nonatomic, strong) OmniboxPopupPresenter* presenter;
@end
......@@ -27,57 +29,45 @@
@synthesize browserState = _browserState;
@synthesize mediator = _mediator;
@synthesize mediatorDelegate = _mediatorDelegate;
@synthesize open = _open;
@synthesize popupViewController = _popupViewController;
@synthesize positioner = _positioner;
@synthesize presenter = _presenter;
#pragma mark - Public
- (instancetype)initWithPopupView:
(std::unique_ptr<OmniboxPopupViewIOS>)popupView {
self = [super init];
if (self) {
_popupView = std::move(popupView);
}
return self;
}
- (void)start {
self.open = NO;
std::unique_ptr<image_fetcher::IOSImageDataFetcherWrapper> imageFetcher =
std::make_unique<image_fetcher::IOSImageDataFetcherWrapper>(
self.browserState->GetRequestContext());
self.mediator =
[[OmniboxPopupMediator alloc] initWithFetcher:std::move(imageFetcher)
delegate:self.mediatorDelegate];
delegate:_popupView.get()];
self.popupViewController = [[OmniboxPopupViewController alloc] init];
self.popupViewController.incognito = self.browserState->IsOffTheRecord();
self.mediator.incognito = self.browserState->IsOffTheRecord();
self.mediator.consumer = self.popupViewController;
self.popupViewController.imageRetriever = self.mediator;
self.popupViewController.delegate = self.mediator;
self.presenter = [[OmniboxPopupPresenter alloc]
self.mediator.presenter = [[OmniboxPopupPresenter alloc]
initWithPopupPositioner:self.positioner
popupViewController:self.popupViewController];
}
- (void)updateWithResults:(const AutocompleteResult&)result {
if (!self.open && !result.empty()) {
// The popup is not currently open and there are results to display. Update
// and animate the cells
[self.mediator updateMatches:result withAnimation:YES];
} else {
// The popup is already displayed or there are no results to display. Update
// the cells without animating.
[self.mediator updateMatches:result withAnimation:NO];
}
self.open = !result.empty();
self.popupViewController.imageRetriever = self.mediator;
self.popupViewController.delegate = self.mediator;
self.popupViewController.incognito = self.browserState->IsOffTheRecord();
if (self.open) {
[self.presenter updateHeightAndAnimateAppearanceIfNecessary];
} else {
[self.presenter animateCollapse];
}
_popupView->SetMediator(self.mediator);
}
- (void)setTextAlignment:(NSTextAlignment)alignment {
[self.popupViewController setTextAlignment:alignment];
- (void)stop {
_popupView.reset();
}
@end
......@@ -79,6 +79,7 @@ source_set("toolbar") {
"//ios/chrome/browser/ui/ntp",
"//ios/chrome/browser/ui/ntp:modal_ntp",
"//ios/chrome/browser/ui/omnibox",
"//ios/chrome/browser/ui/omnibox/popup",
"//ios/chrome/browser/ui/popup_menu",
"//ios/chrome/browser/ui/qr_scanner/requirements",
"//ios/chrome/browser/ui/toolbar/clean:toolbar",
......
......@@ -39,6 +39,7 @@ source_set("toolbar") {
"//ios/chrome/browser/ui/history_popup/requirements",
"//ios/chrome/browser/ui/omnibox",
"//ios/chrome/browser/ui/omnibox:omnibox_internal",
"//ios/chrome/browser/ui/omnibox/popup",
"//ios/chrome/browser/ui/qr_scanner/requirements",
"//ios/chrome/browser/ui/toolbar/keyboard_assist",
"//ios/chrome/browser/ui/toolbar/public",
......
......@@ -31,8 +31,8 @@
#include "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
#include "ios/chrome/browser/ui/omnibox/location_bar_view.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_positioner.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
#import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h"
#import "ios/chrome/browser/ui/toolbar/clean/toolbar_button_factory.h"
#import "ios/chrome/browser/ui/toolbar/clean/toolbar_button_updater.h"
#import "ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator_delegate.h"
......@@ -64,7 +64,6 @@
@interface ToolbarCoordinator ()<LocationBarDelegate, OmniboxPopupPositioner> {
std::unique_ptr<LocationBarControllerImpl> _locationBar;
std::unique_ptr<OmniboxPopupViewIOS> _popupView;
// Observer that updates |toolbarViewController| for fullscreen events.
std::unique_ptr<FullscreenControllerObserver> _fullscreenObserver;
}
......@@ -84,7 +83,8 @@
// Button observer for the ToolsMenu button.
@property(nonatomic, strong)
ToolsMenuButtonObserverBridge* toolsMenuButtonObserverBridge;
// Coordinator for the omnibox popup.
@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
@end
@implementation ToolbarCoordinator
......@@ -95,6 +95,7 @@
@synthesize keyboardDelegate = _keyboardDelegate;
@synthesize locationBarView = _locationBarView;
@synthesize mediator = _mediator;
@synthesize omniboxPopupCoordinator = _omniboxPopupCoordinator;
@synthesize started = _started;
@synthesize toolbarViewController = _toolbarViewController;
@synthesize toolsMenuButtonObserverBridge = _toolsMenuButtonObserverBridge;
......@@ -173,7 +174,8 @@
ConfigureAssistiveKeyboardViews(self.locationBarView.textField, kDotComTLD,
self.keyboardDelegate);
_popupView = _locationBar->CreatePopupView(self);
self.omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);
[self.omniboxPopupCoordinator start];
// End of TODO(crbug.com/785253):.
ToolbarStyle style = isIncognito ? INCOGNITO : NORMAL;
......@@ -219,7 +221,7 @@
self.started = NO;
[self.mediator disconnect];
// The popup has to be destroyed before the location bar.
_popupView.reset();
[self.omniboxPopupCoordinator stop];
_locationBar.reset();
self.locationBarView = nil;
[self stopObservingTTSNotifications];
......
......@@ -53,6 +53,7 @@
#import "ios/chrome/browser/ui/omnibox/omnibox_popup_presenter.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
#include "ios/chrome/browser/ui/omnibox/omnibox_view_ios.h"
#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h"
#import "ios/chrome/browser/ui/popup_menu/popup_menu_view.h"
#import "ios/chrome/browser/ui/reversed_animation.h"
#include "ios/chrome/browser/ui/rtl_geometry.h"
......@@ -127,7 +128,7 @@ using ios::material::TimingFunction;
UIView* _clippingView;
std::unique_ptr<LocationBarControllerImpl> _locationBar;
std::unique_ptr<OmniboxPopupViewIOS> _popupView;
OmniboxPopupCoordinator* _omniboxPopupCoordinator;
BOOL _initialLayoutComplete;
// If |YES|, toolbar is incognito.
BOOL _incognito;
......@@ -495,7 +496,8 @@ using ios::material::TimingFunction;
[_webToolbar setFrame:[self specificControlsArea]];
_locationBar = base::MakeUnique<LocationBarControllerImpl>(
_locationBarView, _browserState, self, self.dispatcher);
_popupView = _locationBar->CreatePopupView(self);
_omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);
[_omniboxPopupCoordinator start];
// Create the determinate progress bar (phone only).
if (idiom == IPHONE_IDIOM) {
......@@ -596,7 +598,7 @@ using ios::material::TimingFunction;
- (void)browserStateDestroyed {
// The location bar has a browser state reference, so must be destroyed at
// this point. The popup has to be destroyed before the location bar.
_popupView.reset();
[_omniboxPopupCoordinator stop];
_locationBar.reset();
_browserState = nullptr;
}
......
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