[InstantExtended] Fixing how PageLoadSRP is emitted. Previously it was emitted only for Instant sea…

chrome/browser/search_engines/template_url_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 "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/rlz/rlz.h"
@@ skipping 1150 matching lines @@
   CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kExtraSearchQueryParams, "a=b");
   EXPECT_EQ("http://www.google.com/search?a=b&q=abc#oq=def&x",
             url.url_ref().ReplaceSearchTerms(search_terms));
 
   // Turn off the flag.  Now the command-line arg should be ignored again.
   search_terms.append_extra_query_params = false;
   EXPECT_EQ("http://www.google.com/search?q=abc#oq=def&x",
             url.url_ref().ReplaceSearchTerms(search_terms));
 }
+
+// Test the IsSearchResults function.
+TEST_F(TemplateURLTest, IsSearchResults) {
+  TemplateURLData data;
+  data.SetURL("http://bar/search?q={searchTerms}");
+  data.instant_url = "http://bar/instant#q={searchTerms}";
+  data.alternate_urls.push_back("http://bar/?q={searchTerms}");
+  data.alternate_urls.push_back("http://bar/#q={searchTerms}");
+  data.alternate_urls.push_back("http://bar/search#q{searchTerms}");
+  data.alternate_urls.push_back("http://bar/webhp#q={searchTerms}");
+  TemplateURL search_provider(NULL, data);
+
+  static const struct {

[Peter Kasting, 2013/08/06 21:32:19]

Nit: Hmm, should this really be static and not just const?

[rpetterson, 2013/08/06 23:04:17]

True. Static is probably unnecessary. I was looking at the test
GoogleBaseSuggestURL which may be overly static as well.

+    const char* const url;
+    bool result;
+  } url_data[] = {
+    { "http://bar/search?q=foo&oq=foo", true, },
+    { "http://bar/?q=foo&oq=foo", true, },
+    { "http://bar/#output=search&q=foo&oq=foo", true, },
+    { "http://bar/webhp#q=foo&oq=foo", true, },
+    { "http://bar/#q=foo&oq=foo", true, },
+    { "http://bar/?ext=foo&q=foo#ref=bar", true, },
+    { "http://bar/url?url=http://www.foo.com/&q=foo#ref=bar", false, },
+    { "http://bar/", false, },
+    { "http://foo/", false, },
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(url_data); ++i) {
+    bool is_search_results_page =
+        search_provider.IsSearchURL(GURL(url_data[i].url));

[Peter Kasting, 2013/08/06 21:32:19]

Nit: I'd just inline this below and save a line, it's still clear.

[rpetterson, 2013/08/06 23:04:17]

Done.

+    EXPECT_EQ(is_search_results_page, url_data[i].result);

[Peter Kasting, 2013/08/06 21:32:19]

Nit: (expected, actual)

[rpetterson, 2013/08/06 23:04:17]

Done.

+  }
+}