[iOS] Make OmniboxViewController the delegate of omnibox text field

This was something that we wanted to do after the omnibox UI refresh,
which happened last year. Now, we don't need the
AutocompleteTextFieldDelegate or the NSNotifications in
OmniboxViewController.

Bug: 866446
Change-Id: I674bcdb65baecace0b22f6494545985deb22b47a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866632Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Cr-Commit-Position: refs/heads/master@{#709027}

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

@@ -114,6 +114,7 @@ source_set("omnibox_internal") {
     "omnibox_coordinator.mm",
     "omnibox_mediator.h",
     "omnibox_mediator.mm",
+    "omnibox_text_change_delegate.h",
     "omnibox_text_field_delegate.h",
     "omnibox_text_field_ios.h",
     "omnibox_text_field_ios.mm",

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

@@ -85,6 +85,8 @@
       self.textField, self.editController, self.mediator, self.browserState,
       focuser);
 
+  self.viewController.textChangeDelegate = _editView.get();
+
   // Configure the textfield.
   self.textField.suggestionCommandsEndpoint =
       static_cast<id<OmniboxSuggestionCommands>>(self.dispatcher);

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_TEXT_CHANGE_DELEGATE_H_
+#define IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_TEXT_CHANGE_DELEGATE_H_
+
+#import <UIKit/UIKit.h>
+
+class OmniboxTextChangeDelegate {
+ public:
+  // Called when the Omnibox text field starts editing
+  virtual void OnDidBeginEditing() = 0;
+  // Called before the Omnibox text field changes. |new_text| will replace the
+  // text currently in |range|. This should return true if the text change
+  // should happen and false otherwise.
+  // See -textField:shouldChangeCharactersInRange:replacementString: for more
+  // details.
+  virtual bool OnWillChange(NSRange range, NSString* new_text) = 0;
+  // Called after the Omnibox text field changes. |processing_user_input| holds
+  // whether the change was user-initiated or programmatic.
+  virtual void OnDidChange(bool processing_user_input) = 0;
+  // Called before the Omnibox text field finishes editing.
+  virtual void OnWillEndEditing() = 0;
+  // Called when the Omnibox text field returns. (The "go" button is tapped.)
+  virtual void OnAccept() = 0;
+  // Called when the Omnibox text field should copy.
+  virtual bool OnCopy() = 0;
+  // Clear the Omnibox text.
+  virtual void ClearText() = 0;
+  // Called when the Omnibox text field should paste.
+  virtual void WillPaste() = 0;
+  // Called when the backspace button is pressed in the Omnibox text field.
+  virtual void OnDeleteBackward() = 0;
+};
+
+#endif  // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_TEXT_CHANGE_DELEGATE_H_

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

@@ -16,6 +16,7 @@
 @protocol LoadQueryCommands;
 @protocol OmniboxFocuser;
 @class OmniboxViewController;
+class OmniboxTextChangeDelegate;
 
 @protocol OmniboxViewControllerDelegate
 
@@ -54,6 +55,8 @@
 
 // Designated initializer.
 - (instancetype)initWithIncognito:(BOOL)isIncognito;
+
+- (void)setTextChangeDelegate:(OmniboxTextChangeDelegate*)textChangeDelegate;
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_VIEW_CONTROLLER_H_

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

@@ -12,6 +12,8 @@
 #import "ios/chrome/browser/ui/commands/load_query_commands.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_constants.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_container_view.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_text_change_delegate.h"
+#import "ios/chrome/browser/ui/omnibox/omnibox_text_field_delegate.h"
 #import "ios/chrome/browser/ui/toolbar/public/omnibox_focuser.h"
 #import "ios/chrome/browser/ui/toolbar/public/toolbar_constants.h"
 #include "ios/chrome/browser/ui/ui_feature_flags.h"
@@ -32,7 +34,10 @@ const CGFloat kClearButtonSize = 28.0f;
 
 }  // namespace
 
-@interface OmniboxViewController ()
+@interface OmniboxViewController () <OmniboxTextFieldDelegate> {
+  // Weak, acts as a delegate
+  OmniboxTextChangeDelegate* _textChangeDelegate;
+}
 
 // Override of UIViewController's view with a different type.
 @property(nonatomic, strong) OmniboxContainerView* view;
@@ -43,6 +48,17 @@ const CGFloat kClearButtonSize = 28.0f;
 
 @property(nonatomic, assign) BOOL incognito;
 
