Moving common code into OmniboxView from OmniboxView*

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

Index: chrome/browser/ui/views/omnibox/omnibox_view_win.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
index 44a6e1ae928ea5d0d20920314ef45b40fde28138..ed68fedadc1daaf0d25cdbf707a21b812a1ad52f 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_win.cc
@@ -439,14 +439,12 @@ OmniboxViewWin::OmniboxViewWin(OmniboxEditController* controller,
                                bool popup_window_mode,
                                views::View* location_bar,
                                views::View* popup_parent_view)
-    : model_(new OmniboxEditModel(this, controller, parent_view->profile())),
+    : OmniboxView(parent_view->profile(), controller, toolbar_model,
+          command_updater),
       popup_view_(OmniboxPopupContentsView::Create(
           parent_view->font(), this, model_.get(), location_bar,
           popup_parent_view)),
-      controller_(controller),
       parent_view_(parent_view),
-      toolbar_model_(toolbar_model),
-      command_updater_(command_updater),
       popup_window_mode_(popup_window_mode),
       force_hidden_(false),
       tracking_click_(),
@@ -628,15 +626,12 @@ void OmniboxViewWin::OpenMatch(const AutocompleteMatch& match,
                                WindowOpenDisposition disposition,
                                const GURL& alternate_nav_url,
                                size_t selected_line) {
-  if (!match.destination_url.is_valid())
-    return;
-
   // When we navigate, we first revert to the unedited state, then if necessary
   // synchronously change the permanent text to the new URL.  If we don't freeze
   // here, the user could potentially see a flicker of the current URL before
   // the new one reappears, which would look glitchy.
   ScopedFreeze freeze(this, GetTextObjectModel());
-  model_->OpenMatch(match, disposition, alternate_nav_url, selected_line);
+  OmniboxView::OpenMatch(match, disposition, alternate_nav_url, selected_line);
 }
 
 string16 OmniboxViewWin::GetText() const {
@@ -647,28 +642,12 @@ string16 OmniboxViewWin::GetText() const {
   return str;
 }
 
-bool OmniboxViewWin::IsEditingOrEmpty() const {
-  return model_->user_input_in_progress() || (GetTextLength() == 0);
-}
-
-int OmniboxViewWin::GetIcon() const {
-  return IsEditingOrEmpty() ?
-      AutocompleteMatch::TypeToIcon(model_->CurrentTextType()) :
-      toolbar_model_->GetIcon();
-}
-
-void OmniboxViewWin::SetUserText(const string16& text) {
-  SetUserText(text, text, true);
-}
-
 void OmniboxViewWin::SetUserText(const string16& text,
                                  const string16& display_text,
                                  bool update_popup) {
   ScopedFreeze freeze(this, GetTextObjectModel());
-  model_->SetUserText(text);
   saved_selection_for_focus_change_.cpMin = -1;
-  SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup,
-      true);
+  OmniboxView::SetUserText(text, display_text, update_popup);
 }
 
 void OmniboxViewWin::SetWindowTextAndCaretPos(const string16& text,
@@ -689,7 +668,7 @@ void OmniboxViewWin::SetForcedQuery() {
   const string16 current_text(GetText());
   const size_t start = current_text.find_first_not_of(kWhitespaceWide);
   if (start == string16::npos || (current_text[start] != '?'))
-    SetUserText(L"?");
+    OmniboxView::SetUserText(L"?");

[Peter Kasting, 2012/07/26 03:59:17]

Why is this a direct superclass call?  That changes the functionality.

[dominich, 2012/07/26 22:33:24]

There is no version in the derived class that takes a single parameter. Instead,
we call the base class version which then dispatches to the virtual call with
multiple parameters.

[Peter Kasting, 2012/07/26 23:03:24]

Oh, and scope resolution requires that we qualify this for it to compile?  Ugh.

   else
     SetSelection(current_text.length(), start + 1);
 }
@@ -721,9 +700,8 @@ void OmniboxViewWin::SelectAll(bool reversed) {
 
 void OmniboxViewWin::RevertAll() {
   ScopedFreeze freeze(this, GetTextObjectModel());
-  ClosePopup();
   saved_selection_for_focus_change_.cpMin = -1;
-  model_->Revert();
+  OmniboxView::RevertAll();
 }
 
 void OmniboxViewWin::UpdatePopup() {
@@ -758,10 +736,6 @@ void OmniboxViewWin::UpdatePopup() {
                             (sel.cpMax < GetTextLength()) || IsImeComposing());
 }
 
-void OmniboxViewWin::ClosePopup() {
-  model_->StopAutocomplete();
-}
-
 void OmniboxViewWin::SetFocus() {
   ::SetFocus(m_hWnd);
 }
@@ -941,10 +915,6 @@ gfx::NativeView OmniboxViewWin::GetRelativeWindowForPopup() const {
   return GetRelativeWindowForNativeView(GetNativeView());
 }
 
-CommandUpdater* OmniboxViewWin::GetCommandUpdater() {
-  return command_updater_;
-}
-
 void OmniboxViewWin::SetInstantSuggestion(const string16& suggestion,
                                           bool animate_to_complete) {
   parent_view_->SetInstantSuggestion(suggestion, animate_to_complete);
@@ -1010,8 +980,8 @@ int OmniboxViewWin::OnPerformDropImpl(const views::DropTargetEvent& event,
     string16 title;
     if (data.GetURLAndTitle(&url, &title)) {
       string16 text(StripJavascriptSchemas(UTF8ToUTF16(url.spec())));
-      SetUserText(text);
-      model()->AcceptInput(CURRENT_TAB, true);
+      OmniboxView::SetUserText(text);

[Peter Kasting, 2012/07/26 03:59:17]

Another case where I'm not sure why this is a superclass call.

[dominich, 2012/07/26 22:33:24]

see above. There shouldn't be a functional change here.

+      GetModel()->AcceptInput(CURRENT_TAB, true);
       return CopyOrLinkDragOperation(event.source_operations());
     }
   } else if (data.HasString()) {
@@ -2261,6 +2231,10 @@ LONG OmniboxViewWin::ClipXCoordToVisibleText(LONG x,
   return is_triple_click ? (right_bound - 1) : right_bound;
 }
 
+int OmniboxViewWin::GetOmniboxTextLength() const {
+  return static_cast<int>(GetTextLength());
+}
+
 void OmniboxViewWin::EmphasizeURLComponents() {
   ITextDocument* const text_object_model = GetTextObjectModel();
   ScopedFreeze freeze(this, text_object_model);
@@ -2457,8 +2431,7 @@ void OmniboxViewWin::DrawDropHighlight(HDC hdc,
 
 void OmniboxViewWin::TextChanged() {
   ScopedFreeze freeze(this, GetTextObjectModel());
-  EmphasizeURLComponents();
-  model_->OnChanged();
+  OmniboxView::TextChanged();
 }
 
 ITextDocument* OmniboxViewWin::GetTextObjectModel() const {
@@ -2509,8 +2482,8 @@ void OmniboxViewWin::StartDragIfNecessary(const CPoint& point) {
 
   // |sel| was set by GetSelection(), which preserves selection direction, so
   // sel.cpMin may not be the smaller value.
-  model()->AdjustTextForCopy(std::min(sel.cpMin, sel.cpMax), is_all_selected,
-                             &text_to_write, &url, &write_url);
+  GetModel()->AdjustTextForCopy(std::min(sel.cpMin, sel.cpMax), is_all_selected,
+                                &text_to_write, &url, &write_url);
 
   if (write_url) {
     string16 title;
@@ -2666,10 +2639,3 @@ int OmniboxViewWin::WidthNeededToDisplay(const string16& text) const {
   // PosFromChar(i) might return 0 when i is greater than 1.
   return font_.GetStringWidth(text) + GetHorizontalMargin();
 }
-
-bool OmniboxViewWin::IsCaretAtEnd() const {
-  long length = GetTextLength();
-  CHARRANGE sel;
-  GetSelection(sel);
-  return sel.cpMin == sel.cpMax && sel.cpMin == length;
-}