Merge AutocompleteTextfield views::Textfield subclass into OmniboxView.

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Index: chrome/browser/ui/views/omnibox/omnibox_view_views.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index 2a8b73ffdd58bc9dff75de8c5970948449d6de99..70463e4538bcbe112eb321e7d3498cb1990aaf59 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -102,13 +102,6 @@ int GetEditFontPixelSize(bool popup_window_mode) {
                              kAutocompleteEditFontPixelSize;
 }
 
-// Copies |selected_text| as text to the primary clipboard.
-void DoCopyText(const string16& selected_text) {
-  ui::ScopedClipboardWriter scw(ui::Clipboard::GetForCurrentThread(),
-                                ui::Clipboard::BUFFER_STANDARD);
-  scw.WriteText(selected_text);
-}
-
 // This will write |url| and |text| to the clipboard as a well-formed URL.
 void DoCopyURL(const GURL& url, const string16& text) {
   BookmarkNodeData data;
@@ -118,79 +111,6 @@ void DoCopyURL(const GURL& url, const string16& text) {
 
 }  // namespace
 
-// Textfield for autocomplete that intercepts events that are necessary
-// for OmniboxViewViews.
-class OmniboxViewViews::AutocompleteTextfield : public views::Textfield {
- public:
-  AutocompleteTextfield(OmniboxViewViews* omnibox_view,
-                        LocationBarView* location_bar_view)
-      : views::Textfield(views::Textfield::STYLE_DEFAULT),
-        omnibox_view_(omnibox_view),
-        location_bar_view_(location_bar_view) {
-    DCHECK(omnibox_view_);
-    RemoveBorder();
-    set_id(VIEW_ID_OMNIBOX);
-  }
-
-  // views::View implementation
-  virtual void OnFocus() OVERRIDE {
-    views::Textfield::OnFocus();
-    omnibox_view_->HandleFocusIn();
-  }
-
-  virtual void OnBlur() OVERRIDE {
-    views::Textfield::OnBlur();
-    omnibox_view_->HandleFocusOut();
-  }
-
-  virtual bool OnKeyPressed(const ui::KeyEvent& event) OVERRIDE {
-    // Use our own implementation of paste. See OnPaste() for details.
-    if (!read_only() && event.IsControlDown() &&
-        event.key_code() == ui::VKEY_V) {
-      omnibox_view_->OnBeforePossibleChange();
-      omnibox_view_->OnPaste();
-      omnibox_view_->OnAfterPossibleChange();
-      return true;
-    }
-    bool handled = views::Textfield::OnKeyPressed(event);
-    return omnibox_view_->HandleAfterKeyEvent(event, handled) || handled;
-  }
-
-  virtual bool OnKeyReleased(const ui::KeyEvent& event) OVERRIDE {
-    return omnibox_view_->HandleKeyReleaseEvent(event);
-  }
-
-  virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE {
-    // Pass through the views::Textfield's return value; we don't need to
-    // override its behavior.
-    bool result = views::Textfield::OnMousePressed(event);
-    omnibox_view_->HandleMousePressEvent(event);
-    return result;
-  }
-
-  virtual bool OnMouseDragged(const ui::MouseEvent& event) OVERRIDE {
-    bool result = views::Textfield::OnMouseDragged(event);
-    omnibox_view_->HandleMouseDragEvent(event);
-    return result;
-  }
-
-  virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE {
-    views::Textfield::OnMouseReleased(event);
-    omnibox_view_->HandleMouseReleaseEvent(event);
-  }
-
-  virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE {
-    views::Textfield::OnGestureEvent(event);
-    omnibox_view_->HandleGestureEvent(*event);
-  }
-
- private:
-  OmniboxViewViews* omnibox_view_;
-  LocationBarView* location_bar_view_;
-
-  DISALLOW_COPY_AND_ASSIGN(AutocompleteTextfield);
-};
-
 // static
 const char OmniboxViewViews::kViewClassName[] = "BrowserOmniboxViewViews";
 
@@ -201,7 +121,6 @@ OmniboxViewViews::OmniboxViewViews(OmniboxEditController* controller,
                                    bool popup_window_mode,
                                    LocationBarView* location_bar)
     : OmniboxView(profile, controller, toolbar_model, command_updater),
-      textfield_(NULL),
       popup_window_mode_(popup_window_mode),
       security_level_(ToolbarModel::NONE),
       ime_composing_before_change_(false),
@@ -210,6 +129,8 @@ OmniboxViewViews::OmniboxViewViews(OmniboxEditController* controller,
       ime_candidate_window_open_(false),
       select_all_on_mouse_release_(false),
       select_all_on_gesture_tap_(false) {
+  RemoveBorder();
+  set_id(VIEW_ID_OMNIBOX);
 }
 
 OmniboxViewViews::~OmniboxViewViews() {
@@ -230,24 +151,21 @@ void OmniboxViewViews::Init() {
   // The height of the text view is going to change based on the font used.  We
   // don't want to stretch the height, and we want it vertically centered.
   // TODO(oshima): make sure the above happens with views.
-  textfield_ = new AutocompleteTextfield(this, location_bar_view_);
-  textfield_->SetController(this);
-  textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_URL);
-  textfield_->SetBackgroundColor(location_bar_view_->GetColor(
+  SetController(this);
+  SetTextInputType(ui::TEXT_INPUT_TYPE_URL);
+  SetBackgroundColor(location_bar_view_->GetColor(
       ToolbarModel::NONE, LocationBarView::BACKGROUND));
 
   if (popup_window_mode_)
-    textfield_->SetReadOnly(true);
+    SetReadOnly(true);
 
   const int font_size = GetEditFontPixelSize(popup_window_mode_);
-  const int old_size = textfield_->font().GetFontSize();
-  if (font_size != old_size)
-    textfield_->SetFont(textfield_->font().DeriveFont(font_size - old_size));
+  if (font_size != font().GetFontSize())
+    SetFont(font().DeriveFont(font_size - font().GetFontSize()));
 
-  // Create popup view using the same font as |textfield_|'s.
-  popup_view_.reset(
-      OmniboxPopupContentsView::Create(
-          textfield_->font(), this, model(), location_bar_view_));
+  // Initialize the popup view using the same font.
+  popup_view_.reset(OmniboxPopupContentsView::Create(
+      font(), this, model(), location_bar_view_));
 
   // A null-border to zero out the focused border on textfield.
   const int vertical_margin = !popup_window_mode_ ?
@@ -261,28 +179,92 @@ void OmniboxViewViews::Init() {
 }
 
 gfx::Font OmniboxViewViews::GetFont() {
-  return textfield_->font();
+  return font();
 }
 
 int OmniboxViewViews::WidthOfTextAfterCursor() {
   ui::Range sel;
-  textfield_->GetSelectedRange(&sel);
+  GetSelectedRange(&sel);
   // See comments in LocationBarView::Layout as to why this uses -1.
   const int start = std::max(0, static_cast<int>(sel.end()) - 1);
   // TODO: add horizontal margin.
-  return textfield_->font().GetStringWidth(GetText().substr(start));
+  return font().GetStringWidth(text().substr(start));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// OmniboxViewViews, views::View implementation:

[Peter Kasting, 2013/02/18 18:57:47]

Nit: Again, we don't directly override View.

Personally, I'm not a fan of these sorts of dividers between the different
blocks of virtual function implementations anyway.

[msw, 2013/02/18 20:58:21]

I'm not a big fan either, but I fixed it rather than deleting them throughout.

+
+std::string OmniboxViewViews::GetClassName() const {
+  return kViewClassName;
+}
+
+void OmniboxViewViews::OnGestureEvent(ui::GestureEvent* event) {
+  views::Textfield::OnGestureEvent(event);
+  if (!HasFocus() && event->type() == ui::ET_GESTURE_TAP_DOWN) {
+    select_all_on_gesture_tap_ = true;
+    return;
+  }
+  if (select_all_on_gesture_tap_ && event->type() == ui::ET_GESTURE_TAP)
+    SelectAll(false);
+  select_all_on_gesture_tap_ = false;
+}
+
+void OmniboxViewViews::GetAccessibleState(ui::AccessibleViewState* state) {
+  state->name = l10n_util::GetStringUTF16(IDS_ACCNAME_LOCATION);
+}
+
+void OmniboxViewViews::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  if (popup_view_->IsOpen())
+    popup_view_->UpdatePopupAppearance();
+}
+
+bool OmniboxViewViews::OnMousePressed(const ui::MouseEvent& event) {
+  const bool result = views::Textfield::OnMousePressed(event);
+  select_all_on_mouse_release_ =
+      (event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) &&
+      (!HasFocus() || (model()->focus_state() == OMNIBOX_FOCUS_INVISIBLE));
+  // Restore caret visibility whenever the user clicks in the omnibox in a way
+  // that would give it focus.  We must handle this case separately here because
+  // if the omnibox currently has invisible focus, the mouse event won't trigger
+  // either SetFocus() or OmniboxEditModel::OnSetFocus().
+  if (select_all_on_mouse_release_)
+    model()->SetCaretVisibility(true);
+  return result;
+}
+
+bool OmniboxViewViews::OnMouseDragged(const ui::MouseEvent& event) {
+  select_all_on_mouse_release_ = false;
+  return views::Textfield::OnMouseDragged(event);
+}
+
+void OmniboxViewViews::OnMouseReleased(const ui::MouseEvent& event) {
+  views::Textfield::OnMouseReleased(event);
+  if ((event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) &&
+      select_all_on_mouse_release_) {
+    // Select all in the reverse direction so as not to scroll the caret
+    // into view and shift the contents jarringly.
+    SelectAll(true);
+  }
+  select_all_on_mouse_release_ = false;
 }
 
-bool OmniboxViewViews::HandleAfterKeyEvent(const ui::KeyEvent& event,
-                                           bool handled) {
+bool OmniboxViewViews::OnKeyPressed(const ui::KeyEvent& event) {
+  // Use our own implementation of paste. See OnPaste() for details.
+  if (!read_only() && event.IsControlDown() &&
+      event.key_code() == ui::VKEY_V) {
+    OnBeforePossibleChange();
+    OnPaste();
+    OnAfterPossibleChange();
+    return true;
+  }
+  bool handled = views::Textfield::OnKeyPressed(event);
+
   if (event.key_code() == ui::VKEY_RETURN) {
     bool alt_held = event.IsAltDown();
     model()->AcceptInput(alt_held ? NEW_FOREGROUND_TAB : CURRENT_TAB, false);
     handled = true;
   } else if (!handled && event.key_code() == ui::VKEY_ESCAPE) {
-    // We can handle the Escape key if textfield did not handle it.
-    // If it's not handled by us, then we need to propagate it up to the parent
-    // widgets, so that Escape accelerator can still work.
+    // Let the model handle the Escape key or continue its propagation.
     handled = model()->OnEscapeKeyPressed();
   } else if (event.key_code() == ui::VKEY_CONTROL) {
     // Omnibox2 can switch its contents while pressing a control key. To switch
@@ -311,7 +293,7 @@ bool OmniboxViewViews::HandleAfterKeyEvent(const ui::KeyEvent& event,
       if (event.IsShiftDown() &&
           model()->popup_model()->selected_line_state() ==
               OmniboxPopupModel::KEYWORD) {
-        model()->ClearKeyword(GetText());
+        model()->ClearKeyword(text());
       } else {
         model()->OnUpOrDownKeyPressed(event.IsShiftDown() ? -1 : 1);
       }
@@ -319,24 +301,24 @@ bool OmniboxViewViews::HandleAfterKeyEvent(const ui::KeyEvent& event,
     } else {
       string16::size_type start = 0;
       string16::size_type end = 0;
-      size_t length = GetTextLength();
+      const size_t length = text().length();
       GetSelectionBounds(&start, &end);
       if (start != end || start < length) {
         OnBeforePossibleChange();
-        textfield_->SelectRange(ui::Range(length, length));
+        SelectRange(ui::Range(length, length));
         OnAfterPossibleChange();
         handled = true;
       }
 
-      // TODO(Oshima): handle instant
+      // TODO(msw|oshima): Handle Instant.
     }
   }
-  // TODO(oshima): page up & down
+  // TODO(msw|oshima): Handle page up and page down.
 
   return handled;
 }
 
-bool OmniboxViewViews::HandleKeyReleaseEvent(const ui::KeyEvent& event) {
+bool OmniboxViewViews::OnKeyReleased(const ui::KeyEvent& event) {
   // Omnibox2 can switch its contents while pressing a control key. To switch
   // the contents of omnibox2, we notify the OmniboxEditModel class when the
   // control-key state is changed.
@@ -349,51 +331,16 @@ bool OmniboxViewViews::HandleKeyReleaseEvent(const ui::KeyEvent& event) {
   return false;
 }
 
-void OmniboxViewViews::HandleMousePressEvent(const ui::MouseEvent& event) {
-  select_all_on_mouse_release_ =
-      (event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) &&
-      (!textfield_->HasFocus() ||
-       (model()->focus_state() == OMNIBOX_FOCUS_INVISIBLE));
-  // Restore caret visibility whenever the user clicks in the omnibox in a way
-  // that would give it focus.  We must handle this case separately here because
-  // if the omnibox currently has invisible focus, the mouse event won't trigger
-  // either SetFocus() or OmniboxEditModel::OnSetFocus().
-  if (select_all_on_mouse_release_)
-    model()->SetCaretVisibility(true);
-}
-
-void OmniboxViewViews::HandleMouseDragEvent(const ui::MouseEvent& event) {
-  select_all_on_mouse_release_ = false;
-}
-
-void OmniboxViewViews::HandleMouseReleaseEvent(const ui::MouseEvent& event) {
-  if ((event.IsOnlyLeftMouseButton() || event.IsOnlyRightMouseButton()) &&
-      select_all_on_mouse_release_) {
-    // Select all in the reverse direction so as not to scroll the caret
-    // into view and shift the contents jarringly.
-    SelectAll(true);
-  }
-  select_all_on_mouse_release_ = false;
-}
-
-void OmniboxViewViews::HandleGestureEvent(const ui::GestureEvent& event) {
-  if (!textfield_->HasFocus() && event.type() == ui::ET_GESTURE_TAP_DOWN) {
-    select_all_on_gesture_tap_ = true;
-    return;
-  }
-  if (select_all_on_gesture_tap_ && event.type() == ui::ET_GESTURE_TAP)
-    SelectAll(false);
-  select_all_on_gesture_tap_ = false;
-}
-
-void OmniboxViewViews::HandleFocusIn() {
+void OmniboxViewViews::OnFocus() {
+  views::Textfield::OnFocus();
   // TODO(oshima): Get control key state.
   model()->OnSetFocus(false);
   // Don't call controller()->OnSetFocus as this view has already
   // acquired the focus.
 }
 
-void OmniboxViewViews::HandleFocusOut() {
+void OmniboxViewViews::OnBlur() {
+  views::Textfield::OnBlur();
   gfx::NativeView native_view = NULL;
 #if defined(USE_AURA)
   views::Widget* widget = GetWidget();
@@ -412,36 +359,6 @@ void OmniboxViewViews::HandleFocusOut() {
   controller()->OnKillFocus();
 }
 
-void OmniboxViewViews::SetLocationEntryFocusable(bool focusable) {
-  textfield_->set_focusable(focusable);
-}
-
-bool OmniboxViewViews::IsLocationEntryFocusableInRootView() const {
-  return textfield_->IsFocusable();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// OmniboxViewViews, views::View implementation:
-void OmniboxViewViews::Layout() {
-  gfx::Insets insets = GetInsets();
-  textfield_->SetBounds(insets.left(), insets.top(),
-                        width() - insets.width(),
-                        height() - insets.height());
-}
-
-void OmniboxViewViews::GetAccessibleState(ui::AccessibleViewState* state) {
-  state->name = l10n_util::GetStringUTF16(IDS_ACCNAME_LOCATION);
-}
-
-std::string OmniboxViewViews::GetClassName() const {
-  return kViewClassName;
-}
-
-void OmniboxViewViews::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  if (popup_view_->IsOpen())
-    popup_view_->UpdatePopupAppearance();
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxViewViews, OmniboxView implementation:
 
@@ -451,15 +368,15 @@ void OmniboxViewViews::SaveStateToTab(WebContents* tab) {
   // We don't want to keep the IME status, so force quit the current
   // session here.  It may affect the selection status, so order is
   // also important.
-  if (textfield_->IsIMEComposing()) {
-    textfield_->GetTextInputClient()->ConfirmCompositionText();
-    textfield_->GetInputMethod()->CancelComposition(textfield_);
+  if (IsIMEComposing()) {
+    GetTextInputClient()->ConfirmCompositionText();
+    GetInputMethod()->CancelComposition(this);
   }
 
   // NOTE: GetStateForTabSwitch may affect GetSelection, so order is important.
   OmniboxEditModel::State model_state = model()->GetStateForTabSwitch();
   gfx::SelectionModel selection;
-  textfield_->GetSelectionModel(&selection);
+  GetSelectionModel(&selection);
   tab->SetUserData(
       kAutocompleteEditStateKey,
       new AutocompleteEditState(model_state, ViewState(selection)));
@@ -486,10 +403,10 @@ void OmniboxViewViews::Update(const WebContents* contents) {
 
       // Move the marks for the cursor and the other end of the selection to
       // the previously-saved offsets (but preserve PRIMARY).
-      textfield_->SelectSelectionModel(state->view_state.selection_model);
+      SelectSelectionModel(state->view_state.selection_model);
       // We do not carry over the current edit history to another tab.
       // TODO(oshima): consider saving/restoring edit history.
-      textfield_->ClearEditHistory();
+      ClearEditHistory();
     }
   } else if (visibly_changed_permanent_text) {
     RevertAll();
@@ -500,7 +417,7 @@ void OmniboxViewViews::Update(const WebContents* contents) {
 
 string16 OmniboxViewViews::GetText() const {
   // TODO(oshima): IME support
-  return textfield_->text();
+  return text();
 }
 
 void OmniboxViewViews::SetWindowTextAndCaretPos(const string16& text,
@@ -518,17 +435,17 @@ void OmniboxViewViews::SetWindowTextAndCaretPos(const string16& text,
 }
 
 void OmniboxViewViews::SetForcedQuery() {
-  const string16 current_text(GetText());
+  const string16 current_text(text());
   const size_t start = current_text.find_first_not_of(kWhitespaceUTF16);
   if (start == string16::npos || (current_text[start] != '?'))
     SetUserText(ASCIIToUTF16("?"));
   else
-    textfield_->SelectRange(ui::Range(current_text.size(), start + 1));
+    SelectRange(ui::Range(current_text.size(), start + 1));
 }
 
 bool OmniboxViewViews::IsSelectAll() const {
   // TODO(oshima): IME support.
-  return textfield_->text() == textfield_->GetSelectedText();
+  return text() == GetSelectedText();
 }
 
 bool OmniboxViewViews::DeleteAtEndPressed() {
@@ -538,12 +455,12 @@ bool OmniboxViewViews::DeleteAtEndPressed() {
 void OmniboxViewViews::GetSelectionBounds(string16::size_type* start,
                                           string16::size_type* end) const {
   ui::Range range;
-  textfield_->GetSelectedRange(&range);
+  GetSelectedRange(&range);
   if (range.is_empty()) {
     // Omnibox API expects that selection bounds is at cursor position
     // if there is no selection.
-    *start = textfield_->GetCursorPosition();
-    *end = textfield_->GetCursorPosition();
+    *start = GetCursorPosition();
+    *end = GetCursorPosition();
   } else {
     *start = static_cast<size_t>(range.end());
     *end = static_cast<size_t>(range.start());
@@ -551,7 +468,7 @@ void OmniboxViewViews::GetSelectionBounds(string16::size_type* start,
 }
 
 void OmniboxViewViews::SelectAll(bool reversed) {
-  textfield_->SelectAll(reversed);
+  views::Textfield::SelectAll(reversed);
 }
 
 void OmniboxViewViews::UpdatePopup() {
@@ -561,19 +478,16 @@ void OmniboxViewViews::UpdatePopup() {
   if (!model()->has_focus())
     return;
 
-  // Don't inline autocomplete when the caret/selection isn't at the end of
-  // the text, or in the middle of composition.
+  // Prevent inline autocomplete when the caret isn't at the end of the text,
+  // and during IME composition editing.
   ui::Range sel;
-  textfield_->GetSelectedRange(&sel);
-  bool no_inline_autocomplete =
-      sel.GetMax() < GetTextLength() || textfield_->IsIMEComposing();
-
-  model()->StartAutocomplete(!sel.is_empty(), no_inline_autocomplete);
+  GetSelectedRange(&sel);
+  model()->StartAutocomplete(!sel.is_empty(),

[Peter Kasting, 2013/02/18 18:57:47]

Nit: Each line with arguments should have the arguments starting at the same
horizontal position

[msw, 2013/02/18 20:58:21]

Done.

+      sel.GetMax() < text().length() || IsIMEComposing());
 }
 
 void OmniboxViewViews::SetFocus() {
-  // In views-implementation, the focus is on textfield rather than OmniboxView.
-  textfield_->RequestFocus();
+  RequestFocus();
   // Restore caret visibility if focus is explicitly requested. This is
   // necessary because if we already have invisible focus, the RequestFocus()
   // call above will short-circuit, preventing us from reaching
@@ -583,7 +497,7 @@ void OmniboxViewViews::SetFocus() {
 }
 
 void OmniboxViewViews::ApplyCaretVisibility() {
-  textfield_->SetCursorEnabled(model()->is_caret_visible());
+  SetCursorEnabled(model()->is_caret_visible());
 }
 
 void OmniboxViewViews::OnTemporaryTextMaybeChanged(
@@ -591,7 +505,7 @@ void OmniboxViewViews::OnTemporaryTextMaybeChanged(
     bool save_original_selection,
     bool notify_text_changed) {
   if (save_original_selection)
-    textfield_->GetSelectedRange(&saved_temporary_selection_);
+    GetSelectedRange(&saved_temporary_selection_);
 
   SetWindowTextAndCaretPos(display_text, display_text.length(), false,
                            notify_text_changed);
@@ -600,7 +514,7 @@ void OmniboxViewViews::OnTemporaryTextMaybeChanged(
 bool OmniboxViewViews::OnInlineAutocompleteTextMaybeChanged(
     const string16& display_text,
     size_t user_text_length) {
-  if (display_text == GetText())
+  if (display_text == text())
     return false;
   ui::Range range(display_text.size(), user_text_length);
   SetTextAndSelectedRange(display_text, range);
@@ -609,24 +523,25 @@ bool OmniboxViewViews::OnInlineAutocompleteTextMaybeChanged(
 }
 
 void OmniboxViewViews::OnRevertTemporaryText() {
-  textfield_->SelectRange(saved_temporary_selection_);
+  SelectRange(saved_temporary_selection_);
+  TextChanged();
 }
 
 void OmniboxViewViews::OnBeforePossibleChange() {
   // Record our state.
-  text_before_change_ = GetText();
-  textfield_->GetSelectedRange(&sel_before_change_);
-  ime_composing_before_change_ = textfield_->IsIMEComposing();
+  text_before_change_ = text();
+  GetSelectedRange(&sel_before_change_);
+  ime_composing_before_change_ = IsIMEComposing();
 }
 
 bool OmniboxViewViews::OnAfterPossibleChange() {
   ui::Range new_sel;
-  textfield_->GetSelectedRange(&new_sel);
+  GetSelectedRange(&new_sel);
 
   // See if the text or selection have changed since OnBeforePossibleChange().
-  const string16 new_text = GetText();
+  const string16 new_text = text();
   const bool text_changed = (new_text != text_before_change_) ||
-      (ime_composing_before_change_ != textfield_->IsIMEComposing());
+      (ime_composing_before_change_ != IsIMEComposing());
   const bool selection_differs =
       !((sel_before_change_.is_empty() && new_sel.is_empty()) ||
         sel_before_change_.EqualsIgnoringDirection(new_sel));
@@ -642,8 +557,7 @@ bool OmniboxViewViews::OnAfterPossibleChange() {
 
   const bool something_changed = model()->OnAfterPossibleChange(
       text_before_change_, new_text, new_sel.start(), new_sel.end(),
-      selection_differs, text_changed, just_deleted_text,
-      !textfield_->IsIMEComposing());
+      selection_differs, text_changed, just_deleted_text, !IsIMEComposing());
 
   // If only selection was changed, we don't need to call model()'s
   // OnChanged() method, which is called in TextChanged().
@@ -683,7 +597,7 @@ string16 OmniboxViewViews::GetInstantSuggestion() const {
 
 int OmniboxViewViews::TextWidth() const {
   // TODO(oshima): add horizontal margin.
-  return textfield_->font().GetStringWidth(textfield_->text());
+  return font().GetStringWidth(text());
 }
 
 bool OmniboxViewViews::IsImeComposing() const {
@@ -696,7 +610,6 @@ int OmniboxViewViews::GetMaxEditWidth(int entry_width) const {
 
 views::View* OmniboxViewViews::AddToView(views::View* parent) {
   parent->AddChildView(this);

[Peter Kasting, 2013/02/18 18:57:47]

Can we nuke this function entirely now?  Seems like callers could just do the
AddChildView() call directly.

[msw, 2013/02/18 20:58:21]

I'll try this in a followup, OmniboxViewWin::AddToView isn't as straightforward.

-  AddChildView(textfield_);
   return this;
 }
 
@@ -720,27 +633,25 @@ bool OmniboxViewViews::HandleKeyEvent(views::Textfield* textfield,
     // Checks if it's currently in keyword search mode.
     if (model()->is_keyword_hint() || model()->keyword().empty())

[Peter Kasting, 2013/02/18 18:57:47]

Nit: I wouldn't mind just combining these three conditionals.

[msw, 2013/02/18 20:58:21]

Done.

       return false;
-    // If there is selection, let textfield handle the backspace.
-    if (textfield_->HasSelection())
+    // No extra handling is needed if there is a selection.
+    if (HasSelection())
       return false;
-    // If not at the begining of the text, let textfield handle the backspace.
-    if (textfield_->GetCursorPosition())
+    // No extra handling is needed if the cursor is not leading the text.
+    if (GetCursorPosition())
       return false;
-    model()->ClearKeyword(GetText());
+    model()->ClearKeyword(text());
     return true;
   }
 
   if (event.key_code() == ui::VKEY_DELETE && !event.IsAltDown()) {
     delete_at_end_pressed_ =
-        (!textfield_->HasSelection() &&
-         textfield_->GetCursorPosition() == textfield_->text().length());
+        (!HasSelection() && GetCursorPosition() == text().length());
   }
 
-  // Though the Textfield usually handles the right-arrow key for LTR text or
-  // left-arrow key for RTL text, we need to handle it if we have gray text
-  // (Instant suggestion) that needs to be committed.
-  if (textfield_->GetCursorPosition() == textfield_->text().length()) {
-    base::i18n::TextDirection direction = textfield_->GetTextDirection();
+  // Handle the right-arrow key for LTR text and the left-arrow key for RTL text
+  // if there is an Instant suggestion (gray text) that needs to be committed.
+  if (GetCursorPosition() == text().length()) {
+    base::i18n::TextDirection direction = GetTextDirection();
     if ((direction == base::i18n::LEFT_TO_RIGHT &&
          event.key_code() == ui::VKEY_RIGHT) ||
         (direction == base::i18n::RIGHT_TO_LEFT &&
@@ -762,29 +673,30 @@ void OmniboxViewViews::OnAfterUserAction(views::Textfield* sender) {
 
 void OmniboxViewViews::OnAfterCutOrCopy() {
   ui::Range selection_range;
-  textfield_->GetSelectedRange(&selection_range);
+  GetSelectedRange(&selection_range);
   ui::Clipboard* cb = ui::Clipboard::GetForCurrentThread();
   string16 selected_text;
   cb->ReadText(ui::Clipboard::BUFFER_STANDARD, &selected_text);
-  const string16 text = textfield_->text();
   GURL url;
   bool write_url;
-  model()->AdjustTextForCopy(selection_range.GetMin(), selected_text == text,
+  model()->AdjustTextForCopy(selection_range.GetMin(), selected_text == text(),
       &selected_text, &url, &write_url);
-  if (write_url)
+  if (write_url) {
     DoCopyURL(url, selected_text);
-  else
-    DoCopyText(selected_text);
+  } else {
+    ui::ScopedClipboardWriter scoped_clipboard_writer(
+        ui::Clipboard::GetForCurrentThread(), ui::Clipboard::BUFFER_STANDARD);
+    scoped_clipboard_writer.WriteText(selected_text);
+  }
 }
 
 void OmniboxViewViews::OnWriteDragData(ui::OSExchangeData* data) {
   ui::Range selection_range;
-  textfield_->GetSelectedRange(&selection_range);
-  string16 selected_text = textfield_->GetSelectedText();
-  const string16 text = textfield_->text();
+  GetSelectedRange(&selection_range);
+  string16 selected_text = GetSelectedText();
   GURL url;
   bool write_url;
-  model()->AdjustTextForCopy(selection_range.start(), selected_text == text,
+  model()->AdjustTextForCopy(selection_range.start(), selected_text == text(),
       &selected_text, &url, &write_url);
   data->SetString(selected_text);
   if (write_url)
@@ -895,13 +807,9 @@ void OmniboxViewViews::CandidateWindowClosed(
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxViewViews, private:
 
-size_t OmniboxViewViews::GetTextLength() const {
-  // TODO(oshima): Support instant, IME.
-  return textfield_->text().length();
-}
-
 int OmniboxViewViews::GetOmniboxTextLength() const {
-  return static_cast<int>(GetTextLength());
+  // TODO(oshima): Support instant, IME.
+  return static_cast<int>(text().length());
 }
 
 void OmniboxViewViews::EmphasizeURLComponents() {
@@ -911,15 +819,14 @@ void OmniboxViewViews::EmphasizeURLComponents() {
   // this input.  This can tell us whether an UNKNOWN input string is going to
   // be treated as a search or a navigation, and is the same method the Paste
   // And Go system uses.
-  string16 text = GetText();
   url_parse::Component scheme, host;
-  AutocompleteInput::ParseForEmphasizeComponents(text, model()->GetDesiredTLD(),
-                                                 &scheme, &host);
+  AutocompleteInput::ParseForEmphasizeComponents(
+      text(), model()->GetDesiredTLD(), &scheme, &host);
   const bool emphasize = model()->CurrentTextIsURL() && (host.len > 0);
-  textfield_->SetColor(location_bar_view_->GetColor(security_level_,
+  SetColor(location_bar_view_->GetColor(security_level_,
       emphasize ? LocationBarView::DEEMPHASIZED_TEXT : LocationBarView::TEXT));
   if (emphasize) {
-    textfield_->ApplyColor(
+    ApplyColor(
         location_bar_view_->GetColor(security_level_, LocationBarView::TEXT),
         ui::Range(host.begin, host.end()));
   }
@@ -935,21 +842,20 @@ void OmniboxViewViews::EmphasizeURLComponents() {
         security_level_, LocationBarView::SECURITY_TEXT);
     const bool strike = (security_level_ == ToolbarModel::SECURITY_ERROR);
     const ui::Range scheme_range(scheme.begin, scheme.end());
-    textfield_->ApplyColor(security_color, scheme_range);
-    textfield_->ApplyStyle(gfx::DIAGONAL_STRIKE, strike, scheme_range);
+    ApplyColor(security_color, scheme_range);
+    ApplyStyle(gfx::DIAGONAL_STRIKE, strike, scheme_range);
   }
 }
 
 void OmniboxViewViews::SetTextAndSelectedRange(const string16& text,
                                                const ui::Range& range) {
-  if (text != GetText())
-    textfield_->SetText(text);
-  textfield_->SelectRange(range);
+  SetText(text);
+  SelectRange(range);
 }
 
 string16 OmniboxViewViews::GetSelectedText() const {
   // TODO(oshima): Support instant, IME.
-  return textfield_->GetSelectedText();
+  return views::Textfield::GetSelectedText();
 }
 
 void OmniboxViewViews::CopyURL() {
@@ -963,9 +869,8 @@ void OmniboxViewViews::OnPaste() {
     // Record this paste, so we can do different behavior.
     model()->on_paste();
     // Force a Paste operation to trigger the text_changed code in
-    // OnAfterPossibleChange(), even if identical contents are pasted into the
-    // text box.
+    // OnAfterPossibleChange(), even if identical contents are pasted.
     text_before_change_.clear();
-    textfield_->ReplaceSelection(text);
+    ReplaceSelection(text);
   }
 }