Move LocationBarControllerImpl and PopupCoordinator to LocationBar

Moves the _locationBar ivar (it becomes _locationBarController) to
LocationBarCoordinator. Moves popup coordinator, too.

Bug: 785253, 806028
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4bea5a069fc2caeb76f27d204eeddd8d5b281430
Reviewed-on: https://chromium-review.googlesource.com/883504
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532489}

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

@@ -30,6 +30,7 @@ source_set("location_bar") {
     "//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/toolbar/clean:toolbar_components_ui",
     "//ios/chrome/browser/ui/toolbar/keyboard_assist:keyboard_assist",
     "//ios/chrome/browser/ui/toolbar/public",
@@ -71,6 +72,8 @@ source_set("unit_tests") {
     "//ios/chrome/app/strings",
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/ui/toolbar:test_support",
+    "//ios/chrome/browser/ui/toolbar/clean:toolbar",
     "//ios/chrome/browser/ui/toolbar/test",
     "//ios/chrome/browser/web_state_list",
     "//ios/chrome/browser/web_state_list:test_support",

ios/chrome/browser/ui/location_bar/location_bar_coordinator.h

@@ -15,12 +15,12 @@
 namespace ios {
 class ChromeBrowserState;
 }
+class WebStateList;
 @protocol ApplicationCommands;
 @protocol BrowserCommands;
-@protocol UrlLoader;
+@protocol OmniboxPopupPositioner;
 @protocol ToolbarCoordinatorDelegate;
-class LocationBarControllerImpl;
-class WebStateList;
+@protocol UrlLoader;
 
 @interface LocationBarCoordinator
     : NSObject<LocationBarURLLoader, OmniboxFocuser, LocationBarDelegate>
@@ -33,20 +33,28 @@ class WebStateList;
 @property(nonatomic, weak) id<ApplicationCommands, BrowserCommands> dispatcher;
 // URL loader for the location bar.
 @property(nonatomic, weak) id<UrlLoader> URLLoader;
-// The location bar controller.
-// TODO: this class needs to own this instance instead of ToolbarCoordinator.
-@property(nonatomic, assign) LocationBarControllerImpl* locationBarController;
 // Delegate for this coordinator.
 // TODO(crbug.com/799446): Change this.
 @property(nonatomic, weak) id<ToolbarCoordinatorDelegate> delegate;
 // The web state list this ToolbarCoordinator is handling.
 @property(nonatomic, assign) WebStateList* webStateList;
 
+@property(nonatomic, weak) id<OmniboxPopupPositioner> popupPositioner;
+
 // Start this coordinator.
 - (void)start;
 // Stop this coordinator.
 - (void)stop;
 
+// Indicates whether the popup has results to show or not.
+- (BOOL)omniboxPopupHasAutocompleteResults;
+
+// Indicates if the omnibox currently displays a popup with suggestions.
+- (BOOL)showingOmniboxPopup;
+
+// Focuses the omnibox and sets the caret visibility as if it was called from
+// the fakebox on NTP.
+- (void)focusOmniboxFromFakebox;
 
 @end
 

ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm

@@ -10,13 +10,17 @@
 #include "base/metrics/histogram_macros.h"
 #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/strings/grit/components_strings.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.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/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"
 #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_coordinator_delegate.h"
 #import "ios/chrome/browser/ui/toolbar/keyboard_assist/toolbar_assistive_keyboard_delegate.h"
 #import "ios/chrome/browser/ui/toolbar/keyboard_assist/toolbar_assistive_keyboard_views.h"
@@ -33,10 +37,14 @@
 #error "This file requires ARC support."
 #endif
 
-@interface LocationBarCoordinator ()<LocationBarConsumer>
+@interface LocationBarCoordinator ()<LocationBarConsumer> {
+  std::unique_ptr<LocationBarControllerImpl> _locationBarController;
+}
 // Object taking care of adding the accessory views to the keyboard.
 @property(nonatomic, strong)
     ToolbarAssistiveKeyboardDelegateImpl* keyboardDelegate;
+// Coordinator for the omnibox popup.
+@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
 @property(nonatomic, strong) LocationBarMediator* mediator;
 @end
 
@@ -47,9 +55,10 @@
 @synthesize browserState = _browserState;
 @synthesize dispatcher = dispatcher;
 @synthesize URLLoader = _URLLoader;
-@synthesize locationBarController = _locationBarController;
 @synthesize delegate = _delegate;
 @synthesize webStateList = _webStateList;
+@synthesize omniboxPopupCoordinator = _omniboxPopupCoordinator;
+@synthesize popupPositioner = _popupPositioner;
 
 #pragma mark - public
 
@@ -90,17 +99,31 @@
   ConfigureAssistiveKeyboardViews(self.locationBarView.textField, kDotComTLD,
                                   self.keyboardDelegate);
 
+  _locationBarController = std::make_unique<LocationBarControllerImpl>(
+      self.locationBarView, self.browserState, self, self.dispatcher);
+  _locationBarController->SetURLLoader(self);
+  self.omniboxPopupCoordinator =
+      _locationBarController->CreatePopupCoordinator(self.popupPositioner);
+  [self.omniboxPopupCoordinator start];
   self.mediator = [[LocationBarMediator alloc] init];
   self.mediator.webStateList = self.webStateList;
   self.mediator.consumer = self;
 }
 
 - (void)stop {
+  // The popup has to be destroyed before the location bar.
+  [self.omniboxPopupCoordinator stop];
+  _locationBarController.reset();
+
   self.locationBarView = nil;
   [self.mediator disconnect];
   self.mediator = nil;
 }
 
+- (BOOL)omniboxPopupHasAutocompleteResults {
+  return self.omniboxPopupCoordinator.hasResults;
+}
+
 #pragma mark - LocationBarConsumer
 
 - (void)updateOmniboxState {
@@ -109,6 +132,23 @@
   _locationBarController->OnToolbarUpdated();
 }
 
+- (BOOL)showingOmniboxPopup {
+  OmniboxViewIOS* omniboxViewIOS = static_cast<OmniboxViewIOS*>(
+      _locationBarController.get()->GetLocationEntry());
+  return omniboxViewIOS->IsPopupOpen();
+}
+
+- (void)focusOmniboxFromFakebox {
+  OmniboxEditModel* model = _locationBarController->GetLocationEntry()->model();
+  // Setting the caret visibility to false causes OmniboxEditModel to indicate
+  // that omnibox interaction was initiated from the fakebox. Note that
+  // SetCaretVisibility is a no-op unless OnSetFocus is called first.  Only
+  // set fakebox on iPad, where there is a distinction between the omnibox
+  // and the fakebox on the NTP.
+  model->OnSetFocus(false);
+  model->SetCaretVisibility(false);
+}
+
 #pragma mark - LocationBarURLLoader
 
 - (void)loadGURLFromLocationBar:(const GURL&)url

ios/chrome/browser/ui/location_bar/location_bar_coordinator_unittest.mm

@@ -6,18 +6,45 @@
 
 #include <memory>
 
-#include "base/test/scoped_task_environment.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#import "ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator_delegate.h"
+#include "ios/chrome/browser/ui/toolbar/test_toolbar_model_ios.h"
 #include "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
 #include "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/chrome/browser/web_state_list/web_state_opener.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
+#import "ios/web/public/test/test_web_thread_bundle.h"
 #include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
+@interface TestToolbarCoordinatorDelegate : NSObject<ToolbarCoordinatorDelegate>
+
+@end
+
+@implementation TestToolbarCoordinatorDelegate {
+  std::unique_ptr<ToolbarModelIOS> _model;
+}
+
+- (void)locationBarDidBecomeFirstResponder {
+}
+- (void)locationBarDidResignFirstResponder {
+}
+- (void)locationBarBeganEdit {
+}
+
+- (ToolbarModelIOS*)toolbarModelIOS {
+  if (!_model) {
+    _model = std::make_unique<TestToolbarModelIOS>();
+  }
+
+  return _model.get();
+}
+
+@end
+
 namespace {
 
 class LocationBarCoordinatorTest : public PlatformTest {
@@ -36,16 +63,20 @@ class LocationBarCoordinatorTest : public PlatformTest {
                                    WebStateList::INSERT_FORCE_INDEX,
                                    WebStateOpener());
 
+    delegate_ = [[TestToolbarCoordinatorDelegate alloc] init];
+
     coordinator_ = [[LocationBarCoordinator alloc] init];
     coordinator_.browserState = browser_state_.get();
     coordinator_.webStateList = &web_state_list_;
+    coordinator_.delegate = delegate_;
   }
 
-  base::test::ScopedTaskEnvironment task_environment_;
+  web::TestWebThreadBundle web_thread_bundle_;
   LocationBarCoordinator* coordinator_;
   std::unique_ptr<TestChromeBrowserState> browser_state_;
   FakeWebStateListDelegate web_state_list_delegate_;
   WebStateList web_state_list_;
+  TestToolbarCoordinatorDelegate* delegate_;
 };
 
 TEST_F(LocationBarCoordinatorTest, Stops) {

ios/chrome/browser/ui/toolbar/adaptive/BUILD.gn

@@ -30,7 +30,6 @@ source_set("adaptive") {
     "//ios/chrome/browser/ui/location_bar",
     "//ios/chrome/browser/ui/ntp:util",
     "//ios/chrome/browser/ui/omnibox",
-    "//ios/chrome/browser/ui/omnibox:omnibox_internal",
     "//ios/chrome/browser/ui/omnibox/popup",
     "//ios/chrome/browser/ui/toolbar",
     "//ios/chrome/browser/ui/toolbar/clean:toolbar",

ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_coordinator.mm

@@ -10,17 +10,14 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/sys_string_conversions.h"
-#include "components/google/core/browser/google_util.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"
 #import "ios/chrome/browser/ui/fullscreen/fullscreen_ui_updater.h"
 #import "ios/chrome/browser/ui/location_bar/location_bar_coordinator.h"
 #import "ios/chrome/browser/ui/ntp/ntp_util.h"
-#include "ios/chrome/browser/ui/omnibox/location_bar_controller_impl.h"
-#include "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_positioner.h"
-#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h"
+#import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
 #import "ios/chrome/browser/ui/toolbar/adaptive/adaptive_toolbar_coordinator+subclassing.h"
 #import "ios/chrome/browser/ui/toolbar/adaptive/primary_toolbar_view_controller.h"
 #import "ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator_delegate.h"
@@ -35,7 +32,6 @@
 #endif
 
 @interface PrimaryToolbarCoordinator ()<OmniboxPopupPositioner> {
-  std::unique_ptr<LocationBarControllerImpl> _locationBar;
   // Observer that updates |toolbarViewController| for fullscreen events.
   std::unique_ptr<FullscreenControllerObserver> _fullscreenObserver;
 }
@@ -44,8 +40,6 @@
 @property(nonatomic, strong) PrimaryToolbarViewController* viewController;
 // The coordinator for the location bar in the toolbar.
 @property(nonatomic, strong) LocationBarCoordinator* locationBarCoordinator;
-// Coordinator for the omnibox popup.
-@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
 
 @end
 
@@ -53,7 +47,6 @@
 
 @dynamic viewController;
 @synthesize locationBarCoordinator = _locationBarCoordinator;
-@synthesize omniboxPopupCoordinator = _omniboxPopupCoordinator;
 @synthesize delegate = _delegate;
 @synthesize URLLoader = _URLLoader;
 
@@ -110,9 +103,7 @@
 }
 
 - (BOOL)showingOmniboxPopup {
-  OmniboxViewIOS* omniboxViewIOS =
-      static_cast<OmniboxViewIOS*>(_locationBar.get()->GetLocationEntry());
-  return omniboxViewIOS->IsPopupOpen();
+  return [self.locationBarCoordinator showingOmniboxPopup];
 }
 
 - (void)transitionToLocationBarFocusedState:(BOOL)focused {
@@ -187,18 +178,8 @@
   self.locationBarCoordinator.URLLoader = self.URLLoader;
   self.locationBarCoordinator.delegate = self.delegate;
   self.locationBarCoordinator.webStateList = self.webStateList;
+  self.locationBarCoordinator.popupPositioner = self;
   [self.locationBarCoordinator start];
-
-  // TODO(crbug.com/785253): Move this to the LocationBarCoordinator once it is
-  // created.
-  _locationBar = std::make_unique<LocationBarControllerImpl>(
-      self.locationBarCoordinator.locationBarView, self.browserState,
-      self.locationBarCoordinator, self.dispatcher);
-  self.locationBarCoordinator.locationBarController = _locationBar.get();
-  _locationBar->SetURLLoader(self.locationBarCoordinator);
-  self.omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);
-  [self.omniboxPopupCoordinator start];
-  // End of TODO(crbug.com/785253):.
 }
 
 @end

ios/chrome/browser/ui/toolbar/clean/BUILD.gn

@@ -40,7 +40,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/omnibox/popup",
     "//ios/chrome/browser/ui/qr_scanner/requirements",
     "//ios/chrome/browser/ui/toolbar/keyboard_assist",
     "//ios/chrome/browser/ui/toolbar/public",

ios/chrome/browser/ui/toolbar/clean/toolbar_coordinator.mm

@@ -10,7 +10,7 @@
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/strings/sys_string_conversions.h"
-#include "components/omnibox/browser/omnibox_edit_model.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"
@@ -24,12 +24,8 @@
 #import "ios/chrome/browser/ui/fullscreen/fullscreen_ui_updater.h"
 #import "ios/chrome/browser/ui/location_bar/location_bar_coordinator.h"
 #import "ios/chrome/browser/ui/ntp/ntp_util.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"
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_positioner.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_button_visibility_configuration.h"
@@ -55,7 +51,6 @@
 #endif
 
 @interface ToolbarCoordinator ()<OmniboxPopupPositioner> {
-  std::unique_ptr<LocationBarControllerImpl> _locationBar;
   // Observer that updates |toolbarViewController| for fullscreen events.
   std::unique_ptr<FullscreenControllerObserver> _fullscreenObserver;
 }
@@ -69,8 +64,6 @@
 // Button observer for the ToolsMenu button.
 @property(nonatomic, strong)
     ToolsMenuButtonObserverBridge* toolsMenuButtonObserverBridge;
-// Coordinator for the omnibox popup.
-@property(nonatomic, strong) OmniboxPopupCoordinator* omniboxPopupCoordinator;
 // The coordinator for the location bar in the toolbar.
 @property(nonatomic, strong) LocationBarCoordinator* locationBarCoordinator;
 
@@ -82,7 +75,6 @@
 @synthesize buttonUpdater = _buttonUpdater;
 @synthesize dispatcher = _dispatcher;
 @synthesize mediator = _mediator;
-@synthesize omniboxPopupCoordinator = _omniboxPopupCoordinator;
 @synthesize started = _started;
 @synthesize toolbarViewController = _toolbarViewController;
 @synthesize toolsMenuButtonObserverBridge = _toolsMenuButtonObserverBridge;
@@ -130,19 +122,9 @@
   self.locationBarCoordinator.URLLoader = self.URLLoader;
   self.locationBarCoordinator.delegate = self.delegate;
   self.locationBarCoordinator.webStateList = self.webStateList;
+  self.locationBarCoordinator.popupPositioner = self;
   [self.locationBarCoordinator start];
 
-  // TODO(crbug.com/785253): Move this to the LocationBarCoordinator once it is
-  // created.
-  _locationBar = std::make_unique<LocationBarControllerImpl>(
-      self.locationBarCoordinator.locationBarView, self.browserState,
-      self.locationBarCoordinator, self.dispatcher);
-  self.locationBarCoordinator.locationBarController = _locationBar.get();
-  _locationBar->SetURLLoader(self.locationBarCoordinator);
-  self.omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);
-  [self.omniboxPopupCoordinator start];
-  // End of TODO(crbug.com/785253):.
-
   ToolbarStyle style = isIncognito ? INCOGNITO : NORMAL;
   ToolbarButtonFactory* factory =
       [[ToolbarButtonFactory alloc] initWithStyle:style];
