Misc. omnibox cleanup:

components/omnibox/browser/omnibox_edit_unittest.cc

Index: components/omnibox/browser/omnibox_edit_unittest.cc
diff --git a/components/omnibox/browser/omnibox_edit_unittest.cc b/components/omnibox/browser/omnibox_edit_unittest.cc
index f07cb49552ea479e71242c2bf3a4843c78e5921e..9b1158c6736a0ee5e99527b70a24c0a10ac1c487 100644
--- a/components/omnibox/browser/omnibox_edit_unittest.cc
+++ b/components/omnibox/browser/omnibox_edit_unittest.cc
@@ -11,6 +11,7 @@
 #include "components/omnibox/browser/autocomplete_classifier.h"
 #include "components/omnibox/browser/autocomplete_controller.h"
 #include "components/omnibox/browser/autocomplete_scheme_classifier.h"
+#include "components/omnibox/browser/history_url_provider.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "components/omnibox/browser/omnibox_client.h"
 #include "components/omnibox/browser/omnibox_edit_controller.h"
@@ -23,9 +24,6 @@
 #include "components/toolbar/test_toolbar_model.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using base::ASCIIToUTF16;
-using base::UTF8ToUTF16;
-
 namespace {
 
 class TestingOmniboxView : public OmniboxView {
@@ -80,8 +78,8 @@ class TestingOmniboxView : public OmniboxView {
   bool OnAfterPossibleChange(bool allow_keyword_ui_change) override {
     return false;
   }
-  gfx::NativeView GetNativeView() const override { return NULL; }
-  gfx::NativeView GetRelativeWindowForPopup() const override { return NULL; }
+  gfx::NativeView GetNativeView() const override { return nullptr; }
+  gfx::NativeView GetRelativeWindowForPopup() const override { return nullptr; }
   void SetGrayTextAutocompletion(const base::string16& input) override {}
   base::string16 GetGrayTextAutocompletion() const override {
     return base::string16();
@@ -246,11 +244,24 @@ TestingOmniboxClient::CreateAutocompleteProviderClient() {
 
 class OmniboxEditTest : public ::testing::Test {
  public:
+  OmniboxEditTest()
+      : controller_(&toolbar_model_),
+        view_(&controller_),
+        model_(&view_, &controller_, base::MakeUnique<TestingOmniboxClient>()) {
+  }
+
   TestToolbarModel* toolbar_model() { return &toolbar_model_; }
+  const TestingOmniboxView& view() { return view_; }

[Mark P, 2016/06/08 21:41:40]

nit: It looks a little weird that you've chosen to return a const reference here
rather than a regular pointer as above and below.  Any particular reason other
than "that's all we need"?

[Peter Kasting, 2016/06/08 21:46:42]

I thought about returning a const pointer but it seemed like we normally use
const refs in preference when they would work (i.e. when the return value cannot
be null).

I didn't want to return something non-const, since the caller only needs
something const.

+  OmniboxEditModel* model() { return &model_; }
 
  private:
   base::MessageLoop message_loop_;
   TestToolbarModel toolbar_model_;
+  TestingOmniboxEditController controller_;
+  TestingOmniboxView view_;
+  OmniboxEditModel model_;
+
+  DISALLOW_COPY_AND_ASSIGN(OmniboxEditTest);
 };
 
 // Tests various permutations of AutocompleteModel::AdjustTextForCopy.
@@ -307,24 +318,21 @@ TEST_F(OmniboxEditTest, AdjustTextForCopy) {
     { "www.google.com/webhp?", 0, true, "hello world", "hello world", false,
       "", true },
   };
-  TestingOmniboxEditController controller(toolbar_model());
-  TestingOmniboxView view(&controller);
-  OmniboxEditModel model(&view, &controller,
-                         base::WrapUnique(new TestingOmniboxClient()));
 
   for (size_t i = 0; i < arraysize(input); ++i) {
-    toolbar_model()->set_text(ASCIIToUTF16(input[i].perm_text));
-    model.UpdatePermanentText();
+    toolbar_model()->set_text(base::ASCIIToUTF16(input[i].perm_text));
+    model()->UpdatePermanentText();
 
     toolbar_model()->set_perform_search_term_replacement(
         input[i].extracted_search_terms);
 
-    base::string16 result = ASCIIToUTF16(input[i].input);
+    base::string16 result = base::ASCIIToUTF16(input[i].input);
     GURL url;
     bool write_url;
-    model.AdjustTextForCopy(input[i].sel_start, input[i].is_all_selected,
-                            &result, &url, &write_url);
-    EXPECT_EQ(ASCIIToUTF16(input[i].expected_output), result) << "@: " << i;
+    model()->AdjustTextForCopy(input[i].sel_start, input[i].is_all_selected,
+                               &result, &url, &write_url);
+    EXPECT_EQ(base::ASCIIToUTF16(input[i].expected_output), result) << "@: "
+                                                                    << i;
     EXPECT_EQ(input[i].write_url, write_url) << " @" << i;
     if (write_url)
       EXPECT_EQ(input[i].expected_url, url.spec()) << " @" << i;
@@ -332,35 +340,34 @@ TEST_F(OmniboxEditTest, AdjustTextForCopy) {
 }
 
 TEST_F(OmniboxEditTest, InlineAutocompleteText) {
-  TestingOmniboxEditController controller(toolbar_model());
-  TestingOmniboxView view(&controller);
-  OmniboxEditModel model(&view, &controller,
-                         base::WrapUnique(new TestingOmniboxClient()));
-
   // Test if the model updates the inline autocomplete text in the view.

[Peter Kasting, 2016/06/08 01:11:20]

It's hard to see in this diff, but the only changes here are to convert "UTF8"
to "base::ASCII" and convert references to now-removed locals to use accessors.

-  EXPECT_EQ(base::string16(), view.inline_autocomplete_text());
-  model.SetUserText(UTF8ToUTF16("he"));
-  model.OnPopupDataChanged(UTF8ToUTF16("llo"), NULL, base::string16(), false);
-  EXPECT_EQ(UTF8ToUTF16("hello"), view.GetText());
-  EXPECT_EQ(UTF8ToUTF16("llo"), view.inline_autocomplete_text());
-
-  model.OnAfterPossibleChange(UTF8ToUTF16("he"), UTF8ToUTF16("hel"), 3, 3,
-                              false, true, false, true);
-  EXPECT_EQ(base::string16(), view.inline_autocomplete_text());
-  model.OnPopupDataChanged(UTF8ToUTF16("lo"), NULL, base::string16(), false);
-  EXPECT_EQ(UTF8ToUTF16("hello"), view.GetText());
-  EXPECT_EQ(UTF8ToUTF16("lo"), view.inline_autocomplete_text());
-
-  model.Revert();
-  EXPECT_EQ(base::string16(), view.GetText());
-  EXPECT_EQ(base::string16(), view.inline_autocomplete_text());
-
-  model.SetUserText(UTF8ToUTF16("he"));
-  model.OnPopupDataChanged(UTF8ToUTF16("llo"), NULL, base::string16(), false);
-  EXPECT_EQ(UTF8ToUTF16("hello"), view.GetText());
-  EXPECT_EQ(UTF8ToUTF16("llo"), view.inline_autocomplete_text());
-
-  model.AcceptTemporaryTextAsUserText();
-  EXPECT_EQ(UTF8ToUTF16("hello"), view.GetText());
-  EXPECT_EQ(base::string16(), view.inline_autocomplete_text());
+  EXPECT_EQ(base::string16(), view().inline_autocomplete_text());
+  model()->SetUserText(base::ASCIIToUTF16("he"));
+  model()->OnPopupDataChanged(base::ASCIIToUTF16("llo"), nullptr,
+                              base::string16(), false);
+  EXPECT_EQ(base::ASCIIToUTF16("hello"), view().GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("llo"), view().inline_autocomplete_text());
+
+  model()->OnAfterPossibleChange(base::ASCIIToUTF16("he"),
+                                 base::ASCIIToUTF16("hel"), 3, 3, false, true,
+                                 false, true);
+  EXPECT_EQ(base::string16(), view().inline_autocomplete_text());
+  model()->OnPopupDataChanged(base::ASCIIToUTF16("lo"), nullptr,
+                              base::string16(), false);
+  EXPECT_EQ(base::ASCIIToUTF16("hello"), view().GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("lo"), view().inline_autocomplete_text());
+
+  model()->Revert();
+  EXPECT_EQ(base::string16(), view().GetText());
+  EXPECT_EQ(base::string16(), view().inline_autocomplete_text());
+
+  model()->SetUserText(base::ASCIIToUTF16("he"));
+  model()->OnPopupDataChanged(base::ASCIIToUTF16("llo"), nullptr,
+                              base::string16(), false);
+  EXPECT_EQ(base::ASCIIToUTF16("hello"), view().GetText());
+  EXPECT_EQ(base::ASCIIToUTF16("llo"), view().inline_autocomplete_text());
+
+  model()->AcceptTemporaryTextAsUserText();
+  EXPECT_EQ(base::ASCIIToUTF16("hello"), view().GetText());
+  EXPECT_EQ(base::string16(), view().inline_autocomplete_text());
 }