Moving common code into OmniboxView from OmniboxView*

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

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 
 #include "base/logging.h"
 #include "base/property_bag.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ skipping 206 matching lines @@
 
 // static
 const char OmniboxViewViews::kViewClassName[] = "BrowserOmniboxViewViews";
 
 OmniboxViewViews::OmniboxViewViews(OmniboxEditController* controller,
                                    ToolbarModel* toolbar_model,
                                    Profile* profile,
                                    CommandUpdater* command_updater,
                                    bool popup_window_mode,
                                    LocationBarView* location_bar)
-    : textfield_(NULL),
+    : OmniboxView(profile, controller, toolbar_model, command_updater),
+      textfield_(NULL),
       popup_window_mode_(popup_window_mode),
-      model_(new OmniboxEditModel(this, controller, profile)),
-      controller_(controller),
-      toolbar_model_(toolbar_model),
-      command_updater_(command_updater),
       security_level_(ToolbarModel::NONE),
       ime_composing_before_change_(false),
       delete_at_end_pressed_(false),
       location_bar_view_(location_bar),
       ime_candidate_window_open_(false),
       select_all_on_mouse_release_(false) {
   if (chrome::search::IsInstantExtendedAPIEnabled(
           location_bar_view_->profile())) {
     set_background(views::Background::CreateSolidBackground(
         chrome::search::kOmniboxBackgroundColor));
   }
 }
 
 OmniboxViewViews::~OmniboxViewViews() {
 #if defined(OS_CHROMEOS)
   chromeos::input_method::InputMethodManager::GetInstance()->
       RemoveCandidateWindowObserver(this);
 #endif
 
   // Explicitly teardown members which have a reference to us.  Just to be safe
   // we want them to be destroyed before destroying any other internal state.
   popup_view_.reset();
   model_.reset();

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

Sort of worries me that we had this code in the first place when the comment
doesn't make it clear what it's for -- and that we're no longer doing half of it
if it mattered.

Can you spelunk this a bit and figure out if we can just nuke the whole block?

[dominich, 2012/07/30 16:42:59]

What are we no longer doing? The model_.reset() call has been moved into the
base destructor.

The comment is clear to me: We pass 'this' to the model when we initialize it so
when we're going away we want to make sure we destroy the model_. Then anyone
accessing it can't get hold of a stale 'this'.

Unless they've cached it, of course.

[Peter Kasting, 2012/07/30 17:06:03]

What I mean is that the bit about "reset these before we tear down any internal
state" is no longer fully true -- with the moved call to model_.reset(), we now
tear down all subclass members before we reset the model in the base class
destructor.  Don't know if that matters or not.

If it doesn't matter, then we can remove the explicit calls and let things be
cleaned up implicitly.  But if it does then this is problematic.

 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxViewViews public:
 
 void OmniboxViewViews::Init(views::View* popup_parent_view) {
   // 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_);
@@ skipping 195 matching lines @@
 }
 
 void OmniboxViewViews::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   if (popup_view_->IsOpen())
     popup_view_->UpdatePopupAppearance();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxViewViews, AutocopmleteEditView implementation:
 
-OmniboxEditModel* OmniboxViewViews::model() {
-  return model_.get();
-}
-
-const OmniboxEditModel* OmniboxViewViews::model() const {
-  return model_.get();
-}
-
 void OmniboxViewViews::SaveStateToTab(WebContents* tab) {
   DCHECK(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_);
   }
@@ skipping 33 matching lines @@
       // TODO(oshima): consider saving/restoring edit history.
       textfield_->ClearEditHistory();
     }
   } else if (visibly_changed_permanent_text) {
     RevertAll();
   } else if (changed_security_level) {
     EmphasizeURLComponents();
   }
 }
 
-void OmniboxViewViews::OpenMatch(const AutocompleteMatch& match,
-                                 WindowOpenDisposition disposition,
-                                 const GURL& alternate_nav_url,
-                                 size_t selected_line) {
-  if (!match.destination_url.is_valid())
-    return;
-
-  model_->OpenMatch(match, disposition, alternate_nav_url, selected_line);
-}
-
 string16 OmniboxViewViews::GetText() const {
   // TODO(oshima): IME support
   return textfield_->text();
 }
 
