Componentize builtin_provider_unittest

chrome/browser/autocomplete/builtin_provider_unittest.cc

Index: chrome/browser/autocomplete/builtin_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/builtin_provider_unittest.cc b/chrome/browser/autocomplete/builtin_provider_unittest.cc
index 988c2c6341286317f8418259bd00226cf62e5fae..f84be4b3b3b0e341d997073abcdda46007e24246 100644
--- a/chrome/browser/autocomplete/builtin_provider_unittest.cc
+++ b/chrome/browser/autocomplete/builtin_provider_unittest.cc
@@ -11,20 +11,118 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
-#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h"
-#include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
-#include "chrome/common/url_constants.h"
-#include "chrome/test/base/testing_profile.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/autocomplete_provider.h"
+#include "components/omnibox/browser/mock_autocomplete_provider_client.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 using base::ASCIIToUTF16;
 
+namespace {
+const char kChromeUIChromeURLsURL[] = "chrome://chrome-urls/";
+const char kChromeUIVersionURL[] = "chrome://version/";
+const char kChromeUISettingsURL[] = "chrome://settings/";
+const char kChromeUIScheme[] = "chrome";
+const char kChromeUIMediaInternalsHost[] = "media-internals";
+const char kChromeUIMemoryHost[] = "memory";
+const char kChromeUIMemoryInternalsHost[] = "memory-internals";
+const char kChromeUIBlobInternalsHost[] = "blob-internals";
+const char kAutofillSubPage[] = "autofill";
+const char kClearBrowserDataSubPage[] = "clearBrowserData";
+const char kSearchEnginesSubPage[] = "searchEngines";
+const char kSyncSetupSubPage[] = "syncSetup";
+const char kChromeUISettingsHost[] = "settings";
+
+const char* const kChromeHostURLs[] = {
+    kChromeUIMemoryHost, kChromeUIMemoryInternalsHost,
+    kChromeUIBlobInternalsHost, kChromeUISettingsHost,
+};
+
+const size_t kNumberOfChromeHostURLs = arraysize(kChromeHostURLs);
+
+// This list should be kept in sync with chrome/common/url_constants.h.
+// Only include useful sub-pages, confirmation alerts are not useful.
+const char* const kChromeSettingsSubPages[] = {
+    kAutofillSubPage, kClearBrowserDataSubPage,
+    /*    chrome::kContentSettingsSubPage,
+        chrome::kContentSettingsExceptionsSubPage,
+        chrome::kImportDataSubPage,
+        chrome::kLanguageOptionsSubPage,
+        chrome::kPasswordManagerSubPage,
+        chrome::kResetProfileSettingsSubPage,*/
+    kSearchEnginesSubPage, kSyncSetupSubPage,
+    //#if defined(OS_CHROMEOS)
+    //    chrome::kInternetOptionsSubPage,
+    //#endif
+};

[droger, 2016/01/08 15:00:05]

If we want to componentize this test, then it should not need all these chrome
constants.

Then you need to define some arrays of strings, like:

const char* const kTestBuiltins[] = {
  "kBuiltinFoo",
  "kBuiltinBar",
}

const char* kTestUserTypeBuiltins[] {
  "kUserBuiltinFoo",
  "kUserBuiltinBar",
}

And change GetBuiltinURLs() and GetBuiltinsToProvideAsUserTypes() to just return
these.

Then you should update the tests to use these instead of the chrome:// URLs.
This might be a bit tedious since the tests make very heavy assumptions about
the values of the builtins (like how they share prefixes and such).

You can get rid of all the android specific code.

[Abhishek, 2016/01/12 19:59:29]

Thanks! I've made all the necessary changes.
I'm checking this part, if any help, that will be a great push.
Done.

+
+class TestingSchemeClassifier : public AutocompleteSchemeClassifier {
+ public:
+  TestingSchemeClassifier() {}
+
+  metrics::OmniboxInputType::Type GetInputTypeForScheme(
+      const std::string& scheme) const override {
+    return net::URLRequest::IsHandledProtocol(scheme)
+               ? metrics::OmniboxInputType::URL
+               : metrics::OmniboxInputType::INVALID;

[droger, 2016/01/08 15:00:05]

Is this really needed? Could we just always return
metrics::OmniboxInputType::URL?

[Abhishek, 2016/01/12 19:59:29]

Done.

+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestingSchemeClassifier);
+};
+
+class TestAutocompleteProviderClient : public MockAutocompleteProviderClient {
+ public:
+  TestAutocompleteProviderClient() {}
+
+  std::vector<base::string16> GetBuiltinURLs() override;
+  std::vector<base::string16> GetBuiltinsToProvideAsUserTypes() override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestAutocompleteProviderClient);
+};
+
+std::vector<base::string16> TestAutocompleteProviderClient::GetBuiltinURLs() {
+  LOG(INFO) << " abhishek " << __FUNCTION__;
+  // std::vector<base::string16> builtins;

[droger, 2016/01/08 15:00:05]

Cleanup comments and debug code.

[Abhishek, 2016/01/12 19:59:29]

Done.

+  std::vector<std::string> chrome_builtins(
+      kChromeHostURLs, kChromeHostURLs + kNumberOfChromeHostURLs);
+  std::sort(chrome_builtins.begin(), chrome_builtins.end());
+
+  std::vector<base::string16> builtins;
+
+  for (std::vector<std::string>::iterator i(chrome_builtins.begin());
+       i != chrome_builtins.end(); ++i)
+    builtins.push_back(base::ASCIIToUTF16(*i));
+
+#if !defined(OS_ANDROID)
+  base::string16 settings(base::ASCIIToUTF16(kChromeUISettingsHost) +
+                          base::ASCIIToUTF16("/"));
+  for (size_t i = 0; i < arraysize(kChromeSettingsSubPages); i++) {
+    builtins.push_back(settings +
+                       base::ASCIIToUTF16(kChromeSettingsSubPages[i]));
+  }
+#endif
+  return builtins;
+}
+
+std::vector<base::string16>
+TestAutocompleteProviderClient::GetBuiltinsToProvideAsUserTypes() {
+  std::vector<base::string16> builtins_to_provide;
+  builtins_to_provide.push_back(base::ASCIIToUTF16(kChromeUIChromeURLsURL));
+#if !defined(OS_ANDROID)
+  builtins_to_provide.push_back(base::ASCIIToUTF16(kChromeUISettingsURL));
+#endif
+  builtins_to_provide.push_back(base::ASCIIToUTF16(kChromeUIVersionURL));
+  return builtins_to_provide;
+}
+
+}  // namespace
+
 class BuiltinProviderTest : public testing::Test {
  protected:
   struct TestData {
@@ -37,8 +135,7 @@ class BuiltinProviderTest : public testing::Test {
   ~BuiltinProviderTest() override {}
 
   void SetUp() override {
-    profile_.reset(new TestingProfile());
-    client_.reset(new ChromeAutocompleteProviderClient(profile_.get()));
+    client_.reset(new TestAutocompleteProviderClient());
     provider_ = new BuiltinProvider(client_.get());
   }
   void TearDown() override { provider_ = NULL; }
@@ -51,7 +148,7 @@ class BuiltinProviderTest : public testing::Test {
       const AutocompleteInput input(
           cases[i].input, base::string16::npos, std::string(), GURL(),
           metrics::OmniboxEventProto::INVALID_SPEC, true, false, true, true,
-          false, ChromeAutocompleteSchemeClassifier(NULL));
+          false, TestingSchemeClassifier());
       provider_->Start(input, false);
       EXPECT_TRUE(provider_->done());
       matches = provider_->matches();
@@ -67,8 +164,7 @@ class BuiltinProviderTest : public testing::Test {
 
   content::TestBrowserThreadBundle thread_bundle_;
 
-  scoped_ptr<TestingProfile> profile_;
-  scoped_ptr<ChromeAutocompleteProviderClient> client_;
+  scoped_ptr<MockAutocompleteProviderClient> client_;
   scoped_refptr<BuiltinProvider> provider_;
 
  private:
@@ -78,16 +174,16 @@ class BuiltinProviderTest : public testing::Test {
 #if !defined(OS_ANDROID)
 TEST_F(BuiltinProviderTest, TypingScheme) {
   const base::string16 kAbout = ASCIIToUTF16(url::kAboutScheme);
-  const base::string16 kChrome = ASCIIToUTF16(content::kChromeUIScheme);
+  const base::string16 kChrome = ASCIIToUTF16(kChromeUIScheme);
   const base::string16 kSeparator1 = ASCIIToUTF16(":");
   const base::string16 kSeparator2 = ASCIIToUTF16(":/");
   const base::string16 kSeparator3 =
       ASCIIToUTF16(url::kStandardSchemeSeparator);
 
   // These default URLs should correspond with those in BuiltinProvider::Start.
-  const GURL kURL1 = GURL(chrome::kChromeUIChromeURLsURL);
-  const GURL kURL2 = GURL(chrome::kChromeUISettingsURL);
-  const GURL kURL3 = GURL(chrome::kChromeUIVersionURL);
+  const GURL kURL1 = GURL(kChromeUIChromeURLsURL);
+  const GURL kURL2 = GURL(kChromeUISettingsURL);
+  const GURL kURL3 = GURL(kChromeUIVersionURL);
 
   TestData typing_scheme_cases[] = {
     // Typing an unrelated scheme should give nothing.
@@ -125,15 +221,15 @@ TEST_F(BuiltinProviderTest, TypingScheme) {
 #else // Android uses a subset of the URLs
 TEST_F(BuiltinProviderTest, TypingScheme) {
   const base::string16 kAbout = ASCIIToUTF16(url::kAboutScheme);
-  const base::string16 kChrome = ASCIIToUTF16(content::kChromeUIScheme);
+  const base::string16 kChrome = ASCIIToUTF16(kChromeUIScheme);
   const base::string16 kSeparator1 = ASCIIToUTF16(":");
   const base::string16 kSeparator2 = ASCIIToUTF16(":/");
   const base::string16 kSeparator3 =
       ASCIIToUTF16(url::kStandardSchemeSeparator);
 
   // These default URLs should correspond with those in BuiltinProvider::Start.
-  const GURL kURL1 = GURL(chrome::kChromeUIChromeURLsURL);
-  const GURL kURL2 = GURL(chrome::kChromeUIVersionURL);
+  const GURL kURL1 = GURL(kChromeUIChromeURLsURL);
+  const GURL kURL2 = GURL(kChromeUIVersionURL);
 
   TestData typing_scheme_cases[] = {
     // Typing an unrelated scheme should give nothing.
@@ -190,7 +286,7 @@ TEST_F(BuiltinProviderTest, NonChromeURLs) {
 
 TEST_F(BuiltinProviderTest, ChromeURLs) {
   const base::string16 kAbout = ASCIIToUTF16(url::kAboutScheme);
-  const base::string16 kChrome = ASCIIToUTF16(content::kChromeUIScheme);
+  const base::string16 kChrome = ASCIIToUTF16(kChromeUIScheme);
   const base::string16 kSeparator1 = ASCIIToUTF16(":");
   const base::string16 kSeparator2 = ASCIIToUTF16(":/");
   const base::string16 kSeparator3 =
@@ -198,12 +294,9 @@ TEST_F(BuiltinProviderTest, ChromeURLs) {
 
   // This makes assumptions about the chrome URLs listed by the BuiltinProvider.
   // Currently they are derived from chrome::kChromeHostURLs[].
-  const base::string16 kHostM1 =
-      ASCIIToUTF16(content::kChromeUIMediaInternalsHost);
-  const base::string16 kHostM2 =
-      ASCIIToUTF16(chrome::kChromeUIMemoryHost);
-  const base::string16 kHostM3 =
-      ASCIIToUTF16(chrome::kChromeUIMemoryInternalsHost);
+  const base::string16 kHostM1 = ASCIIToUTF16(kChromeUIMediaInternalsHost);
+  const base::string16 kHostM2 = ASCIIToUTF16(kChromeUIMemoryHost);
+  const base::string16 kHostM3 = ASCIIToUTF16(kChromeUIMemoryInternalsHost);
   const GURL kURLM1 = GURL(kChrome + kSeparator3 + kHostM1);
   const GURL kURLM2 = GURL(kChrome + kSeparator3 + kHostM2);
   const GURL kURLM3 = GURL(kChrome + kSeparator3 + kHostM3);
@@ -243,7 +336,7 @@ TEST_F(BuiltinProviderTest, ChromeURLs) {
 
 TEST_F(BuiltinProviderTest, AboutBlank) {
   const base::string16 kAbout = ASCIIToUTF16(url::kAboutScheme);
-  const base::string16 kChrome = ASCIIToUTF16(content::kChromeUIScheme);
+  const base::string16 kChrome = ASCIIToUTF16(kChromeUIScheme);
   const base::string16 kAboutBlank = ASCIIToUTF16(url::kAboutBlankURL);
   const base::string16 kBlank = ASCIIToUTF16("blank");
   const base::string16 kSeparator1 =
@@ -251,8 +344,8 @@ TEST_F(BuiltinProviderTest, AboutBlank) {
   const base::string16 kSeparator2 = ASCIIToUTF16(":///");
   const base::string16 kSeparator3 = ASCIIToUTF16(";///");
 
-  const GURL kURLBlob = GURL(kChrome + kSeparator1 +
-                             ASCIIToUTF16(content::kChromeUIBlobInternalsHost));
+  const GURL kURLBlob =
+      GURL(kChrome + kSeparator1 + ASCIIToUTF16(kChromeUIBlobInternalsHost));
   const GURL kURLBlank = GURL(kAboutBlank);
 
   TestData about_blank_cases[] = {
@@ -300,7 +393,7 @@ TEST_F(BuiltinProviderTest, DoesNotSupportMatchesOnFocus) {
   const AutocompleteInput input(
       ASCIIToUTF16("chrome://s"), base::string16::npos, std::string(), GURL(),
       metrics::OmniboxEventProto::INVALID_SPEC, true, false, true, true, true,
-      ChromeAutocompleteSchemeClassifier(NULL));
+      TestingSchemeClassifier());
   provider_->Start(input, false);
    EXPECT_TRUE(provider_->matches().empty());
 }
@@ -310,14 +403,13 @@ TEST_F(BuiltinProviderTest, DoesNotSupportMatchesOnFocus) {
 TEST_F(BuiltinProviderTest, ChromeSettingsSubpages) {
   // This makes assumptions about the chrome URLs listed by the BuiltinProvider.
   // Currently they are derived from chrome::kChromeHostURLs[].
-  const base::string16 kSettings = ASCIIToUTF16(chrome::kChromeUISettingsURL);
-  const base::string16 kDefaultPage1 = ASCIIToUTF16(chrome::kAutofillSubPage);
-  const base::string16 kDefaultPage2 =
-      ASCIIToUTF16(chrome::kClearBrowserDataSubPage);
+  const base::string16 kSettings = ASCIIToUTF16(kChromeUISettingsURL);
+  const base::string16 kDefaultPage1 = ASCIIToUTF16(kAutofillSubPage);
+  const base::string16 kDefaultPage2 = ASCIIToUTF16(kClearBrowserDataSubPage);
   const GURL kDefaultURL1 = GURL(kSettings + kDefaultPage1);
   const GURL kDefaultURL2 = GURL(kSettings + kDefaultPage2);
-  const base::string16 kPage1 = ASCIIToUTF16(chrome::kSearchEnginesSubPage);
-  const base::string16 kPage2 = ASCIIToUTF16(chrome::kSyncSetupSubPage);
+  const base::string16 kPage1 = ASCIIToUTF16(kSearchEnginesSubPage);
+  const base::string16 kPage2 = ASCIIToUTF16(kSyncSetupSubPage);
   const GURL kURL1 = GURL(kSettings + kPage1);
   const GURL kURL2 = GURL(kSettings + kPage2);