Change relationship between BVC, Toolbar and Location Bar delegation.

Before this CL, BVC is Toolbar's delegate, which is a
LocationBarControllerImpl's delegate. We would like for the
LocationBarCoordinator to own LBControllerImpl fully. For this, the
LocationBarCoordinator needs to become its delegate. However, the
delegate is mostly forwarding commands to BVC. So after this CL:

1. BVC is the delegate for LBCoordinator.
2. BVC calls Toolbar methods for Toolbar to react to events in LB.
3. LocationBarCoordinator is delegate for LBControllerImpl.
4. Remove old TODO and related ivar in BVC.

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

ios/chrome/browser/ui/browser_view_controller.mm

@@ -499,9 +499,6 @@ NSString* const kBrowserViewControllerSnackbarCategory =
   // YES if Voice Search should be started when the new tab animation is
   // finished.
   BOOL _startVoiceSearchAfterNewTabAnimation;
-
-  // YES if the user interacts with the location bar.
-  BOOL _locationBarHasFocus;
   // YES if a load was cancelled due to typing in the location bar.
   BOOL _locationBarEditCancelledLoad;
   // YES if waiting for a foreground tab due to expectNewForegroundTab.
@@ -4232,9 +4229,6 @@ bubblePresenterForFeature:(const base::Feature&)feature
 #pragma mark - WebToolbarDelegate (Public)
 
 - (void)locationBarDidBecomeFirstResponder {
-  if (_locationBarHasFocus)
-    return;  // TODO(crbug.com/244366): This should not be necessary.
-  _locationBarHasFocus = YES;
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kLocationBarBecomesFirstResponderNotification
                     object:nil];
@@ -4250,12 +4244,11 @@ bubblePresenterForFeature:(const base::Feature&)feature
   }
   [[OmniboxGeolocationController sharedInstance]
       locationBarDidBecomeFirstResponder:_browserState];
+
+  [self.primaryToolbarCoordinator transitionToLocationBarFocusedState:YES];
 }
 
 - (void)locationBarDidResignFirstResponder {
-  if (!_locationBarHasFocus)
-    return;  // TODO(crbug.com/244366): This should not be necessary.
-  _locationBarHasFocus = NO;
   [self.sideSwipeController setEnabled:YES];
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kLocationBarResignsFirstResponderNotification
@@ -4287,6 +4280,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
       webState->GetNavigationManager()->Reload(web::ReloadType::NORMAL,
                                                false /* check_for_repost */);
   }
