Moving common code into OmniboxView from OmniboxView*

chrome/browser/ui/omnibox/omnibox_view.cc

Index: chrome/browser/ui/omnibox/omnibox_view.cc
diff --git a/chrome/browser/ui/omnibox/omnibox_view.cc b/chrome/browser/ui/omnibox/omnibox_view.cc
index a7ead88ae454aec011e2e2f6da1929650fffe828..a1f6d7f496d0a67d0241bebd8b49f3da7e99040d 100644
--- a/chrome/browser/ui/omnibox/omnibox_view.cc
+++ b/chrome/browser/ui/omnibox/omnibox_view.cc
@@ -10,9 +10,11 @@
 #include "base/string_util.h"
 #include "base/string16.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/browser_process.h"
 #include "ui/base/clipboard/clipboard.h"
 
+// static
 string16 OmniboxView::StripJavascriptSchemas(const string16& text) {
   const string16 kJsPrefix(ASCIIToUTF16(chrome::kJavaScriptScheme) +
                            ASCIIToUTF16(":"));
@@ -66,3 +68,80 @@ string16 OmniboxView::GetClipboardText() {
 
   return string16();
 }
+
+OmniboxView::~OmniboxView() {
+}
+
+void OmniboxView::OpenMatch(const AutocompleteMatch& match,
+                            WindowOpenDisposition disposition,
+                            const GURL& alternate_nav_url,
+                            size_t selected_line) {
+  // Invalid URLs such as chrome://history can end up here.

[Peter Kasting, 2012/08/07 02:47:05]

Oh?  I thought this didn't happen?  Did you find a repro case?

[dominich, 2012/08/07 17:48:20]

Yes - typing chrome://history :)

+  if (!match.destination_url.is_valid())
+    return;
+  // |model_| can be NULL in tests.

[Peter Kasting, 2012/08/07 02:47:05]

Nit: Instead of putting these comments in every function, put one comment on the
class members' declaration.

[dominich, 2012/08/07 17:48:20]

Done.

+  if (model_.get())
+    model_->OpenMatch(match, disposition, alternate_nav_url, selected_line);
+}
+
+bool OmniboxView::IsEditingOrEmpty() const {
+  // |model_| can be NULL in tests.
+  return (model_.get() && model_->user_input_in_progress()) ||
+      (GetOmniboxTextLength() == 0);
+}
+
+int OmniboxView::GetIcon() const {
+  // |model_| can be NULL in tests.
+  return IsEditingOrEmpty() ?

[Peter Kasting, 2012/08/07 02:47:05]

Nit: Don't nest ?:s

[dominich, 2012/08/07 17:48:20]

Done.

+      AutocompleteMatch::TypeToIcon(model_.get() ?
+          model_->CurrentTextType() : AutocompleteMatch::URL_WHAT_YOU_TYPED) :
+      toolbar_model_->GetIcon();
+}
+
+void OmniboxView::SetUserText(const string16& text) {
+  SetUserText(text, text, true);
+}
+
+void OmniboxView::SetUserText(const string16& text,
+                              const string16& display_text,
+                              bool update_popup) {
+  // |model_| can be NULL in tests.
+  if (model_.get())
+    model_->SetUserText(text);
+  // TODO(deanm): something about selection / focus change here.

[Peter Kasting, 2012/08/07 02:47:05]

Nit: I thought you were going to kill this comment?

[dominich, 2012/08/07 17:48:20]

Done.

+  SetWindowTextAndCaretPos(display_text, display_text.length(), update_popup,
+                           true);
+}
+
+void OmniboxView::RevertAll() {
+  CloseAutocompletePopup();
+  // |model_| can be NULL in tests.
+  if (model_.get())
+    model_->Revert();
+  TextChanged();
+}
+
+void OmniboxView::CloseAutocompletePopup() {
+  // |model_| can be NULL in tests.
+  if (model_.get())
+    model_->StopAutocomplete();
+}
+
+OmniboxView::OmniboxView(Profile* profile,
+                         OmniboxEditController* controller,
+                         ToolbarModel* toolbar_model,
+                         CommandUpdater* command_updater)
+    : controller_(controller),
+      toolbar_model_(toolbar_model),
+      command_updater_(command_updater) {
+  // |profile| can be NULL in tests.
+  if (profile)
+    model_.reset(new OmniboxEditModel(this, controller, profile));
+}
+
+void OmniboxView::TextChanged() {
+  EmphasizeURLComponents();
+  // |model_| can be NULL in tests.
+  if (model_.get())
+    model_->OnChanged();
+}