Move clear button logic from LocationBarControllerImpl to Omnibox

Since the clear button is in omnibox, it makes sense for it to be
directly configured in the omnibox layer, without going to the location
bar.

Bug: None
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I51be645345550517e120939416a7e0664b74cf87
Reviewed-on: https://chromium-review.googlesource.com/924424
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537857}

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

@@ -29,7 +29,6 @@ class WebState;
 @protocol LocationBarURLLoader;
 @class PageInfoBridge;
 class OmniboxViewIOS;
-@class OmniboxClearButtonBridge;
 @class OmniboxPopupCoordinator;
 @protocol OmniboxPopupPositioner;
 @class LocationBarView;
@@ -89,18 +88,10 @@ class LocationBarControllerImpl : public LocationBarController,
   // Does nothing on tablet.
   void InstallVoiceSearchIcon();
 
-  // Creates the clear text UIButton to be used as a right view of |field_|.
-  void CreateClearTextIcon(bool is_incognito);
-
-  // Updates the view to show the appropriate button (e.g. clear text or voice
-  // search) on the right side of |field_|.
-  void UpdateRightDecorations();
 
   bool show_hint_text_;
-  __strong UIButton* clear_text_button_;
   std::unique_ptr<OmniboxViewIOS> edit_view_;
 
-  __strong OmniboxClearButtonBridge* clear_button_bridge_;
   // A bridge from a UIControl action to the dispatcher to display a page
   // info popup.
   __strong PageInfoBridge* page_info_bridge_;

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

@@ -43,8 +43,6 @@
 #endif
 
 namespace {
-const CGFloat kClearTextButtonWidth = 28;
-const CGFloat kClearTextButtonHeight = 28;
 
 // Workaround for https://crbug.com/527084 . If there is connection
 // information, always show the icon. Remove this once connection info
@@ -81,36 +79,6 @@ bool IsCurrentPageOffline(web::WebState* webState) {
 
 }  // namespace
 
-// An ObjC bridge class to allow taps on the clear button to be sent to a C++
-// class.
-@interface OmniboxClearButtonBridge : NSObject
-
-- (instancetype)initWithOmniboxView:(OmniboxViewIOS*)omniboxView
-    NS_DESIGNATED_INITIALIZER;
-
-- (instancetype)init NS_UNAVAILABLE;
-
-- (void)clearText;
-
-@end
-
-@implementation OmniboxClearButtonBridge {
-  OmniboxViewIOS* _omniboxView;
-}
-
-- (instancetype)initWithOmniboxView:(OmniboxViewIOS*)omniboxView {
-  self = [super init];
-  if (self) {
-    _omniboxView = omniboxView;
-  }
-  return self;
-}
-
-- (void)clearText {
-  _omniboxView->ClearText();
-}
-
-@end
 
 // An ObjC bridge class to map between a UIControl action and the
 // dispatcher command that displays the page info popup.
@@ -156,7 +124,6 @@ LocationBarControllerImpl::LocationBarControllerImpl(
   show_hint_text_ = true;
 
   InstallLocationIcon();
-  CreateClearTextIcon(browser_state->IsOffTheRecord());
 }
 
 LocationBarControllerImpl::~LocationBarControllerImpl() {}
@@ -234,7 +201,6 @@ void LocationBarControllerImpl::OnChanged() {
       }
     }
   }
-  UpdateRightDecorations();
 
   NSString* placeholderText =
       show_hint_text_ ? l10n_util::GetNSString(IDS_OMNIBOX_EMPTY_HINT) : nil;
@@ -277,7 +243,6 @@ void LocationBarControllerImpl::OnKillFocus() {
   // Stop disabling fullscreen since the loation bar is no longer focused.
   fullscreen_disabler_ = nullptr;
 
-  UpdateRightDecorations();
   [delegate_ locationBarHasResignedFirstResponder];
 }
 
@@ -305,7 +270,6 @@ void LocationBarControllerImpl::OnSetFocus() {
       FullscreenControllerFactory::GetInstance()->GetForBrowserState(
           browser_state_));
 
-  UpdateRightDecorations();
   [delegate_ locationBarHasBecomeFirstResponder];
 }
 
@@ -348,47 +312,6 @@ void LocationBarControllerImpl::InstallLocationIcon() {
   [location_bar_view_ setLeadingButtonHidden:!IsIPadIdiom()];
 }
 
