Commit aef4857d authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

[android] Migrate ContextualSearchDelegate using SimpleURLLoader

URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates Android's ContextualSearchDelegate and the
respective unittests away from URLFetcher.

Bug: 773295,872875
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ifd2f3afd0db3ceaf75f6a8f972259c656ec7db69
Reviewed-on: https://chromium-review.googlesource.com/1221729
Commit-Queue: Donn Denman <donnd@chromium.org>
Reviewed-by: default avatarDonn Denman <donnd@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594025}
parent b209442f
...@@ -36,7 +36,8 @@ ...@@ -36,7 +36,8 @@
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "net/base/escape.h" #include "net/base/escape.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "net/url_request/url_fetcher.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "url/gurl.h" #include "url/gurl.h"
using content::RenderFrameHost; using content::RenderFrameHost;
...@@ -73,20 +74,19 @@ const char kDoPreventPreloadValue[] = "1"; ...@@ -73,20 +74,19 @@ const char kDoPreventPreloadValue[] = "1";
// The version of the Contextual Cards API that we want to invoke. // The version of the Contextual Cards API that we want to invoke.
const int kContextualCardsUrlActions = 3; const int kContextualCardsUrlActions = 3;
} // namespace const int kResponseCodeUninitialized = -1;
// URLFetcher ID, only used for tests: we only have one kind of fetcher. } // namespace
const int ContextualSearchDelegate::kContextualSearchURLFetcherID = 1;
// Handles tasks for the ContextualSearchManager in a separable, testable way. // Handles tasks for the ContextualSearchManager in a separable, testable way.
ContextualSearchDelegate::ContextualSearchDelegate( ContextualSearchDelegate::ContextualSearchDelegate(
net::URLRequestContextGetter* url_request_context, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
TemplateURLService* template_url_service, TemplateURLService* template_url_service,
const ContextualSearchDelegate::SearchTermResolutionCallback& const ContextualSearchDelegate::SearchTermResolutionCallback&
search_term_callback, search_term_callback,
const ContextualSearchDelegate::SurroundingTextCallback& const ContextualSearchDelegate::SurroundingTextCallback&
surrounding_text_callback) surrounding_text_callback)
: url_request_context_(url_request_context), : url_loader_factory_(std::move(url_loader_factory)),
template_url_service_(template_url_service), template_url_service_(template_url_service),
search_term_callback_(search_term_callback), search_term_callback_(search_term_callback),
surrounding_text_callback_(surrounding_text_callback) { surrounding_text_callback_(surrounding_text_callback) {
...@@ -132,7 +132,7 @@ void ContextualSearchDelegate::StartSearchTermResolutionRequest( ...@@ -132,7 +132,7 @@ void ContextualSearchDelegate::StartSearchTermResolutionRequest(
// Immediately cancel any request that's in flight, since we're building a new // Immediately cancel any request that's in flight, since we're building a new
// context (and the response disposes of any existing context). // context (and the response disposes of any existing context).
search_term_fetcher_.reset(); url_loader_.reset();
// Decide if the URL should be sent with the context. // Decide if the URL should be sent with the context.
GURL page_url(web_contents->GetURL()); GURL page_url(web_contents->GetURL());
...@@ -149,47 +149,47 @@ void ContextualSearchDelegate::ResolveSearchTermFromContext() { ...@@ -149,47 +149,47 @@ void ContextualSearchDelegate::ResolveSearchTermFromContext() {
GURL request_url(BuildRequestUrl(context_->GetHomeCountry())); GURL request_url(BuildRequestUrl(context_->GetHomeCountry()));
DCHECK(request_url.is_valid()); DCHECK(request_url.is_valid());
// Reset will delete any previous fetcher, and we won't get any callback. auto resource_request = std::make_unique<network::ResourceRequest>();
search_term_fetcher_.reset( resource_request->url = request_url;
net::URLFetcher::Create(kContextualSearchURLFetcherID, request_url,
net::URLFetcher::GET, this).release());
search_term_fetcher_->SetRequestContext(url_request_context_);
// Add Chrome experiment state to the request headers.
net::HttpRequestHeaders headers;
variations::AppendVariationHeadersUnknownSignedIn(
search_term_fetcher_->GetOriginalURL(),
variations::InIncognito::kNo, // Impossible to be incognito at this
// point.
&headers);
search_term_fetcher_->SetExtraRequestHeaders(headers.ToString());
SetDiscourseContextAndAddToHeader(*context_); // Populates the discourse context and adds it to the HTTP header of the
// search term resolution request.
resource_request->headers.AddHeadersFromString(
GetDiscourseContext(*context_));
// Disable cookies for this request. // Disable cookies for this request.
search_term_fetcher_->SetAllowCredentials(false); resource_request->allow_credentials = false;
search_term_fetcher_->Start(); // Add Chrome experiment state to the request headers.
// Reset will delete any previous loader, and we won't get any callback.
url_loader_ =
variations::CreateSimpleURLLoaderWithVariationsHeadersUnknownSignedIn(
std::move(resource_request),
variations::InIncognito::kNo, // Impossible to be incognito at this
// point.
NO_TRAFFIC_ANNOTATION_YET);
url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&ContextualSearchDelegate::OnUrlLoadComplete,
base::Unretained(this)));
} }
void ContextualSearchDelegate::OnURLFetchComplete( void ContextualSearchDelegate::OnUrlLoadComplete(
const net::URLFetcher* source) { std::unique_ptr<std::string> response_body) {
if (context_ == nullptr) if (!context_)
return; return;
DCHECK(source == search_term_fetcher_.get()); int response_code = kResponseCodeUninitialized;
int response_code = source->GetResponseCode(); if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {
response_code = url_loader_->ResponseInfo()->headers->response_code();
}
std::unique_ptr<ResolvedSearchTerm> resolved_search_term( std::unique_ptr<ResolvedSearchTerm> resolved_search_term(
new ResolvedSearchTerm(response_code)); new ResolvedSearchTerm(response_code));
if (source->GetStatus().is_success() && response_code == net::HTTP_OK) { if (response_body && response_code == net::HTTP_OK) {
std::string response; resolved_search_term =
bool has_string_response = source->GetResponseAsString(&response); GetResolvedSearchTermFromJson(response_code, *response_body);
DCHECK(has_string_response);
if (has_string_response && context_ != nullptr) {
resolved_search_term =
GetResolvedSearchTermFromJson(response_code, response);
}
} }
search_term_callback_.Run(*resolved_search_term); search_term_callback_.Run(*resolved_search_term);
} }
...@@ -234,7 +234,7 @@ ContextualSearchDelegate::GetResolvedSearchTermFromJson( ...@@ -234,7 +234,7 @@ ContextualSearchDelegate::GetResolvedSearchTermFromJson(
end_adjust = mention_end - context_->GetEndOffset(); end_adjust = mention_end - context_->GetEndOffset();
} }
} }
bool is_invalid = response_code == net::URLFetcher::RESPONSE_CODE_INVALID; bool is_invalid = response_code == kResponseCodeUninitialized;
return std::unique_ptr<ResolvedSearchTerm>(new ResolvedSearchTerm( return std::unique_ptr<ResolvedSearchTerm>(new ResolvedSearchTerm(
is_invalid, response_code, search_term, display_text, alternate_term, mid, is_invalid, response_code, search_term, display_text, alternate_term, mid,
prevent_preload == kDoPreventPreloadValue, start_adjust, end_adjust, prevent_preload == kDoPreventPreloadValue, start_adjust, end_adjust,
...@@ -325,11 +325,6 @@ void ContextualSearchDelegate::OnTextSurroundingSelectionAvailable( ...@@ -325,11 +325,6 @@ void ContextualSearchDelegate::OnTextSurroundingSelectionAvailable(
selection_end); selection_end);
} }
void ContextualSearchDelegate::SetDiscourseContextAndAddToHeader(
const ContextualSearchContext& context) {
search_term_fetcher_->AddExtraRequestHeader(GetDiscourseContext(context));
}
std::string ContextualSearchDelegate::GetDiscourseContext( std::string ContextualSearchDelegate::GetDiscourseContext(
const ContextualSearchContext& context) { const ContextualSearchContext& context) {
discourse_context::ClientDiscourseContext proto; discourse_context::ClientDiscourseContext proto;
......
...@@ -17,15 +17,15 @@ ...@@ -17,15 +17,15 @@
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/android/contextualsearch/contextual_search_context.h" #include "chrome/browser/android/contextualsearch/contextual_search_context.h"
#include "chrome/browser/android/contextualsearch/resolved_search_term.h" #include "chrome/browser/android/contextualsearch/resolved_search_term.h"
#include "net/url_request/url_fetcher_delegate.h"
namespace content { namespace content {
class WebContents; class WebContents;
} }
namespace net { namespace network {
class URLRequestContextGetter; class SharedURLLoaderFactory;
} // namespace net class SimpleURLLoader;
} // namespace network
class Profile; class Profile;
class TemplateURLService; class TemplateURLService;
...@@ -34,8 +34,7 @@ class ContextualSearchFieldTrial; ...@@ -34,8 +34,7 @@ class ContextualSearchFieldTrial;
// Handles tasks for the ContextualSearchManager in a separable and testable // Handles tasks for the ContextualSearchManager in a separable and testable
// way, without the complication of being connected to a Java object. // way, without the complication of being connected to a Java object.
class ContextualSearchDelegate class ContextualSearchDelegate
: public net::URLFetcherDelegate, : public base::SupportsWeakPtr<ContextualSearchDelegate> {
public base::SupportsWeakPtr<ContextualSearchDelegate> {
public: public:
// Provides the Resolved Search Term, called when the Resolve Request returns. // Provides the Resolved Search Term, called when the Resolve Request returns.
typedef base::Callback<void(const ResolvedSearchTerm&)> typedef base::Callback<void(const ResolvedSearchTerm&)>
...@@ -45,17 +44,14 @@ class ContextualSearchDelegate ...@@ -45,17 +44,14 @@ class ContextualSearchDelegate
void(const std::string&, const base::string16&, size_t, size_t)> void(const std::string&, const base::string16&, size_t, size_t)>
SurroundingTextCallback; SurroundingTextCallback;
// ID used in creating URLFetcher for Contextual Search results.
static const int kContextualSearchURLFetcherID;
// Constructs a delegate that will always call back to the given callbacks // Constructs a delegate that will always call back to the given callbacks
// when search term resolution or surrounding text responses are available. // when search term resolution or surrounding text responses are available.
ContextualSearchDelegate( ContextualSearchDelegate(
net::URLRequestContextGetter* url_request_context, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
TemplateURLService* template_url_service, TemplateURLService* template_url_service,
const SearchTermResolutionCallback& search_term_callback, const SearchTermResolutionCallback& search_term_callback,
const SurroundingTextCallback& surrounding_callback); const SurroundingTextCallback& surrounding_callback);
~ContextualSearchDelegate() override; virtual ~ContextualSearchDelegate();
// Gathers surrounding text and saves it locally in the given context. // Gathers surrounding text and saves it locally in the given context.
void GatherAndSaveSurroundingText( void GatherAndSaveSurroundingText(
...@@ -101,8 +97,7 @@ class ContextualSearchDelegate ...@@ -101,8 +97,7 @@ class ContextualSearchDelegate
FRIEND_TEST_ALL_PREFIXES(ContextualSearchDelegateTest, FRIEND_TEST_ALL_PREFIXES(ContextualSearchDelegateTest,
DecodeSearchTermFromJsonResponse); DecodeSearchTermFromJsonResponse);
// net::URLFetcherDelegate: void OnUrlLoadComplete(std::unique_ptr<std::string> response_body);
void OnURLFetchComplete(const net::URLFetcher* source) override;
// Resolves the search term specified by the current context. // Resolves the search term specified by the current context.
// Only needed for tests. TODO(donnd): make private and friend? // Only needed for tests. TODO(donnd): make private and friend?
...@@ -124,11 +119,6 @@ class ContextualSearchDelegate ...@@ -124,11 +119,6 @@ class ContextualSearchDelegate
int start_offset, int start_offset,
int end_offset); int end_offset);
// Populates the discourse context and adds it to the HTTP header of the
// search term resolution request.
void SetDiscourseContextAndAddToHeader(
const ContextualSearchContext& context);
// Populates and returns the discourse context. // Populates and returns the discourse context.
std::string GetDiscourseContext(const ContextualSearchContext& context); std::string GetDiscourseContext(const ContextualSearchContext& context);
...@@ -191,10 +181,10 @@ class ContextualSearchDelegate ...@@ -191,10 +181,10 @@ class ContextualSearchDelegate
} }
// The current request in progress, or NULL. // The current request in progress, or NULL.
std::unique_ptr<net::URLFetcher> search_term_fetcher_; std::unique_ptr<network::SimpleURLLoader> url_loader_;
// Holds the URL request context. Not owned. // Holds the URL loader factory.
net::URLRequestContextGetter* url_request_context_; scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
// Holds the TemplateURLService. Not owned. // Holds the TemplateURLService. Not owned.
TemplateURLService* template_url_service_; TemplateURLService* template_url_service_;
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "jni/ContextualSearchManager_jni.h" #include "jni/ContextualSearchManager_jni.h"
#include "net/url_request/url_fetcher_impl.h" #include "net/url_request/url_fetcher_impl.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/service_manager/public/cpp/binder_registry.h" #include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/interface_provider.h" #include "services/service_manager/public/cpp/interface_provider.h"
...@@ -84,7 +85,7 @@ ContextualSearchManager::ContextualSearchManager(JNIEnv* env, ...@@ -84,7 +85,7 @@ ContextualSearchManager::ContextualSearchManager(JNIEnv* env,
env, obj, reinterpret_cast<intptr_t>(this)); env, obj, reinterpret_cast<intptr_t>(this));
Profile* profile = ProfileManager::GetActiveUserProfile(); Profile* profile = ProfileManager::GetActiveUserProfile();
delegate_.reset(new ContextualSearchDelegate( delegate_.reset(new ContextualSearchDelegate(
profile->GetRequestContext(), profile->GetURLLoaderFactory(),
TemplateURLServiceFactory::GetForProfile(profile), TemplateURLServiceFactory::GetForProfile(profile),
base::Bind(&ContextualSearchManager::OnSearchTermResolutionResponse, base::Bind(&ContextualSearchManager::OnSearchTermResolutionResponse,
base::Unretained(this)), base::Unretained(this)),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment