Misc. omnibox cleanup:

components/omnibox/browser/omnibox_edit_unittest.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 <stddef.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #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"
 #include "components/omnibox/browser/omnibox_edit_model.h"
 #include "components/omnibox/browser/omnibox_view.h"
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/search_engines/template_url_service_client.h"
 #include "components/sessions/core/session_id.h"
 #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 {
  public:
   explicit TestingOmniboxView(OmniboxEditController* controller)
       : OmniboxView(controller, nullptr) {}
 
   // OmniboxView:
   void Update() override {}
   void OpenMatch(const AutocompleteMatch& match,
@@ skipping 34 matching lines @@
     return text_changed;
   }
   void OnInlineAutocompleteTextCleared() override {
     inline_autocomplete_text_.clear();
   }
   void OnRevertTemporaryText() override {}
   void OnBeforePossibleChange() override {}
   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();
   }
   int GetTextWidth() const override { return 0; }
   int GetWidth() const override { return 0; }
   bool IsImeComposing() const override { return false; }
   int GetOmniboxTextLength() const override { return 0; }
   void EmphasizeURLComponents() override {}
 
@@ skipping 144 matching lines @@
           nullptr, base::Closure()));
   provider_client->set_template_url_service(std::move(template_url_service));
 
   return std::move(provider_client);
 }
 
 }  // namespace
 
 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.
 TEST_F(OmniboxEditTest, AdjustTextForCopy) {
   struct Data {
     const char* perm_text;
     const int sel_start;
     const bool is_all_selected;
     const char* input;
     const char* expected_output;
@@ skipping 36 matching lines @@
     // Makes sure intranet urls get 'http://' prefixed to them.
     { "b/foo", 0, true, "b/foo", "http://b/foo", true, "http://b/foo", false },
 
     // Verifies a search term 'foo' doesn't end up with http.
     { "www.google.com/search?", 0, false, "foo", "foo", false, "", false },
 
     // Makes sure extracted search terms are not modified.
     { "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;
   }
 }
 
 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(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());
 
-  // Test if the model updates the inline autocomplete text in the view.
-  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(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.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.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.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());
+  model()->AcceptTemporaryTextAsUserText();
+  EXPECT_EQ(base::ASCIIToUTF16("hello"), view().GetText());
+  EXPECT_EQ(base::string16(), view().inline_autocomplete_text());
 }