Commit 5b4b5ddf authored by John Abd-El-Malek's avatar John Abd-El-Malek Committed by Commit Bot

Convert PasswordProtectionRequest to use SimpleURLLoader instead of URLFetcher.

Bug: 825242
Change-Id: Id08270e339e5a7ae33d2374f77d56d2de0da2254
Reviewed-on: https://chromium-review.googlesource.com/988693
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarJialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547624}
parent bb4b07b5
......@@ -150,7 +150,7 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
Profile* profile)
: PasswordProtectionService(
sb_service->database_manager(),
sb_service->url_request_context(),
sb_service->GetURLLoaderFactory(),
HistoryServiceFactory::GetForProfile(
profile,
ServiceAccessType::EXPLICIT_ACCESS),
......
......@@ -54,6 +54,7 @@ source_set("password_protection_unittest") {
"//components/sync_preferences:test_support",
"//content/test:test_support",
"//net:test_support",
"//services/network:test_support",
"//testing/gmock",
"//testing/gtest",
"//third_party/protobuf:protobuf_lite",
......
......@@ -5,5 +5,7 @@ include_rules = [
"+components/sync_preferences/testing_pref_service_syncable.h",
"+content/public/test",
"+net",
"+services/network/public",
"+services/network/test",
]
......@@ -19,6 +19,7 @@
#include "net/base/url_util.h"
#include "net/http/http_status_code.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "url/origin.h"
using content::BrowserThread;
......@@ -266,18 +267,20 @@ void PasswordProtectionRequest::SendRequest() {
}
}
})");
fetcher_ = net::URLFetcher::Create(
0, PasswordProtectionService::GetPasswordProtectionRequestUrl(),
net::URLFetcher::POST, this, traffic_annotation);
data_use_measurement::DataUseUserData::AttachToFetcher(
fetcher_.get(), data_use_measurement::DataUseUserData::SAFE_BROWSING);
fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
fetcher_->SetAutomaticallyRetryOn5xx(false);
fetcher_->SetRequestContext(
password_protection_service_->request_context_getter().get());
fetcher_->SetUploadData("application/octet-stream", serialized_request);
auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url =
PasswordProtectionService::GetPasswordProtectionRequestUrl();
resource_request->method = "POST";
resource_request->load_flags = net::LOAD_DISABLE_CACHE;
url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
traffic_annotation);
url_loader_->AttachStringForUpload(serialized_request,
"application/octet-stream");
request_start_time_ = base::TimeTicks::Now();
fetcher_->Start();
url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
password_protection_service_->url_loader_factory().get(),
base::BindOnce(&PasswordProtectionRequest::OnURLLoaderComplete,
base::Unretained(this)));
}
void PasswordProtectionRequest::StartTimeout() {
......@@ -293,16 +296,18 @@ void PasswordProtectionRequest::StartTimeout() {
base::TimeDelta::FromMilliseconds(request_timeout_in_ms_));
}
void PasswordProtectionRequest::OnURLFetchComplete(
const net::URLFetcher* source) {
void PasswordProtectionRequest::OnURLLoaderComplete(
std::unique_ptr<std::string> response_body) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
net::URLRequestStatus status = source->GetStatus();
const bool is_success = status.is_success();
const int response_code = source->GetResponseCode();
int response_code = 0;
if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers)
response_code = url_loader_->ResponseInfo()->headers->response_code();
const bool is_success = url_loader_->NetError() == net::OK;
base::UmaHistogramSparse(
"PasswordProtection.PasswordProtectionResponseOrErrorCode",
is_success ? response_code : status.error());
is_success ? response_code : url_loader_->NetError());
if (!is_success || net::HTTP_OK != response_code) {
Finish(PasswordProtectionService::FETCH_FAILED, nullptr);
......@@ -311,13 +316,11 @@ void PasswordProtectionRequest::OnURLFetchComplete(
std::unique_ptr<LoginReputationClientResponse> response =
std::make_unique<LoginReputationClientResponse>();
std::string response_body;
bool received_data = source->GetResponseAsString(&response_body);
DCHECK(received_data);
fetcher_.reset(); // We don't need it anymore.
DCHECK(response_body.get());
url_loader_.reset(); // We don't need it anymore.
UMA_HISTOGRAM_TIMES("PasswordProtection.RequestNetworkDuration",
base::TimeTicks::Now() - request_start_time_);
if (response->ParseFromString(response_body))
if (response_body.get() && response->ParseFromString(*response_body.get()))
Finish(PasswordProtectionService::SUCCEEDED, std::move(response));
else
Finish(PasswordProtectionService::RESPONSE_MALFORMED, nullptr);
......@@ -373,7 +376,7 @@ void PasswordProtectionRequest::Finish(
void PasswordProtectionRequest::Cancel(bool timed_out) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
fetcher_.reset();
url_loader_.reset();
// If request is canceled because |password_protection_service_| is shutting
// down, ignore all these deferred navigations.
if (!timed_out)
......
......@@ -10,14 +10,15 @@
#include "base/task/cancelable_task_tracker.h"
#include "components/safe_browsing/password_protection/password_protection_service.h"
#include "content/public/browser/browser_thread.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_fetcher_delegate.h"
#include "net/url_request/url_request_status.h"
#include <vector>
class GURL;
namespace network {
class SimpleURLLoader;
}
namespace safe_browsing {
class PasswordProtectionNavigationThrottle;
......@@ -45,10 +46,10 @@ extern const char kProtectedPasswordEntryVerdictHistogram[];
// (7) | UI | On receiving response, handle response and finish.
// | | On request timeout, cancel request.
// | | On deletion of |password_protection_service_|, cancel request.
class PasswordProtectionRequest : public base::RefCountedThreadSafe<
PasswordProtectionRequest,
content::BrowserThread::DeleteOnUIThread>,
public net::URLFetcherDelegate {
class PasswordProtectionRequest
: public base::RefCountedThreadSafe<
PasswordProtectionRequest,
content::BrowserThread::DeleteOnUIThread> {
public:
PasswordProtectionRequest(content::WebContents* web_contents,
const GURL& main_frame_url,
......@@ -73,9 +74,8 @@ class PasswordProtectionRequest : public base::RefCountedThreadSafe<
// due to timeout. This function will call Finish() to destroy |this|.
void Cancel(bool timed_out);
// net::URLFetcherDelegate override.
// Processes the received response.
void OnURLFetchComplete(const net::URLFetcher* source) override;
void OnURLLoaderComplete(std::unique_ptr<std::string> response_body);
GURL main_frame_url() const { return main_frame_url_; }
......@@ -117,7 +117,7 @@ class PasswordProtectionRequest : public base::RefCountedThreadSafe<
content::BrowserThread::UI>;
friend class base::DeleteHelper<PasswordProtectionRequest>;
friend class ChromePasswordProtectionServiceTest;
~PasswordProtectionRequest() override;
virtual ~PasswordProtectionRequest();
// Start checking the whitelist.
void CheckWhitelist();
......@@ -176,8 +176,8 @@ class PasswordProtectionRequest : public base::RefCountedThreadSafe<
// When request is sent.
base::TimeTicks request_start_time_;
// URLFetcher instance for sending request and receiving response.
std::unique_ptr<net::URLFetcher> fetcher_;
// SimpleURLLoader instance for sending request and receiving response.
std::unique_ptr<network::SimpleURLLoader> url_loader_;
// The PasswordProtectionService instance owns |this|.
// Can only be accessed on UI thread.
......
......@@ -98,13 +98,13 @@ const char kSyncPasswordChromeSettingsHistogram[] =
PasswordProtectionService::PasswordProtectionService(
const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
scoped_refptr<net::URLRequestContextGetter> request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
HistoryService* history_service,
HostContentSettingsMap* host_content_settings_map)
: stored_verdict_count_password_on_focus_(-1),
stored_verdict_count_password_entry_(-1),
database_manager_(database_manager),
request_context_getter_(request_context_getter),
url_loader_factory_(url_loader_factory),
history_service_observer_(this),
content_settings_(host_content_settings_map),
weak_factory_(this) {
......
......@@ -21,7 +21,7 @@
#include "components/safe_browsing/common/safe_browsing_prefs.h"
#include "components/safe_browsing/db/v4_protocol_manager_util.h"
#include "components/safe_browsing/proto/csd.pb.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "third_party/protobuf/src/google/protobuf/repeated_field.h"
namespace content {
......@@ -114,7 +114,7 @@ class PasswordProtectionService : public history::HistoryServiceObserver {
PasswordProtectionService(
const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
scoped_refptr<net::URLRequestContextGetter> request_context_getter,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
history::HistoryService* history_service,
HostContentSettingsMap* host_content_settings_map);
......@@ -268,8 +268,8 @@ class PasswordProtectionService : public history::HistoryServiceObserver {
virtual int GetStoredVerdictCount(
LoginReputationClientRequest::TriggerType trigger_type);
scoped_refptr<net::URLRequestContextGetter> request_context_getter() {
return request_context_getter_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory() {
return url_loader_factory_;
}
// Returns the URL where PasswordProtectionRequest instances send requests.
......@@ -402,7 +402,7 @@ class PasswordProtectionService : public history::HistoryServiceObserver {
// The context we use to issue network requests. This request_context_getter
// is obtained from SafeBrowsingService so that we can use the Safe Browsing
// cookie store.
scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
// Set of pending PasswordProtectionRequests that are still waiting for
// verdict.
......
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