Store xsrf token received with psuggest results.

chrome/browser/autocomplete/search_provider_unittest.cc

Index: chrome/browser/autocomplete/search_provider_unittest.cc
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc
index a0257db70f5f4135ea9f8dc0071c6dc65dd65e10..e01f65f4e8bdb03bb7c70220f5a4e905f874d14e 100644
--- a/chrome/browser/autocomplete/search_provider_unittest.cc
+++ b/chrome/browser/autocomplete/search_provider_unittest.cc
@@ -3323,7 +3323,8 @@ TEST_F(SearchProviderTest, RemoveStaleResultsTest) {
       } else {
         provider_->default_results_.suggest_results.push_back(
             SearchProvider::SuggestResult(ASCIIToUTF16(suggestion), string16(),
-                                          string16(), std::string(), false,
+                                          string16(), std::string(),
+                                          std::string(), false,
                                           cases[i].results[j].relevance,
                                           false, false));
       }
@@ -3804,6 +3805,97 @@ TEST_F(SearchProviderTest, XSSIGuardedJSONParsing) {
   }
 }
 
+// Test that deletion url gets set on an AutocompleteMatch when available for a
+// personalized query.
+TEST_F(SearchProviderTest, ParseDeletionUrl) {
+   struct Match {
+     std::string contents;
+     std::string deletion_url;
+     AutocompleteMatchType::Type type;
+   };
+
+   const Match kEmptyMatch = {
+       kNotApplicable, "", AutocompleteMatchType::NUM_TYPES};
+
+   const char url[] = "https://www.google.com/complete/deleteitems"
+       "?delq=ab&client=chrome&deltok=xsrf123";
+
+   struct {
+       const std::string input_text;
+       const std::string response_json;
+       const Match matches[4];
+     } cases[] = {
+       // Personalized query comes with a deletion url
+       { "a",
+         "[\"a\",[\"ab\", \"ac\"],[],[],"
+         "{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\"],"
+         "\"google:suggestrelevance\":[1, 2],"
+         "\"google:suggestdetail\":[{\"du\":"
+         "\"https://www.google.com/complete/deleteitems?delq=ab&client=chrome"
+         "&deltok=xsrf123\"}, {}]}]",
+         { { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
+           { "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
+           { "ab", url, AutocompleteMatchType::SEARCH_SUGGEST },
+           kEmptyMatch,
+         },
+       },
+       // Personalized query, but no deletion url in response -- old server case
+       { "a",
+         "[\"a\",[\"ab\", \"ac\"],[],[],"
+         "{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\"],"
+         "\"google:suggestrelevance\":[1, 2],"
+         "\"google:suggestdetail\":[{}, {}]}]",
+         { { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
+           { "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
+           { "ab", "", AutocompleteMatchType::SEARCH_SUGGEST },
+           kEmptyMatch,
+         },
+       },
+       // Unexpected: non-personalized query comes with a deletion url, this
+       // should be ignored.
+       { "a",
+         "[\"a\",[\"ab\", \"ac\"],[],[],"
+         "{\"google:suggesttype\":[\"PERSONALIZED_QUERY\",\"QUERY\"],"
+         "\"google:suggestrelevance\":[1, 2],"
+         "\"google:suggestdetail\":[{\"du\":\""
+         "https://www.google.com/complete/deleteitems?delq=ab&"
+         "client=chrome&deltok=xsrf123\"}, {\"du\":\""
+         "https://www.google.com/complete/deleteitems?delq=ab&"
+         "client=chrome&deltok=xsrf123\"}]}]",
+         { { "a", "", AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED },
+           { "ac", "", AutocompleteMatchType::SEARCH_SUGGEST },
+           { "ab", url, AutocompleteMatchType::SEARCH_SUGGEST },
+           kEmptyMatch,
+         },
+       },
+     };
+
+     for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
+       QueryForInput(ASCIIToUTF16(cases[i].input_text), false, false);
+
+       net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
+           SearchProvider::kDefaultProviderURLFetcherID);
+       ASSERT_TRUE(fetcher);
+       fetcher->set_response_code(200);
+       fetcher->SetResponseString(cases[i].response_json);
+       fetcher->delegate()->OnURLFetchComplete(fetcher);
+
+       RunTillProviderDone();
+
+       const ACMatches& matches = provider_->matches();
+       ASSERT_FALSE(matches.empty());
+
+       SCOPED_TRACE("for input with json = " + cases[i].response_json);
+
+       for (size_t j = 0; j < matches.size(); ++j) {
+         const Match& match = cases[i].matches[j];
+         SCOPED_TRACE(" and match index: " + base::IntToString(j));
+         EXPECT_EQ(match.contents, UTF16ToUTF8(matches[j].contents));
+         EXPECT_EQ(match.deletion_url, matches[j].GetAdditionalInfo(
+             "deletion_url"));
+       }
+     }
+}
 
 TEST_F(SearchProviderTest, ReflectsBookmarkBarState) {
   profile_.GetPrefs()->SetBoolean(prefs::kShowBookmarkBar, false);
@@ -3950,3 +4042,55 @@ TEST_F(SearchProviderTest, CanSendURL) {
       GURL("https://www.google.com/complete/search"), &google_template_url,
       AutocompleteInput::OTHER, &profile_));
 }
+
+TEST_F(SearchProviderTest, TestDeleteMatch_NoDeletionUrl) {
+  AutocompleteMatch m = AutocompleteMatch(

[Peter Kasting, 2013/11/23 00:08:43]

Nit: Always do "Class varname(...)" instead of "Class varname = Class(...)";
it's not only shorter, it's more efficient.  (severla places)

[Maria, 2013/11/26 02:36:27]

Done.

+      provider_,
+      640,

[Peter Kasting, 2013/11/23 00:08:43]

Nit: Is the value here important?  If not use 0 or some other value that looks
less particularly-important.  (several places)

[Maria, 2013/11/26 02:36:27]

Done.

+      true,
+      AutocompleteMatchType::SEARCH_SUGGEST);
+  provider_->DeleteMatch(m);
+  net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
+      SearchProvider::kDeletionURLFetcherID);
+  // Ensure that an autocomplete match without a deletion URL does not
+  // trigger a fetch.
+  ASSERT_FALSE(fetcher);
+}
+
+TEST_F(SearchProviderTest, TestDeleteMatch_HasDeletionUrlSuccess) {
+  AutocompleteMatch m = AutocompleteMatch(
+      provider_,
+      640,
+      true,
+      AutocompleteMatchType::SEARCH_SUGGEST);
+  m.RecordAdditionalInfo(SearchProvider::kDeletionUrlKey,
+                         "https://www.google.com/complete/deleteitem?q=foo");
+  provider_->DeleteMatch(m);
+  // Set up a default fetcher with provided results.
+  net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
+      SearchProvider::kDeletionURLFetcherID);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(200);
+  fetcher->SetResponseString("");