-void LocationBarControllerImpl::CreateClearTextIcon(bool is_incognito) {
-  UIButton* button = [UIButton buttonWithType:UIButtonTypeCustom];
-  UIImage* omniBoxClearImage = is_incognito
-                                   ? NativeImage(IDR_IOS_OMNIBOX_CLEAR_OTR)
-                                   : NativeImage(IDR_IOS_OMNIBOX_CLEAR);
-  UIImage* omniBoxClearPressedImage =
-      is_incognito ? NativeImage(IDR_IOS_OMNIBOX_CLEAR_OTR_PRESSED)
-                   : NativeImage(IDR_IOS_OMNIBOX_CLEAR_PRESSED);
-  [button setImage:omniBoxClearImage forState:UIControlStateNormal];
-  [button setImage:omniBoxClearPressedImage forState:UIControlStateHighlighted];
-
-  CGRect frame = CGRectZero;
-  frame.size = CGSizeMake(kClearTextButtonWidth, kClearTextButtonHeight);
-  [button setFrame:frame];
-
-  clear_button_bridge_ =
-      [[OmniboxClearButtonBridge alloc] initWithOmniboxView:edit_view_.get()];
-  [button addTarget:clear_button_bridge_
-                action:@selector(clearText)
-      forControlEvents:UIControlEventTouchUpInside];
-  clear_text_button_ = button;
-
-  SetA11yLabelAndUiAutomationName(clear_text_button_,
-                                  IDS_IOS_ACCNAME_CLEAR_TEXT, @"Clear Text");
-}
-
-void LocationBarControllerImpl::UpdateRightDecorations() {
-  DCHECK(clear_text_button_);
-  if (!edit_view_->model()->has_focus()) {
-    // Do nothing for iPhone. The right view will be set to nil after the
-    // omnibox animation is completed.
-    if (IsIPadIdiom())
-      [location_bar_view_.textField setRightView:nil];
-  } else if ([location_bar_view_.textField displayedText].empty()) {
-    [location_bar_view_.textField setRightView:nil];
-  } else {
-    [location_bar_view_.textField setRightView:clear_text_button_];
-    [clear_text_button_ setAlpha:1];
-  }
-}
-
 #pragma mark - LeftImageProvider
 
 void LocationBarControllerImpl::SetLeftImage(int imageId) {

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

@@ -21,6 +21,7 @@ class GURL;
 @class OmniboxTextFieldIOS;
 @class OmniboxTextFieldPasteDelegate;
 class WebOmniboxEditController;
+@class OmniboxClearButtonBridge;
 
 namespace ios {
 class ChromeBrowserState;
@@ -157,6 +158,13 @@ class OmniboxViewIOS : public OmniboxView,
   void EmphasizeURLComponents() override;
 
  private:
+  // Creates the clear text UIButton to be used as a right view of |field_|.
+  void CreateClearTextIcon(bool is_incognito);
+
+  // Updates the view to show the appropriate button (e.g. clear text or voice
+  // search) on the right side of |field_|.
+  void UpdateRightDecorations();
+
   // Calculates text attributes according to |display_text| and
   // returns them in an autoreleased object.
   NSAttributedString* ApplyTextAttributes(const base::string16& text);
@@ -178,6 +186,10 @@ class OmniboxViewIOS : public OmniboxView,
   ios::ChromeBrowserState* browser_state_;
 
   OmniboxTextFieldIOS* field_;
+  __strong UIButton* clear_text_button_;
+
+  __strong OmniboxClearButtonBridge* clear_button_bridge_;
+
   OmniboxTextFieldPasteDelegate* paste_delegate_;
   WebOmniboxEditController* controller_;  // weak, owns us
   LeftImageProvider* left_image_provider_;  // weak

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

@@ -27,6 +27,8 @@
 #include "ios/chrome/browser/ui/omnibox/omnibox_util.h"
 #include "ios/chrome/browser/ui/omnibox/web_omnibox_edit_controller.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"
 #include "ios/chrome/grit/ios_theme_resources.h"
 #include "ios/web/public/referrer.h"
 #import "net/base/mac/url_conversions.h"
@@ -44,6 +46,9 @@
 using base::UserMetricsAction;
 
 namespace {
+const CGFloat kClearTextButtonWidth = 28;
+const CGFloat kClearTextButtonHeight = 28;
+
 // The color of the rest of the URL (i.e. after the TLD) in the omnibox.
 UIColor* BaseTextColor() {
   return [UIColor colorWithWhite:(161 / 255.0) alpha:1.0];
@@ -66,6 +71,8 @@ UIColor* IncognitoSecureTextColor() {
 
 }  // namespace
 
+#pragma mark - AutocompleteTextFieldDelegate
+
 // Simple Obj-C object to forward UITextFieldDelegate method calls back to the
 // OmniboxViewIOS.
 @interface AutocompleteTextFieldDelegate : NSObject<OmniboxTextFieldDelegate> {
@@ -157,6 +164,41 @@ UIColor* IncognitoSecureTextColor() {
 
 @end
 
+#pragma mark - OmniboxClearButtonBridge
+
+// An ObjC bridge class to allow taps on the clear button to be sent to a C++
+// class.
+@interface OmniboxClearButtonBridge : NSObject
+
+- (instancetype)initWithOmniboxView:(OmniboxViewIOS*)omniboxView
+    NS_DESIGNATED_INITIALIZER;
+
+- (instancetype)init NS_UNAVAILABLE;
+
+- (void)clearText;
+
+@end
+
+@implementation OmniboxClearButtonBridge {
+  OmniboxViewIOS* _omniboxView;
+}
+
+- (instancetype)initWithOmniboxView:(OmniboxViewIOS*)omniboxView {
+  self = [super init];
+  if (self) {
+    _omniboxView = omniboxView;
+  }
+  return self;
+}
+
+- (void)clearText {
+  _omniboxView->ClearText();
+}
+
+@end
+
+#pragma mark - OminboxViewIOS
+
 OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
                                WebOmniboxEditController* controller,
                                LeftImageProvider* left_image_provider,
@@ -186,6 +228,8 @@ OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
       forControlEvents:UIControlEventEditingChanged];
   use_strikethrough_workaround_ = base::ios::IsRunningOnOrLater(10, 3, 0) &&
                                   !base::ios::IsRunningOnOrLater(11, 2, 0);
+
+  CreateClearTextIcon(browser_state->IsOffTheRecord());
 }
 
 OmniboxViewIOS::~OmniboxViewIOS() {
@@ -383,6 +427,8 @@ void OmniboxViewIOS::OnDidBeginEditing() {
   if (!popup_was_open_before_editing_began)
     [field_ enterPreEditState];
 
+  UpdateRightDecorations();
+
   // The controller looks at the current pre-edit state, so the call to
   // OnSetFocus() must come after entering pre-edit.
   controller_->OnSetFocus();
@@ -395,6 +441,8 @@ void OmniboxViewIOS::OnDidEndEditing() {
   if ([field_ isPreEditing])
     [field_ exitPreEditState];
 
+  UpdateRightDecorations();
+
   // The controller looks at the current pre-edit state, so the call to
   // OnKillFocus() must come after exiting pre-edit.
   controller_->OnKillFocus();
@@ -498,6 +546,8 @@ void OmniboxViewIOS::OnDidChange(bool processing_user_event) {
     }
   }
 
+  UpdateRightDecorations();
+
   // Clear the autocomplete text, since the omnibox model does not expect to see
   // it in OnAfterPossibleChange().  Clearing the text here should not cause
   // flicker as the UI will not get a chance to redraw before the new
@@ -717,6 +767,47 @@ void OmniboxViewIOS::UpdateAppearance() {
   }
 }
 
+void OmniboxViewIOS::CreateClearTextIcon(bool is_incognito) {
+  UIButton* button = [UIButton buttonWithType:UIButtonTypeCustom];
+  UIImage* omniBoxClearImage = is_incognito
+                                   ? NativeImage(IDR_IOS_OMNIBOX_CLEAR_OTR)
+                                   : NativeImage(IDR_IOS_OMNIBOX_CLEAR);
+  UIImage* omniBoxClearPressedImage =
+      is_incognito ? NativeImage(IDR_IOS_OMNIBOX_CLEAR_OTR_PRESSED)
+                   : NativeImage(IDR_IOS_OMNIBOX_CLEAR_PRESSED);
+  [button setImage:omniBoxClearImage forState:UIControlStateNormal];
+  [button setImage:omniBoxClearPressedImage forState:UIControlStateHighlighted];
+
+  CGRect frame = CGRectZero;
+  frame.size = CGSizeMake(kClearTextButtonWidth, kClearTextButtonHeight);
+  [button setFrame:frame];
+
+  clear_button_bridge_ =
+      [[OmniboxClearButtonBridge alloc] initWithOmniboxView:this];
+  [button addTarget:clear_button_bridge_
+                action:@selector(clearText)
+      forControlEvents:UIControlEventTouchUpInside];
+  clear_text_button_ = button;
+
+  SetA11yLabelAndUiAutomationName(clear_text_button_,
+                                  IDS_IOS_ACCNAME_CLEAR_TEXT, @"Clear Text");
+}
+
+void OmniboxViewIOS::UpdateRightDecorations() {
+  DCHECK(clear_text_button_);
+  if (!model()->has_focus()) {
+    // Do nothing for iPhone. The right view will be set to nil after the
+    // omnibox animation is completed.
+    if (IsIPadIdiom())
+      [field_ setRightView:nil];
+  } else if ([field_ displayedText].empty()) {
+    [field_ setRightView:nil];
+  } else {
+    [field_ setRightView:clear_text_button_];
+    [clear_text_button_ setAlpha:1];
+  }
+}
+
 void OmniboxViewIOS::OnDeleteBackward() {
   if ([field_ text].length == 0) {
     // If the user taps backspace while the pre-edit text is showing,