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 3336 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};

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

Nit: "" -> std::string()

Place '}' on the start of the next line.

[Maria, 2013/11/28 00:17:03]

Done.

+
+   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

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

Nit: How about:

A deletion URL on a personalized query should be reflected in the resulting
AutocompleteMatch.

[Maria, 2013/11/28 00:17:03]

Done.

+       { "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

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

Nit: How about:

Personalized queries without deletion URLs shouldn't cause errors.

[Maria, 2013/11/28 00:17:03]

Done.

+       { "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.

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

Is there any particular reason why we should ignore this?  It seems like if a
server sends us a deletion URL, we should just accept it.  What problem are we
trying to avoid?

[Maria, 2013/11/28 00:17:03]

During results parsing we don't bother looking at the last map and parsing its
contents unless we are expecting some information to be there. So we first do a
check for a relevant suggest type result before doing all the work. While we
could always do it, currently 99% of the time that would be wasted work.

[Peter Kasting, 2013/11/28 00:44:17]

I left a comment on that code.

+       { "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_HasDeletionUrlSuccess) {
+  AutocompleteMatch m(

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

Tiny nit: |match| would be a better name than |m| (2 places)

[Maria, 2013/11/28 00:17:03]

Done.

+      provider_,

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

Nit: For an arg list this short, I'd just wrap as:

  AutocompleteMatch match(provider_, 0, true,
                          AutocompleteMatchType::SEARCH_SUGGEST);

(2 places)

[Maria, 2013/11/28 00:17:03]

Done.

+      0,
+      true,
+      AutocompleteMatchType::SEARCH_SUGGEST);
+  m.RecordAdditionalInfo(SearchProvider::kDeletionUrlKey,
+                         "https://www.google.com/complete/deleteitem?q=foo");
+  provider_->matches_.push_back(m);
+  provider_->DeleteMatch(m);

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

Seems like after this line, we should check that |deletion_handlers_| is
non-empty, to verify that the fetch will actually result in some kind of
processing.  We should probably also check that |matches_| is cleared
immediately.  (2 places)

[Maria, 2013/11/28 00:17:03]

Done.

+  // 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->delegate()->OnURLFetchComplete(fetcher);
+  ASSERT_TRUE(provider_->deletion_handlers_.empty());
+}
+
+TEST_F(SearchProviderTest, TestDeleteMatch_HasDeletionUrlFailure) {
+  AutocompleteMatch m(
+      provider_,
+      0,
+      true,
+      AutocompleteMatchType::SEARCH_SUGGEST);
+  m.RecordAdditionalInfo(SearchProvider::kDeletionUrlKey,
+                         "https://www.google.com/complete/deleteitem?q=foo");
+  provider_->matches_.push_back(m);
+  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->delegate()->OnURLFetchComplete(fetcher);
+  ASSERT_TRUE(provider_->deletion_handlers_.empty());
+}