@@ -193,9 +175,6 @@
   self.started = NO;
   self.delegate = nil;
   [self.mediator disconnect];
-  // The popup has to be destroyed before the location bar.
-  [self.omniboxPopupCoordinator stop];
-  _locationBar.reset();
   [self.locationBarCoordinator stop];
   [self stopObservingTTSNotifications];
 
@@ -261,9 +240,7 @@
 }
 
 - (BOOL)showingOmniboxPopup {
-  OmniboxViewIOS* omniboxViewIOS =
-      static_cast<OmniboxViewIOS*>(_locationBar.get()->GetLocationEntry());
-  return omniboxViewIOS->IsPopupOpen();
+  return [self.locationBarCoordinator showingOmniboxPopup];
 }
 
 - (void)activateFakeSafeAreaInsets:(UIEdgeInsets)fakeSafeAreaInsets {
@@ -313,21 +290,16 @@
 
 - (void)focusFakebox {
   if (IsIPadIdiom()) {
-    OmniboxEditModel* model = _locationBar->GetLocationEntry()->model();
-    // Setting the caret visibility to false causes OmniboxEditModel to indicate
-    // that omnibox interaction was initiated from the fakebox. Note that
-    // SetCaretVisibility is a no-op unless OnSetFocus is called first.  Only
-    // set fakebox on iPad, where there is a distinction between the omnibox
-    // and the fakebox on the NTP.  On iPhone there is no visible omnibox, so
-    // there's no need to indicate interaction was initiated from the fakebox.
-    model->OnSetFocus(false);
-    model->SetCaretVisibility(false);
+    // On iPhone there is no visible omnibox, so there's no need to indicate
+    // interaction was initiated from the fakebox.
+    [self.locationBarCoordinator focusOmniboxFromFakebox];
   } else {
     [self expandOmniboxAnimated:NO];
   }
 
   [self.locationBarCoordinator focusOmnibox];
-  if (self.omniboxPopupCoordinator.hasResults) {
+
+  if ([self.locationBarCoordinator omniboxPopupHasAutocompleteResults]) {
     [self onFakeboxAnimationComplete];
   }
 }