Commit 8f976c44 authored by stkhapugin@chromium.org's avatar stkhapugin@chromium.org Committed by Commit Bot

Remove knowledge of location bar view from toolbar coordinator.

The toolbar coordinator shouldn't access implementation details like
the specific view managed by the location bar.
Instead, make it use location bar coordinator, and move the few
protocols that are mostly forwarded by TC to LB straight to LB.

Bug: none
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I244d39a065c7f0a03d9821b0c84fd5bddc421208
Reviewed-on: https://chromium-review.googlesource.com/883462
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532859}
parent 6e8f4754
......@@ -19,24 +19,31 @@ source_set("location_bar") {
deps = [
"//base",
"//components/google/core/browser",
"//components/google/core/browser",
"//components/omnibox/browser",
"//components/search_engines",
"//components/strings",
"//components/toolbar",
"//ios/chrome/app/strings",
"//ios/chrome/app/theme",
"//ios/chrome/browser",
"//ios/chrome/browser/autocomplete",
"//ios/chrome/browser/browser_state:browser_state",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/ui",
"//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/omnibox: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/clean:toolbar_components_ui",
"//ios/chrome/browser/ui/toolbar/keyboard_assist:keyboard_assist",
"//ios/chrome/browser/ui/toolbar/public",
"//ios/chrome/browser/ui/toolbar/public:toolbar_base_feature",
"//ios/chrome/browser/ui/voice",
"//ios/chrome/browser/web_state_list",
"//ios/chrome/common:timing",
"//ios/public/provider/chrome/browser/voice",
"//ios/third_party/material_components_ios",
"//ios/third_party/material_roboto_font_loader_ios",
"//ios/web/public:public",
......
......@@ -8,9 +8,10 @@
#import <UIKit/UIKit.h>
#import "ios/chrome/browser/ui/location_bar/location_bar_url_loader.h"
#include "ios/chrome/browser/ui/location_bar/location_bar_view.h"
#import "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
#include "ios/chrome/browser/ui/qr_scanner/requirements/qr_scanner_result_loading.h"
#import "ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h"
#include "ios/public/provider/chrome/browser/voice/voice_search_controller_delegate.h"
namespace ios {
class ChromeBrowserState;
......@@ -22,11 +23,13 @@ class WebStateList;
@protocol ToolbarCoordinatorDelegate;
@protocol UrlLoader;
@interface LocationBarCoordinator
: NSObject<LocationBarURLLoader, OmniboxFocuser, LocationBarDelegate>
@interface LocationBarCoordinator : NSObject<LocationBarURLLoader,
OmniboxFocuser,
VoiceSearchControllerDelegate,
QRScannerResultLoading>
// LocationBarView containing the omnibox.
@property(nonatomic, strong) LocationBarView* locationBarView;
// View containing the omnibox.
@property(nonatomic, strong, readonly) UIView* view;
// Weak reference to ChromeBrowserState;
@property(nonatomic, assign) ios::ChromeBrowserState* browserState;
// The dispatcher for this view controller.
......@@ -56,6 +59,24 @@ class WebStateList;
// the fakebox on NTP.
- (void)focusOmniboxFromFakebox;
// Indicates when the omnibox is the first responder.
- (BOOL)isOmniboxFirstResponder;
// Perform animations for expanding the omnibox. This animation can be seen on
// an iPhone when the omnibox is focused. It involves sliding the leading button
// out and fading its alpha.
// The trailing button is faded-in in the |completionAnimator| animations.
- (void)addExpandOmniboxAnimations:(UIViewPropertyAnimator*)animator
completionAnimator:(UIViewPropertyAnimator*)completionAnimator;
// Perform animations for expanding the omnibox. This animation can be seen on
// an iPhone when the omnibox is defocused. It involves sliding the leading
// button in and fading its alpha.
- (void)addContractOmniboxAnimations:(UIViewPropertyAnimator*)animator;
// Updates omnibox state, including the displayed text and the cursor position.
- (void)updateOmniboxState;
@end
#endif // IOS_CHROME_BROWSER_UI_LOCATION_BAR_LOCATION_BAR_COORDINATOR_H_
......@@ -11,11 +11,15 @@
#include "base/strings/sys_string_conversions.h"
#include "components/google/core/browser/google_util.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
#include "components/search_engines/util.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/autocomplete/autocomplete_scheme_classifier_impl.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#import "ios/chrome/browser/ui/location_bar/location_bar_consumer.h"
#import "ios/chrome/browser/ui/location_bar/location_bar_mediator.h"
#import "ios/chrome/browser/ui/location_bar/location_bar_url_loader.h"
#include "ios/chrome/browser/ui/location_bar/location_bar_view.h"
#include "ios/chrome/browser/ui/omnibox/location_bar_controller.h"
#include "ios/chrome/browser/ui/omnibox/location_bar_controller_impl.h"
#include "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
......@@ -37,7 +41,7 @@
#error "This file requires ARC support."
#endif
@interface LocationBarCoordinator ()<LocationBarConsumer> {
@interface LocationBarCoordinator ()<LocationBarConsumer, LocationBarDelegate> {
std::unique_ptr<LocationBarControllerImpl> _locationBarController;
}
// Object taking care of adding the accessory views to the keyboard.
......@@ -46,6 +50,9 @@
// Coordinator for the omnibox popup.
@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
@property(nonatomic, strong) LocationBarMediator* mediator;
// Redefined as readwrite and as LocationBarView.
@property(nonatomic, strong, readwrite) LocationBarView* locationBarView;
@end
@implementation LocationBarCoordinator
......@@ -62,6 +69,10 @@
#pragma mark - public
- (UIView*)view {
return self.locationBarView;
}
- (void)start {
BOOL isIncognito = self.browserState->IsOffTheRecord();
......@@ -149,6 +160,46 @@
model->SetCaretVisibility(false);
}
- (BOOL)isOmniboxFirstResponder {
return [self.locationBarView.textField isFirstResponder];
}
- (void)addExpandOmniboxAnimations:(UIViewPropertyAnimator*)animator
completionAnimator:(UIViewPropertyAnimator*)completionAnimator {
[self.locationBarView addExpandOmniboxAnimations:animator
completionAnimator:completionAnimator];
}
- (void)addContractOmniboxAnimations:(UIViewPropertyAnimator*)animator {
[self.locationBarView addContractOmniboxAnimations:animator];
}
#pragma mark - VoiceSearchControllerDelegate
- (void)receiveVoiceSearchResult:(NSString*)result {
DCHECK(result);
[self loadURLForQuery:result];
}
#pragma mark - QRScannerResultLoading
- (void)receiveQRScannerResult:(NSString*)result loadImmediately:(BOOL)load {
DCHECK(result);
if (load) {
[self loadURLForQuery:result];
} else {
[self focusOmnibox];
[self.locationBarView.textField insertTextWhileEditing:result];
// The call to |setText| shouldn't be needed, but without it the "Go" button
// of the keyboard is disabled.
[self.locationBarView.textField setText:result];
// Notify the accessibility system to start reading the new contents of the
// Omnibox.
UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification,
self.locationBarView.textField);
}
}
#pragma mark - LocationBarURLLoader
- (void)loadGURLFromLocationBar:(const GURL&)url
......@@ -217,4 +268,30 @@
return toolbarModelIOS ? toolbarModelIOS->GetToolbarModel() : nullptr;
}
#pragma mark - private
// Navigate to |query| from omnibox.
- (void)loadURLForQuery:(NSString*)query {
GURL searchURL;
metrics::OmniboxInputType type = AutocompleteInput::Parse(
base::SysNSStringToUTF16(query), std::string(),
AutocompleteSchemeClassifierImpl(), nullptr, nullptr, &searchURL);
if (type != metrics::OmniboxInputType::URL || !searchURL.is_valid()) {
searchURL = GetDefaultSearchURLForSearchTerms(
ios::TemplateURLServiceFactory::GetForBrowserState(self.browserState),
base::SysNSStringToUTF16(query));
}
if (searchURL.is_valid()) {
// It is necessary to include PAGE_TRANSITION_FROM_ADDRESS_BAR in the
// transition type is so that query-in-the-omnibox is triggered for the
// URL.
ui::PageTransition transition = ui::PageTransitionFromInt(
ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
[self.URLLoader loadURL:GURL(searchURL)
referrer:web::Referrer()
transition:transition
rendererInitiated:NO];
}
}
@end
......@@ -80,11 +80,11 @@ class LocationBarCoordinatorTest : public PlatformTest {
};
TEST_F(LocationBarCoordinatorTest, Stops) {
EXPECT_TRUE(coordinator_.locationBarView == nil);
EXPECT_TRUE(coordinator_.view == nil);
[coordinator_ start];
EXPECT_TRUE(coordinator_.locationBarView != nil);
EXPECT_TRUE(coordinator_.view != nil);
[coordinator_ stop];
EXPECT_TRUE(coordinator_.locationBarView == nil);
EXPECT_TRUE(coordinator_.view == nil);
}
} // namespace
......@@ -57,9 +57,7 @@
self.viewController.buttonFactory = [self buttonFactoryWithType:PRIMARY];
[self setUpLocationBar];
self.viewController.locationBarView =
self.locationBarCoordinator.locationBarView;
self.viewController.locationBarView = self.locationBarCoordinator.view;
[super start];
_fullscreenObserver =
......@@ -72,13 +70,11 @@
#pragma mark - PrimaryToolbarCoordinator
- (id<VoiceSearchControllerDelegate>)voiceSearchDelegate {
// TODO(crbug.com/799446): This code should be moved to the location bar.
return nil;
return self.locationBarCoordinator;
}
- (id<QRScannerResultLoading>)QRScannerResultLoader {
// TODO(crbug.com/799446): This code should be moved to the location bar.
return nil;
return self.locationBarCoordinator;
}
- (id<TabHistoryUIUpdater>)tabHistoryUIUpdater {
......@@ -98,8 +94,7 @@
}
- (BOOL)isOmniboxFirstResponder {
return
[self.locationBarCoordinator.locationBarView.textField isFirstResponder];
return [self.locationBarCoordinator isOmniboxFirstResponder];
}
- (BOOL)showingOmniboxPopup {
......@@ -156,16 +151,16 @@
// Don't do anything for a live non-ntp tab.
if (webState == self.webStateList->GetActiveWebState() && !isNTP) {
[self.locationBarCoordinator.locationBarView setHidden:NO];
[self.locationBarCoordinator.view setHidden:NO];
} else {
self.viewController.view.hidden = NO;
[self.locationBarCoordinator.locationBarView setHidden:YES];
[self.locationBarCoordinator.view setHidden:YES];
}
}
- (void)resetToolbarAfterSideSwipeSnapshot {
[super resetToolbarAfterSideSwipeSnapshot];
[self.locationBarCoordinator.locationBarView setHidden:NO];
[self.locationBarCoordinator.view setHidden:NO];
}
#pragma mark - Private
......
......@@ -18,14 +18,12 @@ source_set("toolbar") {
":toolbar_ui",
"//base",
"//components/bookmarks/browser",
"//components/google/core/browser",
"//components/search_engines",
"//components/strings",
"//ios/chrome/browser",
"//ios/chrome/browser/autocomplete",
"//ios/chrome/browser/bookmarks",
"//ios/chrome/browser/browser_state",
"//ios/chrome/browser/reading_list",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/ui",
"//ios/chrome/browser/ui/bookmarks",
"//ios/chrome/browser/ui/broadcaster",
......@@ -40,7 +38,6 @@ source_set("toolbar") {
"//ios/chrome/browser/ui/ntp:util",
"//ios/chrome/browser/ui/omnibox",
"//ios/chrome/browser/ui/omnibox:omnibox_internal",
"//ios/chrome/browser/ui/qr_scanner/requirements",
"//ios/chrome/browser/ui/toolbar/keyboard_assist",
"//ios/chrome/browser/ui/toolbar/public",
"//ios/chrome/browser/ui/tools_menu:configuration",
......
......@@ -7,11 +7,9 @@
#import <UIKit/UIKit.h>
#include "ios/chrome/browser/ui/qr_scanner/requirements/qr_scanner_result_loading.h"
#import "ios/chrome/browser/ui/toolbar/public/fakebox_focuser.h"
#import "ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h"
#import "ios/chrome/browser/ui/tools_menu/public/tools_menu_presentation_provider.h"
#include "ios/public/provider/chrome/browser/voice/voice_search_controller_delegate.h"
@protocol ActivityServicePositioner;
@protocol ApplicationCommands;
......@@ -21,6 +19,8 @@
@protocol ToolbarCommands;
@protocol ToolbarCoordinatorDelegate;
@protocol UrlLoader;
@protocol VoiceSearchControllerDelegate;
@protocol QRScannerResultLoading;
class WebStateList;
namespace ios {
class ChromeBrowserState;
......@@ -30,10 +30,8 @@ class WebState;
}
// Coordinator to run a toolbar -- a UI element housing controls.
@interface ToolbarCoordinator : NSObject<FakeboxFocuser,
QRScannerResultLoading,
ToolsMenuPresentationProvider,
VoiceSearchControllerDelegate>
@interface ToolbarCoordinator
: NSObject<FakeboxFocuser, ToolsMenuPresentationProvider>
// Weak reference to ChromeBrowserState;
@property(nonatomic, assign) ios::ChromeBrowserState* browserState;
......@@ -60,6 +58,10 @@ class WebState;
// Returns the OmniboxFocuser for this toolbar.
- (id<OmniboxFocuser>)omniboxFocuser;
// Returns the VoiceSearchControllerDelegate for this toolbar.
- (id<VoiceSearchControllerDelegate>)voiceSearchControllerDelegate;
// Returns the QRScannerResultLoading for this toolbar.
- (id<QRScannerResultLoading>)QRScannerResultLoader;
// Start this coordinator.
- (void)start;
......
......@@ -10,14 +10,10 @@
#include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h"
#include "base/strings/sys_string_conversions.h"
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/search_engines/util.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/autocomplete/autocomplete_scheme_classifier_impl.h"
#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
#include "ios/chrome/browser/search_engines/template_url_service_factory.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_controller.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_controller_factory.h"
#import "ios/chrome/browser/ui/fullscreen/fullscreen_features.h"
......@@ -139,8 +135,7 @@
buttonFactory:factory
buttonUpdater:self.buttonUpdater
omniboxFocuser:self.locationBarCoordinator];
self.toolbarViewController.locationBarView =
self.locationBarCoordinator.locationBarView;
self.toolbarViewController.locationBarView = self.locationBarCoordinator.view;
self.toolbarViewController.dispatcher = self.dispatcher;
if (base::FeatureList::IsEnabled(fullscreen::features::kNewFullscreen)) {
......@@ -196,17 +191,31 @@
return self.locationBarCoordinator;
}
- (id<VoiceSearchControllerDelegate>)voiceSearchControllerDelegate {
return self.locationBarCoordinator;
}
- (id<QRScannerResultLoading>)QRScannerResultLoader {
return self.locationBarCoordinator;
}
- (void)updateToolbarState {
// TODO(crbug.com/803383): This should be done inside the location bar.
// Updates the omnibox.
[self.locationBarCoordinator updateOmniboxState];
}
- (void)updateToolbarForSideSwipeSnapshot:(web::WebState*)webState {
BOOL isNTP = IsVisibleUrlNewTabPage(webState);
// Don't do anything for a live non-ntp tab.
if (webState == self.webStateList->GetActiveWebState() && !isNTP) {
[self.locationBarCoordinator.locationBarView setHidden:NO];
[self.locationBarCoordinator.view setHidden:NO];
return;
}
self.viewController.view.hidden = NO;
[self.locationBarCoordinator.locationBarView setHidden:YES];
[self.locationBarCoordinator.view setHidden:YES];
[self.mediator updateConsumerForWebState:webState];
[self.toolbarViewController updateForSideSwipeSnapshotOnNTP:isNTP];
}
......@@ -214,7 +223,7 @@
- (void)resetToolbarAfterSideSwipeSnapshot {
[self.mediator
updateConsumerForWebState:self.webStateList->GetActiveWebState()];
[self.locationBarCoordinator.locationBarView setHidden:NO];
[self.locationBarCoordinator.view setHidden:NO];
[self.toolbarViewController resetAfterSideSwipeSnapshot];
}
......@@ -235,8 +244,7 @@
}
- (BOOL)isOmniboxFirstResponder {
return
[self.locationBarCoordinator.locationBarView.textField isFirstResponder];
return [self.locationBarCoordinator isOmniboxFirstResponder];
}
- (BOOL)showingOmniboxPopup {
......@@ -318,34 +326,6 @@
self.viewController.view.hidden = NO;
}
#pragma mark - VoiceSearchControllerDelegate
- (void)receiveVoiceSearchResult:(NSString*)result {
DCHECK(result);
[self loadURLForQuery:result];
}
#pragma mark - QRScannerResultLoading
- (void)receiveQRScannerResult:(NSString*)result loadImmediately:(BOOL)load {
DCHECK(result);
if (load) {
[self loadURLForQuery:result];
} else {
[self.locationBarCoordinator focusOmnibox];
[self.locationBarCoordinator.locationBarView.textField
insertTextWhileEditing:result];
// The call to |setText| shouldn't be needed, but without it the "Go" button
// of the keyboard is disabled.
[self.locationBarCoordinator.locationBarView.textField setText:result];
// Notify the accessibility system to start reading the new contents of the
// Omnibox.
UIAccessibilityPostNotification(
UIAccessibilityScreenChangedNotification,
self.locationBarCoordinator.locationBarView.textField);
}
}
#pragma mark - ToolsMenuPresentationProvider
- (UIButton*)presentingButtonForToolsMenuCoordinator:
......@@ -418,30 +398,6 @@
#pragma mark - Private
// Navigate to |query| from omnibox.
- (void)loadURLForQuery:(NSString*)query {
GURL searchURL;
metrics::OmniboxInputType type = AutocompleteInput::Parse(
base::SysNSStringToUTF16(query), std::string(),
AutocompleteSchemeClassifierImpl(), nullptr, nullptr, &searchURL);
if (type != metrics::OmniboxInputType::URL || !searchURL.is_valid()) {
searchURL = GetDefaultSearchURLForSearchTerms(
ios::TemplateURLServiceFactory::GetForBrowserState(self.browserState),
base::SysNSStringToUTF16(query));
}
if (searchURL.is_valid()) {
// It is necessary to include PAGE_TRANSITION_FROM_ADDRESS_BAR in the
// transition type is so that query-in-the-omnibox is triggered for the
// URL.
ui::PageTransition transition = ui::PageTransitionFromInt(
ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_FROM_ADDRESS_BAR);
[self.URLLoader loadURL:GURL(searchURL)
referrer:web::Referrer()
transition:transition
rendererInitiated:NO];
}
}
// Animates |_toolbar| and |_locationBarView| for omnibox expansion. If
// |animated| is NO the animation will happen instantly.
- (void)expandOmniboxAnimated:(BOOL)animated {
......@@ -460,9 +416,8 @@
[completionAnimator startAnimationAfterDelay:ios::material::kDuration4];
}];
[self.locationBarCoordinator.locationBarView
addExpandOmniboxAnimations:animator
completionAnimator:completionAnimator];
[self.locationBarCoordinator addExpandOmniboxAnimations:animator
completionAnimator:completionAnimator];
[self.toolbarViewController addToolbarExpansionAnimations:animator
completionAnimator:completionAnimator];
[animator startAnimation];
......@@ -483,8 +438,7 @@
curve:UIViewAnimationCurveEaseInOut
animations:^{
}];
[self.locationBarCoordinator.locationBarView
addContractOmniboxAnimations:animator];
[self.locationBarCoordinator addContractOmniboxAnimations:animator];
[self.toolbarViewController addToolbarContractionAnimations:animator];
[animator startAnimation];
}
......
......@@ -166,7 +166,8 @@ initWithDispatcher:
#pragma mark - VoiceSearchControllerDelegate
- (void)receiveVoiceSearchResult:(NSString*)voiceResult {
[self.toolbarCoordinator receiveVoiceSearchResult:voiceResult];
[self.toolbarCoordinator.voiceSearchControllerDelegate
receiveVoiceSearchResult:voiceResult];
}
#pragma mark - ActivityServicePositioner
......@@ -179,8 +180,9 @@ initWithDispatcher:
- (void)receiveQRScannerResult:(NSString*)qrScannerResult
loadImmediately:(BOOL)load {
[self.toolbarCoordinator receiveQRScannerResult:qrScannerResult
loadImmediately:load];
[self.toolbarCoordinator.QRScannerResultLoader
receiveQRScannerResult:qrScannerResult
loadImmediately:load];
}
#pragma mark - BubbleViewAnchorPointProvider
......
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