[Peter Kasting, 2013/11/23 00:08:43]

Nit: Is this line necessary at all?  If it is, use std::string()  (2 places)

[Maria, 2013/11/26 02:36:27]

Done.

+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  ASSERT_TRUE(provider_->deletion_handler_.get() == NULL);

[Peter Kasting, 2013/11/23 00:08:43]

What is |deletion_handler_|?  I don't see it in your change.

Also, it looks like this test and the next both check the same thing: there is
no deletion handler.  It would be nice to have tests that test that something
obviously different happens in response to success versus failure -- if such a
thing is even possible (it may not be if you follow my refactoring suggestions
in the .cc file).

[Maria, 2013/11/26 02:36:27]

I would like to check whether my metric got recorded correctly in UMA, but I am
not sure how to figure that out in a unit test. Any suggestions?

[Peter Kasting, 2013/11/26 02:46:22]

Nope.  Sorry.

[Peter Kasting, 2013/11/27 21:58:18]

OK, here's an idea.  Subclass SearchProvider for this test and override
OnDeletionComplete() so you can tell when it gets called back and with what arg.
 Then these tests can at least check that the callback is called with the
correct arg when it's supposed to be.

+}
+
+TEST_F(SearchProviderTest, TestDeleteMatch_HasDeletionUrlFailure) {
+  AutocompleteMatch m = AutocompleteMatch(
+      provider_,
+      640,
+      true,
+      AutocompleteMatchType::SEARCH_SUGGEST);
+  m.RecordAdditionalInfo(SearchProvider::kDeletionUrlKey,
+                         "https://www.google.com/complete/deleteitem?q=foo");
+  provider_->DeleteMatch(m);
+  // Set up a default fetcher with provided results.
+  net::TestURLFetcher* fetcher = test_factory_.GetFetcherByID(
+      SearchProvider::kDeletionURLFetcherID);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_response_code(500);
+  fetcher->SetResponseString("");
+  fetcher->delegate()->OnURLFetchComplete(fetcher);
+  ASSERT_TRUE(provider_->deletion_handler_.get() == NULL);
+}