+  [self.primaryToolbarCoordinator transitionToLocationBarFocusedState:NO];
 }
 
 - (void)locationBarBeganEdit {

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

@@ -33,7 +33,7 @@ source_set("location_bar") {
     "//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/web_state_list:web_state_list",
+    "//ios/chrome/browser/web_state_list",
     "//ios/chrome/common:timing",
     "//ios/third_party/material_components_ios",
     "//ios/third_party/material_roboto_font_loader_ios",

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

@@ -9,6 +9,7 @@
 
 #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"
 #import "ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h"
 
 namespace ios {
@@ -19,9 +20,10 @@ class ChromeBrowserState;
 @protocol UrlLoader;
 @protocol ToolbarCoordinatorDelegate;
 class LocationBarControllerImpl;
+class WebStateList;
 
 @interface LocationBarCoordinator
-    : NSObject<LocationBarURLLoader, OmniboxFocuser>
+    : NSObject<LocationBarURLLoader, OmniboxFocuser, LocationBarDelegate>
 
 // LocationBarView containing the omnibox.
 @property(nonatomic, strong) LocationBarView* locationBarView;
@@ -37,6 +39,8 @@ class LocationBarControllerImpl;
 // 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;
 
 // Start this coordinator.
 - (void)start;

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

@@ -22,6 +22,7 @@
 #include "ios/chrome/browser/ui/toolbar/toolbar_model_ios.h"
 #import "ios/chrome/browser/ui/uikit_ui_util.h"
 #import "ios/chrome/browser/ui/url_loader.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h"
 #import "ios/web/public/referrer.h"
 #include "url/gurl.h"
@@ -44,6 +45,7 @@
 @synthesize URLLoader = _URLLoader;
 @synthesize locationBarController = _locationBarController;
 @synthesize delegate = _delegate;
+@synthesize webStateList = _webStateList;
 
 #pragma mark - public
 
@@ -139,4 +141,28 @@
   _locationBarController->HideKeyboardAndEndEditing();
   [self updateOmniboxState];
 }
+
+#pragma mark - LocationBarDelegate
+
+- (void)locationBarHasBecomeFirstResponder {
+  [self.delegate locationBarDidBecomeFirstResponder];
+}
+
+- (void)locationBarHasResignedFirstResponder {
+  [self.delegate locationBarDidResignFirstResponder];
+}
+
+- (void)locationBarBeganEdit {
+  [self.delegate locationBarBeganEdit];
+}
+
+- (web::WebState*)webState {
+  return self.webStateList->GetActiveWebState();
+}
+
+- (ToolbarModel*)toolbarModel {
+  ToolbarModelIOS* toolbarModelIOS = [self.delegate toolbarModelIOS];
+  return toolbarModelIOS ? toolbarModelIOS->GetToolbarModel() : nullptr;
+}
+
 @end

ios/chrome/browser/ui/omnibox/location_bar_controller_impl.mm

@@ -328,7 +328,7 @@ ToolbarModel* LocationBarControllerImpl::GetToolbarModel() {
 }
 
 web::WebState* LocationBarControllerImpl::GetWebState() {
-  return [delegate_ getWebState];
+  return [delegate_ webState];
 }
 
 void LocationBarControllerImpl::InstallLocationIcon() {

ios/chrome/browser/ui/omnibox/location_bar_delegate.h

@@ -18,7 +18,7 @@ class WebState;
 - (void)locationBarHasBecomeFirstResponder;
 - (void)locationBarHasResignedFirstResponder;
 - (void)locationBarBeganEdit;
-- (web::WebState*)getWebState;
+- (web::WebState*)webState;
 - (ToolbarModel*)toolbarModel;
 @end
 

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

@@ -27,8 +27,7 @@
 #error "This file requires ARC support."
 #endif
 
-@interface PrimaryToolbarCoordinator ()<LocationBarDelegate,
-                                        OmniboxPopupPositioner> {
+@interface PrimaryToolbarCoordinator ()<OmniboxPopupPositioner> {
   std::unique_ptr<LocationBarControllerImpl> _locationBar;
 }
 
@@ -100,6 +99,10 @@
   return omniboxViewIOS->IsPopupOpen();
 }
 
+- (void)transitionToLocationBarFocusedState:(BOOL)focused {
+  // TODO(crbug.com/801082): implement this.
+}
+
 #pragma mark - ToolbarCoordinating
 
 - (void)updateToolbarState {
@@ -122,29 +125,6 @@
   // TODO(crbug.com/803372): Implement that.
 }
 
-#pragma mark - LocationBarDelegate
-
-- (void)locationBarHasBecomeFirstResponder {
-  [self.delegate locationBarDidBecomeFirstResponder];
-}
-
-- (void)locationBarHasResignedFirstResponder {
-  [self.delegate locationBarDidResignFirstResponder];
-}
-
-- (void)locationBarBeganEdit {
-  [self.delegate locationBarBeganEdit];
-}
-
-- (web::WebState*)getWebState {
-  return self.webStateList->GetActiveWebState();
-}
-
-- (ToolbarModel*)toolbarModel {
-  ToolbarModelIOS* toolbarModelIOS = [self.delegate toolbarModelIOS];
-  return toolbarModelIOS ? toolbarModelIOS->GetToolbarModel() : nullptr;
-}
-
 // TODO(crbug.com/786940): This protocol should move to the ViewController
 // owning the Toolbar. This can wait until the omnibox and toolbar refactoring
 // is more advanced.
@@ -182,13 +162,14 @@
   self.locationBarCoordinator.dispatcher = self.dispatcher;
   self.locationBarCoordinator.URLLoader = self.URLLoader;
   self.locationBarCoordinator.delegate = self.delegate;
+  self.locationBarCoordinator.webStateList = self.webStateList;
   [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,
-      self.dispatcher);
+      self.locationBarCoordinator.locationBarView, self.browserState,
+      self.locationBarCoordinator, self.dispatcher);
   self.locationBarCoordinator.locationBarController = _locationBar.get();
   _locationBar->SetURLLoader(self.locationBarCoordinator);
   self.omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);

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

@@ -42,7 +42,8 @@ class WebState;
         dispatcher;
 // The web state list this ToolbarCoordinator is handling.
 @property(nonatomic, assign) WebStateList* webStateList;
-// Delegate for this coordinator.
+// Delegate for this coordinator. Only used for plumbing to Location Bar
+// coordinator.
 // TODO(crbug.com/799446): Change this.
 @property(nonatomic, weak) id<ToolbarCoordinatorDelegate> delegate;
 // URL loader for the toolbar.
@@ -64,6 +65,10 @@ class WebState;
 // Stop this coordinator.
 - (void)stop;
 
+// Coordinates the location bar focusing/defocusing. For example, initiates
+// transition to the expanded location bar state of the view controller.
+- (void)transitionToLocationBarFocusedState:(BOOL)focused;
+
 // TODO(crbug.com/785253): Move this to the LocationBarCoordinator once it is
 // created.
 // Updates the visibility of the Omnibox text and Toolbar buttons.

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

@@ -56,7 +56,7 @@
 #error "This file requires ARC support."
 #endif
 
-@interface ToolbarCoordinator ()<LocationBarDelegate, OmniboxPopupPositioner> {
+@interface ToolbarCoordinator ()<OmniboxPopupPositioner> {
   std::unique_ptr<LocationBarControllerImpl> _locationBar;
   // Observer that updates |toolbarViewController| for fullscreen events.
   std::unique_ptr<FullscreenControllerObserver> _fullscreenObserver;
@@ -131,13 +131,14 @@
   self.locationBarCoordinator.dispatcher = self.dispatcher;
   self.locationBarCoordinator.URLLoader = self.URLLoader;
   self.locationBarCoordinator.delegate = self.delegate;
+  self.locationBarCoordinator.webStateList = self.webStateList;
   [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,
-      self.dispatcher);
+      self.locationBarCoordinator.locationBarView, self.browserState,
+      self.locationBarCoordinator, self.dispatcher);
   self.locationBarCoordinator.locationBarController = _locationBar.get();
   _locationBar->SetURLLoader(self.locationBarCoordinator);
   self.omniboxPopupCoordinator = _locationBar->CreatePopupCoordinator(self);
@@ -231,7 +232,7 @@
   BOOL isNTP = (URL == GURL(kChromeUINewTabURL));
 
   // Don't do anything for a live non-ntp tab.
-  if (webState == [self getWebState] && !isNTP) {
+  if (webState == self.webStateList->GetActiveWebState() && !isNTP) {
     [self.locationBarCoordinator.locationBarView setHidden:NO];
     return;
   }
@@ -243,7 +244,8 @@
 }
 
 - (void)resetToolbarAfterSideSwipeSnapshot {
-  [self.mediator updateConsumerForWebState:[self getWebState]];
+  [self.mediator
+      updateConsumerForWebState:self.webStateList->GetActiveWebState()];
   [self.locationBarCoordinator.locationBarView setHidden:NO];
   [self.toolbarViewController resetAfterSideSwipeSnapshot];
 }
@@ -298,36 +300,25 @@
 
 #pragma mark - LocationBarDelegate
 
-- (void)locationBarHasBecomeFirstResponder {
-  [self.delegate locationBarDidBecomeFirstResponder];
+- (void)transitionToLocationBarFocusedState:(BOOL)focused {
   if (IsIPadIdiom()) {
-    [self.toolbarViewController locationBarIsFirstResonderOnIPad:YES];
-  } else if (!self.toolbarViewController.expanded) {
-    [self expandOmniboxAnimated:YES];
+    [self.toolbarViewController locationBarIsFirstResonderOnIPad:focused];
+    return;
   }
-}
 
-- (void)locationBarHasResignedFirstResponder {
-  [self.delegate locationBarDidResignFirstResponder];
-  if (IsIPadIdiom()) {
-    [self.toolbarViewController locationBarIsFirstResonderOnIPad:NO];
-  } else if (self.toolbarViewController.expanded) {
-    [self contractOmnibox];
+  DCHECK(!IsIPadIdiom());
+  if (focused == self.toolbarViewController.expanded) {
+    // The view controller is already in the correct state.
+    return;
   }
-}
 
-- (void)locationBarBeganEdit {
-  [self.delegate locationBarBeganEdit];
-}
-
-- (web::WebState*)getWebState {
-  return self.webStateList->GetActiveWebState();
+  if (focused) {
+    [self expandOmniboxAnimated:YES];
+  } else {
+    [self contractOmnibox];
+  }
 }
 
-- (ToolbarModel*)toolbarModel {
-  ToolbarModelIOS* toolbarModelIOS = [self.delegate toolbarModelIOS];
-  return toolbarModelIOS ? toolbarModelIOS->GetToolbarModel() : nullptr;
-}
 
 #pragma mark - FakeboxFocuser
 
@@ -355,8 +346,8 @@
 - (void)onFakeboxBlur {
   DCHECK(!IsIPadIdiom());
   // Hide the toolbar if the NTP is currently displayed.
-  web::WebState* webState = [self getWebState];
-  if (webState && (webState->GetVisibleURL() == GURL(kChromeUINewTabURL))) {
+  web::WebState* webState = self.webStateList->GetActiveWebState();
+  if (webState && (webState->GetVisibleURL() == kChromeUINewTabURL)) {
     self.viewController.view.hidden = YES;
   }
 }

ios/chrome/browser/ui/toolbar/legacy_toolbar_coordinator.mm

@@ -143,6 +143,20 @@
   [self.toolbarController setShareButtonEnabled:enabled];
 }
 
+- (void)currentPageLoadStarted {
+  [self.toolbarController currentPageLoadStarted];
+}
+
+- (CGRect)visibleOmniboxFrame {
+  return [self.toolbarController visibleOmniboxFrame];
+}
+
+- (void)triggerToolsMenuButtonAnimation {
+  [self.toolbarController triggerToolsMenuButtonAnimation];
+}
+
+#pragma mark - PrimaryToolbarCoordinator
+
 - (void)showPrerenderingAnimation {
   [self.toolbarController showPrerenderingAnimation];
 }
@@ -155,16 +169,8 @@
   return [self.toolbarController showingOmniboxPopup];
 }
 
-- (void)currentPageLoadStarted {
-  [self.toolbarController currentPageLoadStarted];
-}
-
-- (CGRect)visibleOmniboxFrame {
-  return [self.toolbarController visibleOmniboxFrame];
-}
-
-- (void)triggerToolsMenuButtonAnimation {
-  [self.toolbarController triggerToolsMenuButtonAnimation];
+- (void)transitionToLocationBarFocusedState:(BOOL)focused {
+  [self.toolbarController transitionToLocationBarFocusedState:focused];
 }
 
 #pragma mark - FakeboxFocuser

ios/chrome/browser/ui/toolbar/public/primary_toolbar_coordinator.h

@@ -39,6 +39,10 @@
 // Whether the omnibox popup is currently presented.
 - (BOOL)showingOmniboxPopup;
 
+// Coordinates the location bar focusing/defocusing. For example, initiates
+// transition to the expanded location bar state of the view controller.
+- (void)transitionToLocationBarFocusedState:(BOOL)focused;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_TOOLBAR_PUBLIC_PRIMARY_TOOLBAR_COORDINATOR_H_

ios/chrome/browser/ui/toolbar/public/toolbar.h

@@ -28,6 +28,9 @@
 - (void)setToolsMenuIsVisibleForToolsMenuButton:(BOOL)isVisible;
 - (void)start;
 - (void)stop;
+// Coordinates the location bar focusing/defocusing. For example, initiates
+// transition to the expanded location bar state of the view controller.
+- (void)transitionToLocationBarFocusedState:(BOOL)focused;
 
 @end
 

ios/chrome/browser/ui/toolbar/toolbar_adapter.mm

@@ -134,6 +134,10 @@
   [self.toolbarCoordinator stop];
 }
 
+- (void)transitionToLocationBarFocusedState:(BOOL)focused {
+  [self.toolbarCoordinator transitionToLocationBarFocusedState:focused];
+}
+
 #pragma mark - OmniboxFocuser
 
 - (void)focusOmnibox {

ios/chrome/browser/ui/toolbar/web_toolbar_controller.mm

@@ -962,7 +962,7 @@ initWithDelegate:(id<WebToolbarDelegate>)delegate
   [self.delegate locationBarBeganEdit];
 }
 
-- (web::WebState*)getWebState {
+- (web::WebState*)webState {
   return [self.delegate currentWebState];
 }
 
@@ -1947,6 +1947,12 @@ initWithDelegate:(id<WebToolbarDelegate>)delegate
   }
 }
 
+#pragma mark - Toolbar
+
+- (void)transitionToLocationBarFocusedState:(BOOL)focused {
+  // This is a no-op, since this class implements LocationBarDelegate directly.
+}
+
 @end
 
 #pragma mark - Testing only.