Skip to content

Commit

Permalink
net: Move ownership of UploadDataStream from URLRequestHttpJob to URL…
Browse files Browse the repository at this point in the history
…Request

URLRequest creates and owns UploadDataStream.
URLRequest::get_upload_mutable and URLRequest::AppendBytesToUpload are removed because they can modify UploadData after UploadDataStream is created.
Return type of URLRequest::get_upload is changed from UploadData to UploadDataStream.
A number of methods are added to UploadElementReader and its subclasses to support URLRequestAutomationJob and some other users.

BUG=156574
TEST=git try

TBR=jam@chromium.org (for chrome/browser/automation, already reviewed by ananta@chromium.org)

Review URL: https://chromiumcodereview.appspot.com/11419034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@170028 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
hashimoto@chromium.org committed Nov 28, 2012
1 parent bf63a0c commit 0736d9e
Show file tree
Hide file tree
Showing 26 changed files with 194 additions and 111 deletions.
42 changes: 40 additions & 2 deletions chrome/browser/automation/url_request_automation_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#include "net/base/host_port_pair.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_data_stream.h"
#include "net/base/upload_file_element_reader.h"
#include "net/cookies/cookie_monster.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_response_headers.h"
Expand All @@ -29,9 +32,11 @@ using base::TimeDelta;
using content::BrowserThread;
using content::ResourceRequestInfo;

