SuggestionsService blacklist handling.

chrome/browser/search/suggestions/suggestions_service.cc

 // Copyright 2014 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/search/suggestions/suggestions_service.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 namespace suggestions {
@@ skipping 29 matching lines @@
   for (it = requestors->begin(); it != requestors->end(); ++it) {
     it->Run(suggestions);
   }
   std::vector<SuggestionsService::ResponseCallback>().swap(*requestors);
 }
 
 }  // namespace
 
 const char kSuggestionsFieldTrialName[] = "ChromeSuggestions";
 const char kSuggestionsFieldTrialURLParam[] = "url";
+const char kSuggestionsFieldTrialSuggestionsSuffixParam[] =
+    "suggestions_suffix";
+const char kSuggestionsFieldTrialBlacklistSuffixParam[] = "blacklist_suffix";
 const char kSuggestionsFieldTrialStateParam[] = "state";
 const char kSuggestionsFieldTrialStateEnabled[] = "enabled";
 
 SuggestionsService::SuggestionsService(Profile* profile)
     : profile_(profile) {
   // Obtain the URL to use to fetch suggestions data from the Variations param.
-  suggestions_url_ = GURL(GetExperimentParam(kSuggestionsFieldTrialURLParam));
+  suggestions_url_ = GURL(
+      GetExperimentParam(kSuggestionsFieldTrialURLParam) +
+      GetExperimentParam(kSuggestionsFieldTrialSuggestionsSuffixParam));
+  blacklist_url_prefix_ = GetExperimentParam(kSuggestionsFieldTrialURLParam) +
+      GetExperimentParam(kSuggestionsFieldTrialBlacklistSuffixParam);
 }
 
 SuggestionsService::~SuggestionsService() {
 }
 
 // static
 bool SuggestionsService::IsEnabled() {
   return GetExperimentParam(kSuggestionsFieldTrialStateParam) ==
       kSuggestionsFieldTrialStateEnabled;
 }
 
 void SuggestionsService::FetchSuggestionsData(
     SuggestionsService::ResponseCallback callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   if (pending_request_.get()) {

[Jered, 2014/05/27 22:33:17]

What if the pending request is a blacklist request?

[manzagop (departed), 2014/05/29 21:40:31]

That's fine as the blacklist request returns updated suggestions.

     // Request already exists, so just add requestor to queue.
     waiting_requestors_.push_back(callback);
     return;
   }
 
   // Form new request.
   DCHECK(waiting_requestors_.empty());
   waiting_requestors_.push_back(callback);
 
-  pending_request_.reset(net::URLFetcher::Create(
-      0, suggestions_url_, net::URLFetcher::GET, this));
-  pending_request_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
-  pending_request_->SetRequestContext(profile_->GetRequestContext());
-  // Add Chrome experiment state to the request headers.
-  net::HttpRequestHeaders headers;
-  chrome_variations::VariationsHttpHeaderProvider::GetInstance()->
-      AppendHeaders(pending_request_->GetOriginalURL(),
-                    profile_->IsOffTheRecord(), false, &headers);
-  pending_request_->SetExtraRequestHeaders(headers.ToString());
+  pending_request_.reset(CreateSuggestionsRequest(suggestions_url_));
   pending_request_->Start();
 
   last_request_started_time_ = base::TimeTicks::Now();
 }
 
+void SuggestionsService::AddBlacklistedURL(
+    const GURL& candidate_url,
+    SuggestionsService::ResponseCallback callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  waiting_requestors_.push_back(callback);
+
+  if (pending_request_.get()) {
+    if (IsBlacklistRequest(pending_request_.get())) {
+      // Pending request is a blacklist request. Silently drop the new blacklist
+      // request. TODO - handle this case.
+      return;
+    } else {
+      // Pending request is not a blacklist request - cancel it and go on to
+      // issuing a blacklist request.
+      pending_request_.reset();
+    }
+  }
+
+  // Send blacklisting request.
+  GURL url(blacklist_url_prefix_ +
+           net::EscapeQueryParamValue(candidate_url.spec(), true));
+  pending_request_.reset(CreateSuggestionsRequest(url));

[Jered, 2014/05/27 22:33:17]

nit: To me this makes more sense as a PUT request than a GET request.

[manzagop (departed), 2014/05/29 21:40:31]

You're quite right: this is a request with server-side side-effects.

Would you be ok leaving this as a TODO, until we come up with a story for the
server accepting PUT requests?

+  pending_request_->Start();
+  last_request_started_time_ = base::TimeTicks::Now();
+}
+
 void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK_EQ(pending_request_.get(), source);
 
   // The fetcher will be deleted when the request is handled.
   scoped_ptr<const net::URLFetcher> request(pending_request_.release());
   const net::URLRequestStatus& request_status = request->GetStatus();
   if (request_status.status() != net::URLRequestStatus::SUCCESS) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("Suggestions.FailedRequestErrorCode",
                                 -request_status.error());
@@ skipping 38 matching lines @@
 }
 
 void SuggestionsService::Shutdown() {
   // Cancel pending request.
   pending_request_.reset(NULL);
 
   // Dispatch empty suggestions to requestors.
   DispatchRequestsAndClear(SuggestionsProfile(), &waiting_requestors_);
 }
 
+bool SuggestionsService::IsBlacklistRequest(net::URLFetcher* request) const {
+  DCHECK(request);
+  return StartsWithASCII(request->GetOriginalURL().spec(),
+                         blacklist_url_prefix_,
+                         true);
+}
+
+net::URLFetcher* SuggestionsService::CreateSuggestionsRequest(const GURL& url) {
+  net::URLFetcher* request = net::URLFetcher::Create(
+      0, url, net::URLFetcher::GET, this);
+  request->SetLoadFlags(net::LOAD_DISABLE_CACHE);
+  request->SetRequestContext(profile_->GetRequestContext());
+  // Add Chrome experiment state to the request headers.
+  net::HttpRequestHeaders headers;
+  chrome_variations::VariationsHttpHeaderProvider::GetInstance()->
+      AppendHeaders(request->GetOriginalURL(), profile_->IsOffTheRecord(),
+                    false, &headers);
+  request->SetExtraRequestHeaders(headers.ToString());
+  return request;
+}
+
 }  // namespace suggestions