Store xsrf token received with psuggest results.

chrome/browser/autocomplete/search_provider_unittest.cc

 // Copyright 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 "chrome/browser/autocomplete/search_provider.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
@@ skipping 3305 matching lines @@
       if (suggestion == kNotApplicable)
         break;
       if (cases[i].results[j].is_navigation_result) {
         provider_->default_results_.navigation_results.push_back(
             SearchProvider::NavigationResult(
                 *provider_.get(), GURL(suggestion), string16(), false,
                 cases[i].results[j].relevance, false));
       } 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));
       }
     }
 
     provider_->input_ = AutocompleteInput(
         ASCIIToUTF16(cases[i].omnibox_input), string16::npos, string16(),
         GURL(), AutocompleteInput::INVALID_SPEC, false, false, true,
         AutocompleteInput::ALL_MATCHES);
     provider_->RemoveAllStaleResults();
@@ skipping 460 matching lines @@
       EXPECT_EQ(cases[i].matches[j].type, matches[j].type);
     }
     for (; j < ARRAYSIZE_UNSAFE(cases[i].matches); ++j) {
       SCOPED_TRACE("and match: " + base::IntToString(j));
       EXPECT_EQ(cases[i].matches[j].contents, kNotApplicable);
       EXPECT_EQ(cases[i].matches[j].type, AutocompleteMatchType::NUM_TYPES);
     }
   }
 }
 
+// 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);
   string16 term = term1_.substr(0, term1_.length() - 1);
   QueryForInput(term, true, false);
   ASSERT_FALSE(provider_->matches().empty());
   EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
             provider_->matches()[0].type);
   ASSERT_TRUE(provider_->matches()[0].search_terms_args != NULL);
   EXPECT_FALSE(provider_->matches()[0].search_terms_args->bookmark_bar_pinned);
@@ skipping 126 matching lines @@
       AutocompleteInput::OTHER, &profile_));
   encrypted_types.Remove(syncer::SESSIONS);
   service->OnEncryptedTypesChanged(encrypted_types, false);
 
   // Check that there were no side effects from previous tests.
   EXPECT_TRUE(SearchProvider::CanSendURL(
       GURL("http://www.google.com/search"),
       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);
+}