namespace {

// The list of filtered headers that are removed from requests sent via
// StartAsync(). These must be lower case.
static const char* const kFilteredHeaderStrings[] = {
const char* const kFilteredHeaderStrings[] = {
"connection",
"cookie",
"expect",
Expand All @@ -43,6 +48,34 @@ static const char* const kFilteredHeaderStrings[] = {
"via"
};

// Creates UploadData from UploadDataStream.
net::UploadData* CreateUploadData(
const net::UploadDataStream* upload_data_stream) {
net::UploadData* upload_data = new net::UploadData();
const ScopedVector<net::UploadElementReader>& element_readers =
upload_data_stream->element_readers();
for (size_t i = 0; i < element_readers.size(); ++i) {
const net::UploadElementReader* reader = element_readers[i];
if (reader->AsBytesReader()) {
const net::UploadBytesElementReader* bytes_reader =
reader->AsBytesReader();
upload_data->AppendBytes(bytes_reader->bytes(), bytes_reader->length());
} else if (reader->AsFileReader()) {
const net::UploadFileElementReader* file_reader =
reader->AsFileReader();
upload_data->AppendFileRange(file_reader->path(),
file_reader->range_offset(),
file_reader->range_length(),
file_reader->expected_modification_time());
} else {
NOTIMPLEMENTED();
}
}
return upload_data;
}

} // namespace

int URLRequestAutomationJob::instance_count_ = 0;
bool URLRequestAutomationJob::is_protocol_factory_registered_ = false;

Expand Down Expand Up @@ -463,13 +496,18 @@ void URLRequestAutomationJob::StartAsync() {
resource_type = info->GetResourceType();
}

// Construct UploadData from UploadDataStream.
scoped_refptr<net::UploadData> upload_data;
if (request_->get_upload())
upload_data = CreateUploadData(request_->get_upload());

// Ask automation to start this request.
AutomationURLRequest automation_request;
automation_request.url = request_->url().spec();
automation_request.method = request_->method();
automation_request.referrer = referrer.spec();
automation_request.extra_request_headers = new_request_headers.ToString();
automation_request.upload_data = request_->get_upload_mutable();
automation_request.upload_data = upload_data;
automation_request.resource_type = resource_type;
automation_request.load_flags = request_->load_flags();

Expand Down
31 changes: 17 additions & 14 deletions chrome/browser/extensions/api/web_request/upload_data_presenter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
#include "base/values.h"
#include "chrome/browser/extensions/api/web_request/form_data_parser.h"
#include "chrome/browser/extensions/api/web_request/web_request_api_constants.h"
#include "net/base/upload_element.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_file_element_reader.h"
#include "net/url_request/url_request.h"

using base::BinaryValue;
Expand Down Expand Up @@ -60,18 +61,19 @@ RawDataPresenter::RawDataPresenter()
}
RawDataPresenter::~RawDataPresenter() {}

void RawDataPresenter::FeedNext(const net::UploadElement& element) {
void RawDataPresenter::FeedNext(const net::UploadElementReader& reader) {
if (!success_)
return;

switch (element.type()) {
case net::UploadElement::TYPE_BYTES:
FeedNextBytes(element.bytes(), element.bytes_length());
break;
case net::UploadElement::TYPE_FILE:
// Insert the file path instead of the contents, which may be too large.
FeedNextFile(element.file_path().AsUTF8Unsafe());
break;
if (reader.AsBytesReader()) {
const net::UploadBytesElementReader* bytes_reader = reader.AsBytesReader();
FeedNextBytes(bytes_reader->bytes(), bytes_reader->length());
} else if (reader.AsFileReader()) {
// Insert the file path instead of the contents, which may be too large.
const net::UploadFileElementReader* file_reader = reader.AsFileReader();
FeedNextFile(file_reader->path().AsUTF8Unsafe());
} else {
NOTIMPLEMENTED();
}
}

Expand Down Expand Up @@ -114,15 +116,16 @@ ParsedDataPresenter::ParsedDataPresenter(const net::URLRequest& request)

ParsedDataPresenter::~ParsedDataPresenter() {}

void ParsedDataPresenter::FeedNext(const net::UploadElement& element) {
void ParsedDataPresenter::FeedNext(const net::UploadElementReader& reader) {
if (!success_)
return;

if (element.type() != net::UploadElement::TYPE_BYTES) {
const net::UploadBytesElementReader* bytes_reader = reader.AsBytesReader();
if (!bytes_reader) {
return;
}
if (!parser_->SetSource(base::StringPiece(element.bytes(),
element.bytes_length()))) {
if (!parser_->SetSource(base::StringPiece(bytes_reader->bytes(),
bytes_reader->length()))) {
Abort();
return;
}
Expand Down
22 changes: 11 additions & 11 deletions chrome/browser/extensions/api/web_request/upload_data_presenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FormDataParser;

namespace net {
class URLRequest;
class UploadElement;
class UploadElementReader;
}

namespace extensions {
Expand All @@ -43,16 +43,16 @@ void AppendKeyValuePair(const char* key,
FORWARD_DECLARE_TEST(WebRequestUploadDataPresenterTest, RawData);

// UploadDataPresenter is an interface for objects capable to consume a series
// of UploadElement and represent this data as a base:Value.
// of UploadElementReader and represent this data as a base:Value.
//
// Workflow for objects implementing this interface:
// 1. Call object->FeedNext(element) for each element from the request's body.
// 1. Call object->FeedNext(reader) for each element from the request's body.
// 2. Check if object->Succeeded().
// 3. If that check passed then retrieve object->Result().
class UploadDataPresenter {
public:
virtual ~UploadDataPresenter();
virtual void FeedNext(const net::UploadElement& element) = 0;
virtual void FeedNext(const net::UploadElementReader& reader) = 0;
virtual bool Succeeded() = 0;
virtual scoped_ptr<base::Value> Result() = 0;

Expand All @@ -63,16 +63,16 @@ class UploadDataPresenter {
DISALLOW_COPY_AND_ASSIGN(UploadDataPresenter);
};

// This class passes all the bytes from elements of TYPE_BYTES as a BinaryValue
// for each such element. Elements of TYPE_FILE are presented as StringValue
// containing the path for that file.
// This class passes all the bytes from bytes elements as a BinaryValue for each
// such element. File elements are presented as StringValue containing the path
// for that file.
class RawDataPresenter : public UploadDataPresenter {
public:
RawDataPresenter();
virtual ~RawDataPresenter();

// Implementation of UploadDataPresenter.
virtual void FeedNext(const net::UploadElement& element) OVERRIDE;
virtual void FeedNext(const net::UploadElementReader& reader) OVERRIDE;
virtual bool Succeeded() OVERRIDE;
virtual scoped_ptr<base::Value> Result() OVERRIDE;

Expand All @@ -88,9 +88,9 @@ class RawDataPresenter : public UploadDataPresenter {
scoped_ptr<base::ListValue> list_;
};

// This class inspects the contents of elements of TYPE_BYTES. It uses the
// This class inspects the contents of bytes elements. It uses the
// parser classes inheriting from FormDataParser to parse the concatenated
// content of such elements. If the parsing is successfull, the parsed form is
// content of such elements. If the parsing is successful, the parsed form is
// returned as a DictionaryValue. For example, a form consisting of
// <input name="check" type="checkbox" value="A" checked />
// <input name="check" type="checkbox" value="B" checked />
Expand All @@ -103,7 +103,7 @@ class ParsedDataPresenter : public UploadDataPresenter {
virtual ~ParsedDataPresenter();

// Implementation of UploadDataPresenter.
virtual void FeedNext(const net::UploadElement& element) OVERRIDE;
virtual void FeedNext(const net::UploadElementReader& reader) OVERRIDE;
virtual bool Succeeded() OVERRIDE;
virtual scoped_ptr<base::Value> Result() OVERRIDE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "base/values.h"
#include "chrome/browser/extensions/api/web_request/upload_data_presenter.h"
#include "chrome/browser/extensions/api/web_request/web_request_api_constants.h"
#include "net/base/upload_element.h"
#include "net/base/upload_bytes_element_reader.h"
#include "testing/gtest/include/gtest/gtest.h"

using base::BinaryValue;
Expand All @@ -24,8 +24,7 @@ namespace extensions {
TEST(WebRequestUploadDataPresenterTest, ParsedData) {
// Input.
const char block[] = "key.with.dots=value";
net::UploadElement element;
element.SetToBytes(block, sizeof(block) - 1);
net::UploadBytesElementReader element(block, sizeof(block) - 1);

// Expected output.
scoped_ptr<ListValue> values(new ListValue);
Expand Down
14 changes: 7 additions & 7 deletions chrome/browser/extensions/api/web_request/web_request_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@
#include "grit/generated_resources.h"
#include "net/base/auth.h"
#include "net/base/net_errors.h"
#include "net/base/upload_data.h"
#include "net/base/upload_element.h"
#include "net/base/upload_data_stream.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_request.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -183,7 +182,7 @@ void ExtractRequestInfo(net::URLRequest* request, DictionaryValue* out) {
// Extracts the body from |request| and writes the data into |out|.
void ExtractRequestInfoBody(const net::URLRequest* request,
DictionaryValue* out) {
const net::UploadData* upload_data = request->get_upload();
const net::UploadDataStream* upload_data = request->get_upload();
if (!upload_data ||
(request->method() != "POST" && request->method() != "PUT"))
return; // Need to exit without "out->Set(keys::kRequestBodyKey, ...);" .
Expand All @@ -204,12 +203,13 @@ void ExtractRequestInfoBody(const net::URLRequest* request,
keys::kRequestBodyRawKey
};

const ScopedVector<net::UploadElement>& elements = upload_data->elements();
const ScopedVector<net::UploadElementReader>& readers =
upload_data->element_readers();
bool some_succeeded = false;
for (size_t i = 0; !some_succeeded && i < arraysize(presenters); ++i) {
ScopedVector<net::UploadElement>::const_iterator element;
for (element = elements.begin(); element != elements.end(); ++element)
presenters[i]->FeedNext(**element);
ScopedVector<net::UploadElementReader>::const_iterator reader;
for (reader = readers.begin(); reader != readers.end(); ++reader)
presenters[i]->FeedNext(**reader);
if (presenters[i]->Succeeded()) {
requestBody->Set(kKeys[i], presenters[i]->Result().release());
some_succeeded = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,11 @@ void ExtensionWebRequestTest::FireURLRequestWithData(
request.SetExtraRequestHeaderByName(net::HttpRequestHeaders::kContentType,
content_type,
true /* overwrite */);
request.AppendBytesToUpload(&(bytes_1[0]), bytes_1.size());
net::UploadData* data = request.get_upload_mutable();
data->AppendFileRange(::FilePath(), 0, 0, base::Time());
request.AppendBytesToUpload(&(bytes_2[0]), bytes_2.size());
scoped_refptr<net::UploadData> upload_data(new net::UploadData());
upload_data->AppendBytes(&(bytes_1[0]), bytes_1.size());
upload_data->AppendFileRange(::FilePath(), 0, 0, base::Time());
upload_data->AppendBytes(&(bytes_2[0]), bytes_2.size());
request.set_upload(upload_data);
ipc_sender_.PushTask(base::Bind(&base::DoNothing));
request.Start();
}
Expand Down
7 changes: 5 additions & 2 deletions chrome/browser/net/chrome_fraudulent_certificate_reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "chrome/browser/net/cert_logger.pb.h"
#include "net/base/load_flags.h"
#include "net/base/ssl_info.h"
#include "net/base/upload_data.h"
#include "net/base/x509_certificate.h"
#include "net/url_request/url_request_context.h"

Expand Down Expand Up @@ -77,7 +78,10 @@ void ChromeFraudulentCertificateReporter::SendReport(

net::URLRequest* url_request = CreateURLRequest(request_context_);
url_request->set_method("POST");
url_request->AppendBytesToUpload(report.data(), report.size());

scoped_refptr<net::UploadData> upload_data(new net::UploadData());
upload_data->AppendBytes(report.data(), report.size());
url_request->set_upload(upload_data);

net::HttpRequestHeaders headers;
headers.SetHeader(net::HttpRequestHeaders::kContentType,
Expand Down Expand Up @@ -117,4 +121,3 @@ void ChromeFraudulentCertificateReporter::OnReadCompleted(
net::URLRequest* request, int bytes_read) {}

} // namespace chrome_browser_net

14 changes: 9 additions & 5 deletions chrome/browser/policy/device_management_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
#include "chrome/browser/policy/cloud_policy_constants.h"
#include "chrome/browser/policy/device_management_service.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "net/base/upload_data.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_data_stream.h"
#include "net/test/test_server.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request.h"
Expand Down Expand Up @@ -45,14 +46,17 @@ class CannedResponseInterceptor : public net::URLRequest::Interceptor {
net::URLRequest* request,
net::NetworkDelegate* network_delegate) OVERRIDE {
em::DeviceManagementRequest dm_request;
net::UploadData* upload = request->get_upload_mutable();
const net::UploadDataStream* upload = request->get_upload();
if (request->url().GetOrigin() == service_url_.GetOrigin() &&
request->url().path() == service_url_.path() &&
upload != NULL &&
upload->elements().size() == 1) {
upload->element_readers().size() == 1 &&
upload->element_readers()[0]->AsBytesReader()) {
std::string response_data;
ConstructResponse(upload->elements()[0]->bytes(),
upload->elements()[0]->bytes_length(),
const net::UploadBytesElementReader* bytes_reader =
upload->element_readers()[0]->AsBytesReader();
ConstructResponse(bytes_reader->bytes(),
bytes_reader->length(),
&response_data);
return new net::URLRequestTestJob(request,
network_delegate,
Expand Down
17 changes: 10 additions & 7 deletions chrome/browser/sessions/better_session_restore_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "net/base/net_util.h"
#include "net/base/upload_data.h"
#include "net/base/upload_element.h"
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_data_stream.h"
#include "net/url_request/url_request.h"
#include "net/url_request/url_request_filter.h"
#include "net/url_request/url_request_test_job.h"
Expand Down Expand Up @@ -57,14 +57,17 @@ net::URLRequestJob* URLRequestFakerForPostRequests(
net::NetworkDelegate* network_delegate,
const std::string& scheme) {
// Read the uploaded data and store it to g_last_upload_bytes.
const net::UploadData* upload_data = request->get_upload();
const net::UploadDataStream* upload_data = request->get_upload();
g_last_upload_bytes.Get().clear();
if (upload_data) {
const ScopedVector<net::UploadElement>& elements = upload_data->elements();
for (size_t i = 0; i < elements.size(); ++i) {
if (elements[i]->type() == net::UploadElement::TYPE_BYTES) {
const ScopedVector<net::UploadElementReader>& readers =
upload_data->element_readers();
for (size_t i = 0; i < readers.size(); ++i) {
const net::UploadBytesElementReader* bytes_reader =
readers[i]->AsBytesReader();
if (bytes_reader) {
g_last_upload_bytes.Get() +=
std::string(elements[i]->bytes(), elements[i]->bytes_length());
std::string(bytes_reader->bytes(), bytes_reader->length());
}
}
}
Expand Down
Loading

0 comments on commit 0736d9e

Please sign in to comment.