-bool OmniboxViewViews::IsEditingOrEmpty() const {
-  return model_->user_input_in_progress() || (GetTextLength() == 0);
-}
-
-int OmniboxViewViews::GetIcon() const {
-  return IsEditingOrEmpty() ?
-      AutocompleteMatch::TypeToIcon(model_->CurrentTextType()) :
-      toolbar_model_->GetIcon();
-}
-
-void OmniboxViewViews::SetUserText(const string16& text) {
-  SetUserText(text, text, true);
-}
-
-void OmniboxViewViews::SetUserText(const string16& text,
-                                   const string16& display_text,
-                                   bool update_popup) {
-  model_->SetUserText(text);
-  SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup,
-      true);
-}
-
 void OmniboxViewViews::SetWindowTextAndCaretPos(const string16& text,
                                                 size_t caret_pos,
                                                 bool update_popup,
                                                 bool notify_text_changed) {
   const ui::Range range(caret_pos, caret_pos);
   SetTextAndSelectedRange(text, range);
 
   if (update_popup)
     UpdatePopup();
 
@@ skipping 31 matching lines @@
   } else {
     *start = static_cast<size_t>(range.end());
     *end = static_cast<size_t>(range.start());
   }
 }
 
 void OmniboxViewViews::SelectAll(bool reversed) {
   textfield_->SelectAll(reversed);
 }
 
-void OmniboxViewViews::RevertAll() {
-  ClosePopup();
-  model_->Revert();
-  TextChanged();
-}
-
 void OmniboxViewViews::UpdatePopup() {
   model_->SetInputInProgress(true);
   if (ime_candidate_window_open_)
     return;
   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.
   ui::Range sel;
   textfield_->GetSelectedRange(&sel);
   bool no_inline_autocomplete =
       sel.GetMax() < GetTextLength() || textfield_->IsIMEComposing();
 
   model_->StartAutocomplete(!sel.is_empty(), no_inline_autocomplete);
 }
 
-void OmniboxViewViews::ClosePopup() {
-  model_->StopAutocomplete();
-}
-
 void OmniboxViewViews::SetFocus() {
   // In views-implementation, the focus is on textfield rather than OmniboxView.
   textfield_->RequestFocus();
 }
 
 void OmniboxViewViews::OnTemporaryTextMaybeChanged(
     const string16& display_text,
     bool save_original_selection) {
   if (save_original_selection)
     textfield_->GetSelectedRange(&saved_temporary_selection_);
@@ skipping 65 matching lines @@
 }
 
 gfx::NativeView OmniboxViewViews::GetNativeView() const {
   return GetWidget()->GetNativeView();
 }
 
 gfx::NativeView OmniboxViewViews::GetRelativeWindowForPopup() const {
   return GetWidget()->GetTopLevelWidget()->GetNativeView();
 }
 
-CommandUpdater* OmniboxViewViews::GetCommandUpdater() {
-  return command_updater_;
-}
-
 void OmniboxViewViews::SetInstantSuggestion(const string16& input,
                                             bool animate_to_complete) {
 #if defined(OS_WIN) || defined(USE_AURA)
   location_bar_view_->SetInstantSuggestion(input, animate_to_complete);
 #endif
 }
 
 string16 OmniboxViewViews::GetInstantSuggestion() const {
 #if defined(OS_WIN) || defined(USE_AURA)
   return location_bar_view_->GetInstantSuggestion();
@@ skipping 159 matching lines @@
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////
 // 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());
+}
+
 void OmniboxViewViews::EmphasizeURLComponents() {
   // See whether the contents are a URL with a non-empty host portion, which we
   // should emphasize.  To check for a URL, rather than using the type returned
   // by Parse(), ask the model, which will check the desired page transition for
   // 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(),
@@ skipping 15 matching lines @@
   if (!model_->user_input_in_progress() && scheme.is_nonempty() &&
       (security_level_ != ToolbarModel::NONE)) {
     SkColor security_color = LocationBarView::GetColor(
         security_level_, LocationBarView::SECURITY_TEXT);
     bool use_strikethrough = (security_level_ == ToolbarModel::SECURITY_ERROR);
     ApplyURLStyle(textfield_, scheme.begin, scheme.end(),
                   security_color, use_strikethrough);
   }
 }
 
-void OmniboxViewViews::TextChanged() {
-  EmphasizeURLComponents();
-  model_->OnChanged();
-}
-
 void OmniboxViewViews::SetTextAndSelectedRange(const string16& text,
                                                const ui::Range& range) {
   if (text != GetText())
     textfield_->SetText(text);
   textfield_->SelectRange(range);
 }
 
 string16 OmniboxViewViews::GetSelectedText() const {
   // TODO(oshima): Support instant, IME.
   return textfield_->GetSelectedText();
 }