Skip to content

Commit

Permalink
Add net log to UploadDataStream.
Browse files Browse the repository at this point in the history
Improve the granularity of net-internals logging for upload
to be able to discover where the hang is occurring in
situations like in the issue 611436.

BUG=613337

Review-Url: https://codereview.chromium.org/2227503003
Cr-Commit-Position: refs/heads/master@{#412474}
  • Loading branch information
maksim.sisov authored and Commit bot committed Aug 17, 2016
1 parent de23c50 commit 819ba85
Show file tree
Hide file tree
Showing 23 changed files with 267 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ class DevToolsNetworkControllerHelper {
}

int ReadUploadData() {
EXPECT_EQ(net::OK,
transaction_->custom_upload_data_stream_->Init(completion_callback_));
EXPECT_EQ(net::OK, transaction_->custom_upload_data_stream_->Init(
completion_callback_, net::BoundNetLog()));
return transaction_->custom_upload_data_stream_->Read(
buffer_.get(), 64, completion_callback_);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ bool DevToolsNetworkUploadDataStream::IsInMemory() const {
return false;
}

int DevToolsNetworkUploadDataStream::InitInternal() {
int DevToolsNetworkUploadDataStream::InitInternal(
const net::BoundNetLog& net_log) {
throttled_byte_count_ = 0;
int result = upload_data_stream_->Init(
base::Bind(&DevToolsNetworkUploadDataStream::StreamInitCallback,
base::Unretained(this)));
base::Unretained(this)),
net_log);
if (result == net::OK && !is_chunked())
SetSize(upload_data_stream_->size());
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class DevToolsNetworkUploadDataStream : public net::UploadDataStream {
private:
// net::UploadDataStream implementation.
bool IsInMemory() const override;
int InitInternal() override;
int InitInternal(const net::BoundNetLog& net_log) override;
int ReadInternal(net::IOBuffer* buf, int buf_len) override;
void ResetInternal() override;

Expand Down
2 changes: 1 addition & 1 deletion components/cronet/android/cronet_upload_data_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ CronetUploadDataStream::~CronetUploadDataStream() {
delegate_->OnUploadDataStreamDestroyed();
}

int CronetUploadDataStream::InitInternal() {
int CronetUploadDataStream::InitInternal(const net::BoundNetLog& net_log) {
// ResetInternal should have been called before init, if the stream was in
// use.
DCHECK(!waiting_on_read_);
Expand Down
2 changes: 1 addition & 1 deletion components/cronet/android/cronet_upload_data_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class CronetUploadDataStream : public net::UploadDataStream {

private:
// net::UploadDataStream implementation:
int InitInternal() override;
int InitInternal(const net::BoundNetLog& net_log) override;
int ReadInternal(net::IOBuffer* buf, int buf_len) override;
void ResetInternal() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ void TestUploadDataStreamHandler::InitOnNetworkThread() {
init_callback_invoked_ = false;
read_buffer_ = nullptr;
bytes_read_ = 0;
int res = upload_data_stream_->Init(base::Bind(
&TestUploadDataStreamHandler::OnInitCompleted, base::Unretained(this)));
int res = upload_data_stream_->Init(
base::Bind(&TestUploadDataStreamHandler::OnInitCompleted,
base::Unretained(this)),
net::BoundNetLog());
JNIEnv* env = base::android::AttachCurrentThread();
cronet::Java_TestUploadDataStreamHandler_onInitCalled(
env, jtest_upload_data_stream_handler_, res);
Expand Down
3 changes: 2 additions & 1 deletion components/domain_reliability/uploader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class UploadMockURLRequestJob : public net::URLRequestJob {
void Start() override {
int rv = upload_stream_->Init(
base::Bind(&UploadMockURLRequestJob::OnStreamInitialized,
base::Unretained(this)));
base::Unretained(this)),
net::BoundNetLog());
if (rv == net::ERR_IO_PENDING)
return;
OnStreamInitialized(rv);
Expand Down
5 changes: 3 additions & 2 deletions content/browser/loader/resource_loader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,11 @@ class NonChunkedUploadDataStream : public net::UploadDataStream {
}

private:
int InitInternal() override {
int InitInternal(const net::BoundNetLog& net_log) override {
SetSize(size_);
stream_.Init(base::Bind(&NonChunkedUploadDataStream::OnInitCompleted,
base::Unretained(this)));
base::Unretained(this)),
net_log);
return net::OK;
}

Expand Down
9 changes: 6 additions & 3 deletions content/browser/loader/upload_data_stream_builder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ TEST(UploadDataStreamBuilderTest,
ASSERT_EQ(3U, readers.size());

net::TestCompletionCallback init_callback;
ASSERT_EQ(net::ERR_IO_PENDING, upload->Init(init_callback.callback()));
ASSERT_EQ(net::ERR_IO_PENDING,
upload->Init(init_callback.callback(), net::BoundNetLog()));
EXPECT_EQ(net::OK, init_callback.WaitForResult());

EXPECT_EQ(kZeroLength, upload->size());
Expand Down Expand Up @@ -183,7 +184,8 @@ TEST(UploadDataStreamBuilderTest, ResetUploadStreamWithBlob) {
base::ThreadTaskRunnerHandle::Get().get()));

net::TestCompletionCallback init_callback;
ASSERT_EQ(net::OK, upload->Init(init_callback.callback()));
ASSERT_EQ(net::OK,
upload->Init(init_callback.callback(), net::BoundNetLog()));

// Read part of the data.
const int kBufferLength = 4;
Expand All @@ -197,7 +199,8 @@ TEST(UploadDataStreamBuilderTest, ResetUploadStreamWithBlob) {
std::memcmp(kBlobData.c_str(), buffer->data(), buffer->size()));

// Reset.
ASSERT_EQ(net::OK, upload->Init(init_callback.callback()));
ASSERT_EQ(net::OK,
upload->Init(init_callback.callback(), net::BoundNetLog()));

// Read all the data.
buffer = new net::IOBufferWithSize(kBlobDataLength);
Expand Down
2 changes: 1 addition & 1 deletion net/base/chunked_upload_data_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void ChunkedUploadDataStream::AppendData(
OnReadCompleted(result);
}

int ChunkedUploadDataStream::InitInternal() {
int ChunkedUploadDataStream::InitInternal(const BoundNetLog& net_log) {
// ResetInternal should already have been called.
DCHECK(!read_buffer_.get());
DCHECK_EQ(0u, read_index_);
Expand Down
2 changes: 1 addition & 1 deletion net/base/chunked_upload_data_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class NET_EXPORT ChunkedUploadDataStream : public UploadDataStream {

private:
// UploadDataStream implementation.
int InitInternal() override;
int InitInternal(const BoundNetLog& net_log) override;
int ReadInternal(IOBuffer* buf, int buf_len) override;
void ResetInternal() override;

Expand Down
42 changes: 28 additions & 14 deletions net/base/chunked_upload_data_stream_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ std::string ReadSync(UploadDataStream* stream, int buffer_size) {
TEST(ChunkedUploadDataStreamTest, AppendOnce) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -66,7 +67,8 @@ TEST(ChunkedUploadDataStreamTest, AppendOnce) {
TEST(ChunkedUploadDataStreamTest, AppendOnceBeforeRead) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -88,7 +90,8 @@ TEST(ChunkedUploadDataStreamTest, AppendOnceBeforeInit) {
ChunkedUploadDataStream stream(0);

stream.AppendData(kTestData, kTestDataSize, true);
ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -104,7 +107,8 @@ TEST(ChunkedUploadDataStreamTest, AppendOnceBeforeInit) {
TEST(ChunkedUploadDataStreamTest, MultipleAppends) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size());
EXPECT_EQ(0u, stream.position());
Expand Down Expand Up @@ -133,7 +137,8 @@ TEST(ChunkedUploadDataStreamTest, MultipleAppends) {
TEST(ChunkedUploadDataStreamTest, MultipleAppendsBetweenReads) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand Down Expand Up @@ -162,7 +167,8 @@ TEST(ChunkedUploadDataStreamTest, MultipleAppendsBeforeInit) {
stream.AppendData(kTestData + 1, 1, false);
stream.AppendData(kTestData + 2, kTestDataSize - 2, true);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -184,7 +190,8 @@ TEST(ChunkedUploadDataStreamTest, MultipleReads) {
stream.AppendData(kTestData, kTestDataSize, false);
stream.AppendData(kTestData, kTestDataSize, true);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand Down Expand Up @@ -214,7 +221,8 @@ TEST(ChunkedUploadDataStreamTest, MultipleReads) {
TEST(ChunkedUploadDataStreamTest, EmptyUpload) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -236,7 +244,8 @@ TEST(ChunkedUploadDataStreamTest, EmptyUploadEndedBeforeInit) {
ChunkedUploadDataStream stream(0);
stream.AppendData(NULL, 0, true);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -253,7 +262,8 @@ TEST(ChunkedUploadDataStreamTest, RewindAfterComplete) {
stream.AppendData(kTestData, 1, false);
stream.AppendData(kTestData + 1, kTestDataSize - 1, true);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -265,7 +275,8 @@ TEST(ChunkedUploadDataStreamTest, RewindAfterComplete) {
ASSERT_TRUE(stream.IsEOF());

// Rewind stream and repeat.
ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -280,7 +291,8 @@ TEST(ChunkedUploadDataStreamTest, RewindAfterComplete) {
TEST(ChunkedUploadDataStreamTest, RewindWhileReading) {
ChunkedUploadDataStream stream(0);

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand All @@ -291,7 +303,8 @@ TEST(ChunkedUploadDataStreamTest, RewindWhileReading) {
int result = stream.Read(buf.get(), kTestBufferSize, callback.callback());
ASSERT_THAT(result, IsError(ERR_IO_PENDING));

ASSERT_THAT(stream.Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream.Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());
EXPECT_FALSE(stream.IsInMemory());
EXPECT_EQ(0u, stream.size()); // Content-Length is 0 for chunked data.
EXPECT_EQ(0u, stream.position());
Expand Down Expand Up @@ -319,7 +332,8 @@ TEST(ChunkedUploadDataStreamTest, ChunkedUploadDataStreamWriter) {

// Write before Init.
ASSERT_TRUE(writer->AppendData(kTestData, 1, false));
ASSERT_THAT(stream->Init(TestCompletionCallback().callback()), IsOk());
ASSERT_THAT(stream->Init(TestCompletionCallback().callback(), BoundNetLog()),
IsOk());

// Write after Init.
ASSERT_TRUE(writer->AppendData(kTestData + 1, kTestDataSize - 1, false));
Expand Down
2 changes: 1 addition & 1 deletion net/base/elements_upload_data_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ std::unique_ptr<UploadDataStream> ElementsUploadDataStream::CreateWithReader(
new ElementsUploadDataStream(std::move(readers), identifier));
}

int ElementsUploadDataStream::InitInternal() {
int ElementsUploadDataStream::InitInternal(const BoundNetLog& net_log) {
return InitElements(0);
}

Expand Down
2 changes: 1 addition & 1 deletion net/base/elements_upload_data_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class NET_EXPORT ElementsUploadDataStream : public UploadDataStream {
bool IsInMemory() const override;
const std::vector<std::unique_ptr<UploadElementReader>>* GetElementReaders()
const override;
int InitInternal() override;
int InitInternal(const net::BoundNetLog& net_log) override;
int ReadInternal(IOBuffer* buf, int buf_len) override;
void ResetInternal() override;

Expand Down
Loading

0 comments on commit 819ba85

Please sign in to comment.