+// YES if we are already forwarding an OnDidChange() message to the edit view.
+// Needed to prevent infinite recursion.
+// TODO(crbug.com/1015413): There must be a better way.
+@property(nonatomic, assign) BOOL forwardingOnDidChange;
+
+// YES if this text field is currently processing a user-initiated event,
+// such as typing in the omnibox or pressing the clear button.  Used to
+// distinguish between calls to textDidChange that are triggered by the user
+// typing vs by calls to setText.
+@property(nonatomic, assign) BOOL processingUserEvent;
+
 @end
 
 @implementation OmniboxViewController
@@ -83,6 +99,8 @@ const CGFloat kClearButtonSize = 28.0f;
            iconTint:iconTintColor];
   self.view.incognito = self.incognito;
 
+  self.textField.delegate = self;
+
   SetA11yLabelAndUiAutomationName(self.textField, IDS_ACCNAME_LOCATION,
                                   @"Address");
 }
@@ -113,13 +131,6 @@ const CGFloat kClearButtonSize = 28.0f;
              name:UITextInputCurrentInputModeDidChangeNotification
            object:nil];
 
-  // TODO(crbug.com/866446): Use UITextFieldDelegate instead.
-  [[NSNotificationCenter defaultCenter]
-      addObserver:self
-         selector:@selector(textFieldDidBeginEditing)
-             name:UITextFieldTextDidBeginEditingNotification
-           object:self.textField];
-
   [self updateLeadingImageVisibility];
 }
 
@@ -141,12 +152,99 @@ const CGFloat kClearButtonSize = 28.0f;
   [self updateLeadingImageVisibility];
 }
 
+- (void)setTextChangeDelegate:(OmniboxTextChangeDelegate*)textChangeDelegate {
+  _textChangeDelegate = textChangeDelegate;
+}
+
 #pragma mark - public methods
 
 - (OmniboxTextFieldIOS*)textField {
   return self.view.textField;
 }
 
+#pragma mark - OmniboxTextFieldDelegate
+
+- (BOOL)textField:(UITextField*)textField
+    shouldChangeCharactersInRange:(NSRange)range
+                replacementString:(NSString*)newText {
+  DCHECK(_textChangeDelegate);
+  self.processingUserEvent = _textChangeDelegate->OnWillChange(range, newText);
+  return self.processingUserEvent;
+}
+
+- (void)textFieldDidChange:(id)sender {
+  // If the text is empty, update the leading image.
+  if (self.textField.text.length == 0) {
+    [self.view setLeadingImage:self.emptyTextLeadingImage];
+  }
+
+  [self updateClearButtonVisibility];
+  self.semanticContentAttribute = [self.textField bestSemanticContentAttribute];
+
+  if (self.forwardingOnDidChange)
+    return;
+
+  BOOL savedProcessingUserEvent = self.processingUserEvent;
+  self.processingUserEvent = NO;
+  self.forwardingOnDidChange = YES;
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->OnDidChange(savedProcessingUserEvent);
+  self.forwardingOnDidChange = NO;
+}
+
+// Delegate method for UITextField, called when user presses the "go" button.
+- (BOOL)textFieldShouldReturn:(UITextField*)textField {
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->OnAccept();
+  return NO;
+}
+
+// Always update the text field colors when we start editing.  It's possible
+// for this method to be called when we are already editing (popup focus
+// change).  In this case, OnDidBeginEditing will be called multiple times.
+// If that becomes an issue a boolean should be added to track editing state.
+- (void)textFieldDidBeginEditing:(UITextField*)textField {
+  // Update the clear button state.
+  [self updateClearButtonVisibility];
+  [self.view setLeadingImage:self.textField.text.length
+                                 ? self.defaultLeadingImage
+                                 : self.emptyTextLeadingImage];
+
+  self.semanticContentAttribute = [self.textField bestSemanticContentAttribute];
+
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->OnDidBeginEditing();
+}
+
+- (BOOL)textFieldShouldEndEditing:(UITextField*)textField {
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->OnWillEndEditing();
+  return YES;
+}
+
+// When editing, forward the message on to |_textChangeDelegate|.
+- (BOOL)textFieldShouldClear:(UITextField*)textField {
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->ClearText();
+  self.processingUserEvent = YES;
+  return YES;
+}
+
+- (BOOL)onCopy {
+  DCHECK(_textChangeDelegate);
+  return _textChangeDelegate->OnCopy();
+}
+
+- (void)willPaste {
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->WillPaste();
+}
+
+- (void)onDeleteBackward {
+  DCHECK(_textChangeDelegate);
+  _textChangeDelegate->OnDeleteBackward();
+}
+
 #pragma mark - OmniboxConsumer
 
 - (void)updateAutocompleteIcon:(UIImage*)icon {
@@ -191,17 +289,6 @@ const CGFloat kClearButtonSize = 28.0f;
 
 #pragma mark notification callbacks
 
-// Called on UITextFieldTextDidBeginEditingNotification for self.textField.
-- (void)textFieldDidBeginEditing {
-  // Update the clear button state.
-  [self updateClearButtonVisibility];
-  [self.view setLeadingImage:self.textField.text.length
-                                 ? self.defaultLeadingImage
-                                 : self.emptyTextLeadingImage];
-
-  self.semanticContentAttribute = [self.textField bestSemanticContentAttribute];
-}
-
 // Called on UITextInputCurrentInputModeDidChangeNotification for self.textField
 - (void)textInputModeDidChange {
   // Only respond to language changes when the omnibox is first responder.
@@ -266,17 +353,6 @@ const CGFloat kClearButtonSize = 28.0f;
   }
 }
 
