Remove search::IsQueryExtractionEnabled

chrome/browser/ui/toolbar/toolbar_model_unittest.cc

Index: chrome/browser/ui/toolbar/toolbar_model_unittest.cc
diff --git a/chrome/browser/ui/toolbar/toolbar_model_unittest.cc b/chrome/browser/ui/toolbar/toolbar_model_unittest.cc
index c22d5bc8722e925a296dc0eaada7b89349f09621..eb6f39fc35d969833606b62d2451695ee2f75fa2 100644
--- a/chrome/browser/ui/toolbar/toolbar_model_unittest.cc
+++ b/chrome/browser/ui/toolbar/toolbar_model_unittest.cc
@@ -8,22 +8,16 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
-#include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
-#include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
-#include "components/google/core/browser/google_switches.h"
-#include "components/search/search.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/toolbar/toolbar_model.h"
-#include "components/variations/entropy_provider.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/ssl_status.h"
@@ -36,105 +30,52 @@ namespace {
 
 struct TestItem {
   GURL url;
-  // The expected text to display when both forms of URL replacement are
-  // inactive.
-  base::string16 expected_text_url_replacement_inactive;
-  // The expected text to display when query extraction is active.
-  base::string16 expected_text_query_extraction;
-  // The expected text to display when both query extraction and URL removal are
-  // active.
-  base::string16 expected_text_both;
-  bool would_perform_search_term_replacement;
-  bool should_display_url;
+  base::string16 expected_text;
 } test_items[] = {
   {
     GURL("view-source:http://www.google.com"),
-    base::ASCIIToUTF16("view-source:www.google.com"),
-    base::ASCIIToUTF16("view-source:www.google.com"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("view-source:www.google.com")
   },
   {
     GURL("view-source:chrome://newtab/"),
-    base::ASCIIToUTF16("view-source:chrome://newtab"),
-    base::ASCIIToUTF16("view-source:chrome://newtab"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("view-source:chrome://newtab")
   },
   {
     GURL("chrome-extension://monkey/balls.html"),
-    base::ASCIIToUTF16("chrome-extension://monkey/balls.html"),
-    base::ASCIIToUTF16("chrome-extension://monkey/balls.html"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("chrome-extension://monkey/balls.html")
   },
   {
     GURL(url::kAboutBlankURL),
-    base::ASCIIToUTF16(url::kAboutBlankURL),
-    base::ASCIIToUTF16(url::kAboutBlankURL),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16(url::kAboutBlankURL)
   },
   {
     GURL("http://searchurl/?q=tractor+supply"),
-    base::ASCIIToUTF16("searchurl/?q=tractor+supply"),
-    base::ASCIIToUTF16("searchurl/?q=tractor+supply"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("searchurl/?q=tractor+supply")
   },
   {
     GURL("http://google.com/search?q=tractor+supply&espv=1"),
-    base::ASCIIToUTF16("google.com/search?q=tractor+supply&espv=1"),
-    base::ASCIIToUTF16("google.com/search?q=tractor+supply&espv=1"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("google.com/search?q=tractor+supply&espv=1")
   },
   {
     GURL("https://google.ca/search?q=tractor+supply"),
-    base::ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
-    base::ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("https://google.ca/search?q=tractor+supply")
   },
   {
     GURL("https://google.com/search?q=tractor+supply"),
-    base::ASCIIToUTF16("https://google.com/search?q=tractor+supply"),
-    base::ASCIIToUTF16("https://google.com/search?q=tractor+supply"),
-    base::string16(),
-    false,
-    true
+    base::ASCIIToUTF16("https://google.com/search?q=tractor+supply")
   },
   {
     GURL("https://google.com/search?q=tractor+supply&espv=1"),
-    base::ASCIIToUTF16("https://google.com/search?q=tractor+supply&espv=1"),
-    base::ASCIIToUTF16("tractor supply"),
-    base::ASCIIToUTF16("tractor supply"),
-    true,
-    true
+    base::ASCIIToUTF16("https://google.com/search?q=tractor+supply&espv=1")
   },
   {
     GURL("https://google.com/search?q=tractorsupply.com&espv=1"),
-    base::ASCIIToUTF16("https://google.com/search?q=tractorsupply.com&espv=1"),
-    base::ASCIIToUTF16("tractorsupply.com"),
-    base::ASCIIToUTF16("tractorsupply.com"),
-    true,
-    true
+    base::ASCIIToUTF16("https://google.com/search?q=tractorsupply.com&espv=1")
   },
   {
     GURL("https://google.com/search?q=ftp://tractorsupply.ie&espv=1"),
     base::ASCIIToUTF16(
-        "https://google.com/search?q=ftp://tractorsupply.ie&espv=1"),
-    base::ASCIIToUTF16("ftp://tractorsupply.ie"),
-    base::ASCIIToUTF16("ftp://tractorsupply.ie"),
-    true,
-    true
+        "https://google.com/search?q=ftp://tractorsupply.ie&espv=1")
   },
 };
 
@@ -154,14 +95,10 @@ class ToolbarModelTest : public BrowserWithTestWindowTest {
 
  protected:
   void NavigateAndCheckText(const GURL& url,
-                            const base::string16& expected_text,
-                            bool would_perform_search_term_replacement,
-                            bool should_display_url);
+                            const base::string16& expected_text);
   void NavigateAndCheckElided(const GURL& https_url);
 
  private:
-  std::unique_ptr<base::FieldTrialList> field_trial_list_;
-
   DISALLOW_COPY_AND_ASSIGN(ToolbarModelTest);
 };
 
@@ -185,9 +122,7 @@ void ToolbarModelTest::SetUp() {
 
 void ToolbarModelTest::NavigateAndCheckText(
     const GURL& url,
-    const base::string16& expected_text,
-    bool would_perform_search_term_replacement,
-    bool should_display_url) {
+    const base::string16& expected_text) {
   // Check while loading.
   content::NavigationController* controller =
       &browser()->tab_strip_model()->GetWebContentsAt(0)->GetController();
@@ -195,9 +130,8 @@ void ToolbarModelTest::NavigateAndCheckText(
                       std::string());
   ToolbarModel* toolbar_model = browser()->toolbar_model();
   EXPECT_EQ(expected_text, toolbar_model->GetText());
-  EXPECT_EQ(would_perform_search_term_replacement,
-            toolbar_model->WouldPerformSearchTermReplacement(false));
-  EXPECT_EQ(should_display_url, toolbar_model->ShouldDisplayURL());
+  EXPECT_FALSE(toolbar_model->WouldPerformSearchTermReplacement(false));
+  EXPECT_TRUE(toolbar_model->ShouldDisplayURL());
 
   // Check after commit.
   CommitPendingLoad(controller);
@@ -208,15 +142,13 @@ void ToolbarModelTest::NavigateAndCheckText(
         content::SECURITY_STYLE_AUTHENTICATED;
   }
   EXPECT_EQ(expected_text, toolbar_model->GetText());
-  EXPECT_EQ(would_perform_search_term_replacement,
-            toolbar_model->WouldPerformSearchTermReplacement(false));
-  EXPECT_EQ(should_display_url, toolbar_model->ShouldDisplayURL());
+  EXPECT_FALSE(toolbar_model->WouldPerformSearchTermReplacement(false));
+  EXPECT_TRUE(toolbar_model->ShouldDisplayURL());
 
   // Now pretend the user started modifying the omnibox.
   toolbar_model->set_input_in_progress(true);
   EXPECT_FALSE(toolbar_model->WouldPerformSearchTermReplacement(false));
-  EXPECT_EQ(would_perform_search_term_replacement,
-            toolbar_model->WouldPerformSearchTermReplacement(true));
+  EXPECT_FALSE(toolbar_model->WouldPerformSearchTermReplacement(true));
 
   // Tell the ToolbarModel that the user has stopped editing.  This prevents
   // this function from having side effects.
@@ -260,100 +192,15 @@ PopupToolbarModelTest::~PopupToolbarModelTest() {
 
 // Actual tests ---------------------------------------------------------------
 
-// Test that we only replace URLs when query extraction and URL replacement
-// are enabled.
-TEST_F(ToolbarModelTest, ShouldDisplayURL_QueryExtraction) {
+// Test URL display.
+TEST_F(ToolbarModelTest, ShouldDisplayURL) {

[Marc Treib, 2016/08/09 10:23:43]

I'm not sure how much value this test still has with all the query extraction
parts removed. It still tests that URLs are displayed properly, but now the only
special case is that "http://" gets removed.

[Peter Kasting, 2016/08/09 20:55:13]

I would keep the file, but I'd rip out PopupToolbarModelTest entirely, remove
the last four test item entries, remove the TemplateURLServiceFactory an
SetGoogleBaseURL() stuff from SetUp(), remove the last couple blocks of
NavigateAndCheckText() that toggle whether editing is in progress, remove the
bits about faking a secure connection, see if NavigateAndCheckText() and
NavigateAndCheckElided() can share code somehow, etc.

[Marc Treib, 2016/08/10 12:53:40]

Done, mostly (except for sharing code between the two NavigateAnd... methods -
they could potentially share the first few lines, but IMO not worth it).

Out of curiosity: What was the purpose of the PopupToolbarModelTest? The only
difference to ToolbarModelTest was the "Browser::TYPE_POPUP" part; not sure what
that means or why it would make any difference for the test?

[Peter Kasting, 2016/08/20 02:21:19]

The location bar works differently in popup windows (e.g. is not editable).  The
test verified none of those differences affected this sort of case.  That
distinction isn't meaningful enough, with most of this file gone, to stick
around.

   AddTab(browser(), GURL(url::kAboutBlankURL));
 
-  // Before we enable instant extended, query extraction is disabled.
-  EXPECT_FALSE(search::IsQueryExtractionEnabled())
-      << "This test expects query extraction to be disabled.";
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_url_replacement_inactive,
-                         false, test_item.should_display_url);
-  }
-
-  search::EnableQueryExtractionForTesting();
-  EXPECT_TRUE(search::IsQueryExtractionEnabled());
-  EXPECT_TRUE(browser()->toolbar_model()->url_replacement_enabled());
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_query_extraction,
-                         test_item.would_perform_search_term_replacement,
-                         test_item.should_display_url);
-  }
-
-  // Disabling URL replacement should reset to only showing URLs.
-  browser()->toolbar_model()->set_url_replacement_enabled(false);
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_url_replacement_inactive,
-                         false, test_item.should_display_url);
+  for (const TestItem& test_item : test_items) {
+    NavigateAndCheckText(test_item.url, test_item.expected_text);
   }
 }
 
- // Verify that search terms are extracted while the page is loading.
-TEST_F(ToolbarModelTest, SearchTermsWhileLoading) {
-  search::EnableQueryExtractionForTesting();
-  AddTab(browser(), GURL(url::kAboutBlankURL));
-
-  // While loading, we should be willing to extract search terms.
-  content::NavigationController* controller =
-      &browser()->tab_strip_model()->GetWebContentsAt(0)->GetController();
-  controller->LoadURL(GURL("https://google.com/search?q=tractor+supply&espv=1"),
-                      content::Referrer(), ui::PAGE_TRANSITION_LINK,
-                      std::string());
-  ToolbarModel* toolbar_model = browser()->toolbar_model();
-  controller->GetVisibleEntry()->GetSSL().security_style =
-      content::SECURITY_STYLE_UNKNOWN;
-  EXPECT_TRUE(toolbar_model->WouldPerformSearchTermReplacement(false));
-
-  // When done loading, we shouldn't extract search terms if we didn't get an
-  // authenticated connection.
-  CommitPendingLoad(controller);
-  controller->GetVisibleEntry()->GetSSL().security_style =
-      content::SECURITY_STYLE_UNKNOWN;
-  EXPECT_FALSE(toolbar_model->WouldPerformSearchTermReplacement(false));
-}
-
-// When the Google base URL is overridden on the command line, we should extract
-// search terms from URLs that start with that base URL even when they're not
-// secure.
-TEST_F(ToolbarModelTest, GoogleBaseURL) {
-  search::EnableQueryExtractionForTesting();
-  AddTab(browser(), GURL(url::kAboutBlankURL));
-
-  // If the Google base URL wasn't specified on the command line, then if it's
-  // HTTP, we should not extract search terms.
-  UIThreadSearchTermsData::SetGoogleBaseURL("http://www.foo.com/");
-  NavigateAndCheckText(
-      GURL("http://www.foo.com/search?q=tractor+supply&espv=1"),
-      base::ASCIIToUTF16("www.foo.com/search?q=tractor+supply&espv=1"), false,
-      true);
-
-  // The same URL, when specified on the command line, should allow search term
-  // extraction.
-  UIThreadSearchTermsData::SetGoogleBaseURL(std::string());
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kGoogleBaseURL, "http://www.foo.com/");
-
-  // For the default search engine to reflect the new Google base URL we just
-  // set, the TemplateURLService has to know the base URL changed. Normally the
-  // GoogleURLTracker is the source of such change notifications, but since here
-  // we're modifying the base URL value directly by changing the command-line
-  // flags, we need to manually tell TemplateURLService to check for changes.
-  TemplateURLServiceFactory::GetInstance()->GetForProfile(profile())->
-      GoogleBaseURLChanged();
-
-  NavigateAndCheckText(
-      GURL("http://www.foo.com/search?q=tractor+supply&espv=1"),
-      base::ASCIIToUTF16("tractor supply"), true, true);
-}
-
 TEST_F(ToolbarModelTest, ShouldElideLongURLs) {
   AddTab(browser(), GURL(url::kAboutBlankURL));
   const std::string long_text(content::kMaxURLDisplayChars + 1024, '0');
@@ -362,37 +209,11 @@ TEST_F(ToolbarModelTest, ShouldElideLongURLs) {
   NavigateAndCheckElided(GURL(std::string("data:abc") + long_text));
 }
 
-// Test that URL display in a popup respects the query extraction flag.
+// Test URL display in a popup.
 TEST_F(PopupToolbarModelTest, ShouldDisplayURL) {
   AddTab(browser(), GURL(url::kAboutBlankURL));
 
-  // Check with query extraction disabled.
-  EXPECT_FALSE(search::IsQueryExtractionEnabled());
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_url_replacement_inactive,
-                         false, test_item.should_display_url);
-  }
-
-  // With query extraction enabled, search term replacement should be performed.
-  search::EnableQueryExtractionForTesting();
-  EXPECT_TRUE(search::IsQueryExtractionEnabled());
-  EXPECT_TRUE(browser()->toolbar_model()->url_replacement_enabled());
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_query_extraction,
-                         test_item.would_perform_search_term_replacement,
-                         test_item.should_display_url);
-  }
-
-  // Disabling URL replacement should reset to only showing URLs.
-  browser()->toolbar_model()->set_url_replacement_enabled(false);
-  for (size_t i = 0; i < arraysize(test_items); ++i) {
-    const TestItem& test_item = test_items[i];
-    NavigateAndCheckText(test_item.url,
-                         test_item.expected_text_url_replacement_inactive,
-                         false, test_item.should_display_url);
+  for (const TestItem& test_item : test_items) {
+    NavigateAndCheckText(test_item.url, test_item.expected_text);
   }
 }