-// Called on textField's UIControlEventEditingChanged.
-- (void)textFieldDidChange:(UITextField*)textField {
-  // If the text is empty, update the leading image.
-  if (self.textField.text.length == 0) {
-    [self.view setLeadingImage:self.emptyTextLeadingImage];
-  }
-
-  [self updateClearButtonVisibility];
-  self.semanticContentAttribute = [self.textField bestSemanticContentAttribute];
-}
-
 // Hides the clear button if the textfield is empty; shows it otherwise.
 - (void)updateClearButtonVisibility {
   BOOL hasText = self.textField.text.length > 0;

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

@@ -11,6 +11,7 @@
 #include "components/omnibox/browser/location_bar_model.h"
 #include "components/omnibox/browser/omnibox_view.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_left_image_consumer.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_text_change_delegate.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
 #include "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_provider.h"
 #import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_view_suggestions_delegate.h"
@@ -19,7 +20,6 @@ class AutocompleteResult;
 class GURL;
 class WebOmniboxEditController;
 struct AutocompleteMatch;
-@class AutocompleteTextFieldDelegate;
 @class OmniboxTextFieldIOS;
 @class OmniboxTextFieldPasteDelegate;
 @protocol OmniboxFocuser;
@@ -31,7 +31,8 @@ class ChromeBrowserState;
 // iOS implementation of OmniBoxView.  Wraps a UITextField and
 // interfaces with the rest of the autocomplete system.
 class OmniboxViewIOS : public OmniboxView,
-                       public OmniboxPopupViewSuggestionsDelegate {
+                       public OmniboxPopupViewSuggestionsDelegate,
+                       public OmniboxTextChangeDelegate {
  public:
   // Retains |field|.
   OmniboxViewIOS(OmniboxTextFieldIOS* field,
@@ -39,7 +40,6 @@ class OmniboxViewIOS : public OmniboxView,
                  id<OmniboxLeftImageConsumer> left_image_consumer,
                  ios::ChromeBrowserState* browser_state,
                  id<OmniboxFocuser> omnibox_focuser);
-  ~OmniboxViewIOS() override;
 
   void SetPopupProvider(OmniboxPopupProvider* provider) {
     popup_provider_ = provider;
@@ -92,16 +92,17 @@ class OmniboxViewIOS : public OmniboxView,
   gfx::NativeView GetNativeView() const override;
   gfx::NativeView GetRelativeWindowForPopup() const override;
 
-  // AutocompleteTextFieldDelegate methods
-  void OnDidBeginEditing();
-  bool OnWillChange(NSRange range, NSString* new_text);
-  void OnDidChange(bool processing_user_input);
-  void OnWillEndEditing();
-  void OnAccept();
-  void OnClear();
-  bool OnCopy();
-  void WillPaste();
-  void OnDeleteBackward();
+  // OmniboxTextChangeDelegate methods
+
+  void OnDidBeginEditing() override;
+  bool OnWillChange(NSRange range, NSString* new_text) override;
+  void OnDidChange(bool processing_user_input) override;
+  void OnWillEndEditing() override;
+  void OnAccept() override;
+  bool OnCopy() override;
+  void ClearText() override;
+  void WillPaste() override;
+  void OnDeleteBackward() override;
 
   // OmniboxPopupViewSuggestionsDelegate methods
 
@@ -126,8 +127,7 @@ class OmniboxViewIOS : public OmniboxView,
   // Updates the appearance of popup to have proper text alignment.
   void UpdatePopupAppearance();
 
-  // Clears the text from the omnibox.
-  void ClearText();
+  void OnClear();
 
   // Hide keyboard and call OnDidEndEditing.  This dismisses the keyboard and
   // also finalizes the editing state of the omnibox.
@@ -199,9 +199,6 @@ class OmniboxViewIOS : public OmniboxView,
   // also applied. See https://crbug.com/699702 for discussion.
   BOOL use_strikethrough_workaround_;
 
-  // Bridges delegate method calls from |field_| to C++ land.
-  AutocompleteTextFieldDelegate* field_delegate_;
-
   // Temporary pointer to the attributed display string, stored as color and
   // other emphasis attributes are applied by the superclass.
   NSMutableAttributedString* attributing_display_string_;

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

@@ -66,95 +66,6 @@ UIColor* IncognitoSecureTextColor() {
 
 }  // namespace
 
-#pragma mark - AutocompleteTextFieldDelegate
-
-// Simple Obj-C object to forward UITextFieldDelegate method calls back to the
-// OmniboxViewIOS.
-@interface AutocompleteTextFieldDelegate : NSObject<OmniboxTextFieldDelegate> {
- @private
-  OmniboxViewIOS* editView_;  // weak, owns us
-
-  // YES if we are already forwarding an OnDidChange() message to the edit view.
-  // Needed to prevent infinite recursion.
-  // TODO(rohitrao): There must be a better way.
-  BOOL forwardingOnDidChange_;
-
-  // YES if this text field is currently processing a user-initiated event,
-  // such as typing in the omnibox or pressing the clear button.  Used to
-  // distinguish between calls to textDidChange that are triggered by the user
-  // typing vs by calls to setText.
-  BOOL processingUserEvent_;
-}
-@end
-
-@implementation AutocompleteTextFieldDelegate
-- (id)initWithEditView:(OmniboxViewIOS*)editView {
-  if ((self = [super init])) {
-    editView_ = editView;
-    forwardingOnDidChange_ = NO;
-    processingUserEvent_ = NO;
-  }
-  return self;
-}
-
-- (BOOL)textField:(UITextField*)textField
-    shouldChangeCharactersInRange:(NSRange)range
-                replacementString:(NSString*)newText {
-  processingUserEvent_ = editView_->OnWillChange(range, newText);
-  return processingUserEvent_;
-}
-
-- (void)textFieldDidChange:(id)sender {
-  if (forwardingOnDidChange_)
-    return;
-
-  BOOL savedProcessingUserEvent = processingUserEvent_;
-  processingUserEvent_ = NO;
-  forwardingOnDidChange_ = YES;
-  editView_->OnDidChange(savedProcessingUserEvent);
-  forwardingOnDidChange_ = NO;
-}
-
-// Delegate method for UITextField, called when user presses the "go" button.
-- (BOOL)textFieldShouldReturn:(UITextField*)textField {
-  editView_->OnAccept();
-  return NO;
-}
-
-// Always update the text field colors when we start editing.  It's possible
-// for this method to be called when we are already editing (popup focus
-// change).  In this case, OnDidBeginEditing will be called multiple times.
-// If that becomes an issue a boolean should be added to track editing state.
-- (void)textFieldDidBeginEditing:(UITextField*)textField {
-  editView_->OnDidBeginEditing();
-}
-
-- (BOOL)textFieldShouldEndEditing:(UITextField*)textField {
-  editView_->OnWillEndEditing();
-  return YES;
-}
-
-// When editing, forward the message on to |editView_|.
-- (BOOL)textFieldShouldClear:(UITextField*)textField {
-  editView_->ClearText();
-  processingUserEvent_ = YES;
-  return YES;
-}
-
-- (BOOL)onCopy {
-  return editView_->OnCopy();
-}
-
-- (void)willPaste {
-  editView_->WillPaste();
-}
-
-- (void)onDeleteBackward {
-  editView_->OnDeleteBackward();
-}
-
-@end
-
 #pragma mark - OminboxViewIOS
 
 OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
@@ -176,29 +87,14 @@ OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
       attributing_display_string_(nil),
       popup_provider_(nullptr) {
   DCHECK(field_);
-  field_delegate_ =
-      [[AutocompleteTextFieldDelegate alloc] initWithEditView:this];
 
   paste_delegate_ = [[OmniboxTextFieldPasteDelegate alloc] init];
   [field_ setPasteDelegate:paste_delegate_];
 
-  [field_ setDelegate:field_delegate_];
-  [field_ addTarget:field_delegate_
-                action:@selector(textFieldDidChange:)
-      forControlEvents:UIControlEventEditingChanged];
   use_strikethrough_workaround_ = base::ios::IsRunningOnOrLater(10, 3, 0) &&
                                   !base::ios::IsRunningOnOrLater(11, 2, 0);
 }
 
-OmniboxViewIOS::~OmniboxViewIOS() {
-  // |field_| outlives this object.
-  [field_ setDelegate:nil];
-
-  [field_ removeTarget:field_delegate_
-                action:@selector(textFieldDidChange:)
-      forControlEvents:UIControlEventEditingChanged];
-}
-
 void OmniboxViewIOS::OpenMatch(const AutocompleteMatch& match,
                                WindowOpenDisposition disposition,
                                const GURL& alternate_nav_url,