components/cronet/ios/test/cronet_bidirectional_stream_test.mm - Issue 2050483002: [Cronet] Coalesce small buffers into single QUIC packet in GRPC on iOS.

Unified Diff: components/cronet/ios/test/cronet_bidirectional_stream_test.mm

Issue 2050483002: [Cronet] Coalesce small buffers into single QUIC packet in GRPC on iOS. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Remove flushing_write_data_. Created 4 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: components/cronet/ios/test/cronet_bidirectional_stream_test.mm

diff --git a/components/cronet/ios/test/cronet_bidirectional_stream_test.mm b/components/cronet/ios/test/cronet_bidirectional_stream_test.mm

index 1a26268c551c20bb5ddbc0f477c73c63c5f08ed8..ac74ede1e07e1d4d89bb0cfdc008ac17fcecc6c1 100644

--- a/components/cronet/ios/test/cronet_bidirectional_stream_test.mm

+++ b/components/cronet/ios/test/cronet_bidirectional_stream_test.mm

@@ -10,6 +10,7 @@

#include "base/logging.h"

#include "base/mac/scoped_nsobject.h"

+#include "base/memory/ptr_util.h"

#include "base/strings/string_util.h"

#include "base/strings/sys_string_conversions.h"

#include "base/synchronization/waitable_event.h"

@@ -36,7 +37,7 @@ const cronet_bidirectional_stream_header_array kTestHeadersArray = {

namespace cronet {

-class CronetBidirectionalStreamTest : public ::testing::Test {

+class CronetBidirectionalStreamTest : public ::testing::TestWithParam<bool> {

protected:

CronetBidirectionalStreamTest() {}

~CronetBidirectionalStreamTest() override {}

@@ -49,38 +50,34 @@ class CronetBidirectionalStreamTest : public ::testing::Test {

// during the test, and QuicTestServer not shutting down / restarting

// gracefully.

CronetEnvironment::Initialize();

- }

- StartQuicTestServer();

+ cronet_environment_ = new CronetEnvironment("CronetTest/1.0.0.0");

+ cronet_environment_->set_http2_enabled(true);

+ cronet_environment_->set_quic_enabled(true);

+ cronet_environment_->set_ssl_key_log_file_name("SSLKEYLOGFILE");

- cronet_environment_ = new CronetEnvironment("CronetTest/1.0.0.0");

- cronet_environment_->set_http2_enabled(true);

- cronet_environment_->set_quic_enabled(true);

- cronet_environment_->set_ssl_key_log_file_name("SSLKEYLOGFILE");

+ std::unique_ptr<net::MockCertVerifier> mock_cert_verifier(

+ new net::MockCertVerifier());

+ mock_cert_verifier->set_default_result(net::OK);

- std::unique_ptr<net::MockCertVerifier> mock_cert_verifier(

- new net::MockCertVerifier());

- mock_cert_verifier->set_default_result(net::OK);

+ cronet_environment_->set_cert_verifier(std::move(mock_cert_verifier));

+ cronet_environment_->set_host_resolver_rules(

+ "MAP test.example.com 127.0.0.1,"

+ "MAP notfound.example.com ~NOTFOUND");

+ cronet_environment_->AddQuicHint(kTestServerDomain, kTestServerPort,

+ kTestServerPort);

- cronet_environment_->set_cert_verifier(std::move(mock_cert_verifier));

- cronet_environment_->set_host_resolver_rules(

- "MAP test.example.com 127.0.0.1,"

- "MAP notfound.example.com ~NOTFOUND");

- cronet_environment_->AddQuicHint(kTestServerDomain, kTestServerPort,

- kTestServerPort);

+ cronet_environment_->Start();

- cronet_environment_->Start();

- cronet_engine_.obj = cronet_environment_;

+ cronet_engine_.obj = cronet_environment_;

+ }

+ StartQuicTestServer();

cronet_environment_->StartNetLog("cronet_netlog.json", true);

}

void TearDown() override {

ShutdownQuicTestServer();

cronet_environment_->StopNetLog();

- //[CronetEngine stopNetLog];

- //[CronetEngine uninstall];

}

cronet_engine* engine() { return &cronet_engine_; }

@@ -90,8 +87,8 @@ class CronetBidirectionalStreamTest : public ::testing::Test {

static cronet_engine cronet_engine_;

};

-CronetEnvironment* CronetBidirectionalStreamTest::cronet_environment_;

-cronet_engine CronetBidirectionalStreamTest::cronet_engine_;

+CronetEnvironment* CronetBidirectionalStreamTest::cronet_environment_ = nullptr;

+cronet_engine CronetBidirectionalStreamTest::cronet_engine_ = {0};

class TestBidirectionalStreamCallback {

public:

@@ -107,12 +104,22 @@ class TestBidirectionalStreamCallback {

ON_SUCCEEDED

};

+ struct WriteData {

+ std::string buffer;

+ bool flush;

xunjieli 2016/06/14 14:29:35 Maybe add a brief comment on what |flush| is.

mef 2016/06/14 19:54:31 Done.

+ WriteData(const std::string& buffer, bool flush);

+ ~WriteData();

+ DISALLOW_COPY_AND_ASSIGN(WriteData);

xunjieli 2016/06/14 14:29:35 include macros.h?

mef 2016/06/14 19:54:30 Done.

+ };

cronet_bidirectional_stream* stream;

base::WaitableEvent stream_done_event;

// Test parameters.

std::map<std::string, std::string> request_headers;

- std::list<std::string> write_data;

+ std::list<std::unique_ptr<WriteData>> write_data;

std::string expected_negotiated_protocol;

ResponseStep cancel_from_step;

size_t read_buffer_size;

@@ -164,15 +171,24 @@ class TestBidirectionalStreamCallback {

void BlockForDone() { stream_done_event.Wait(); }

- void AddWriteData(const std::string& data) { write_data.push_back(data); }

+ void AddWriteData(const std::string& data) { AddWriteData(data, true); }

+ void AddWriteData(const std::string& data, bool flush) {

+ write_data.push_back(base::WrapUnique(new WriteData(data, flush)));

+ }

virtual void MaybeWriteNextData(cronet_bidirectional_stream* stream) {

DCHECK_EQ(stream, this->stream);

if (write_data.empty())

return;

- cronet_bidirectional_stream_write(stream, write_data.front().c_str(),

- write_data.front().size(),

- write_data.size() == 1);

+ for (const auto& data : write_data) {

+ cronet_bidirectional_stream_write(stream, data->buffer.c_str(),

+ data->buffer.size(),

+ data == write_data.back());

+ if (data->flush) {

+ cronet_bidirectional_stream_flush(stream);

+ break;

+ }

}

cronet_bidirectional_stream_callback* callback() const { return &s_callback; }

@@ -220,11 +236,13 @@ class TestBidirectionalStreamCallback {

static void on_write_completed_callback(cronet_bidirectional_stream* stream,

const char* data) {

TestBidirectionalStreamCallback* test = FromStream(stream);

- ASSERT_EQ(test->write_data.front().c_str(), data);

+ ASSERT_EQ(test->write_data.front()->buffer.c_str(), data);

if (test->MaybeCancel(stream, ON_WRITE_COMPLETED))

return;

+ bool continue_writing = test->write_data.front()->flush;

test->write_data.pop_front();

xunjieli 2016/06/14 14:29:35 Suggest that we also check |data| is the same as t

mef 2016/06/14 19:54:31 Hrm, is it different than ASSERT_EQ(test->write_da

xunjieli 2016/06/14 19:56:12 Acknowledged. Sorry, I missed that part.

- test->MaybeWriteNextData(stream);

+ if (continue_writing)

+ test->MaybeWriteNextData(stream);

}

static void on_response_trailers_received_callback(

@@ -274,7 +292,13 @@ cronet_bidirectional_stream_callback

on_failed_callback,

on_canceled_callback};

-TEST_F(CronetBidirectionalStreamTest, StartExampleBidiStream) {

+TestBidirectionalStreamCallback::WriteData::WriteData(const std::string& data,

+ bool flush_after)

+ : buffer(data), flush(flush_after) {}

+TestBidirectionalStreamCallback::WriteData::~WriteData() {}

+TEST_P(CronetBidirectionalStreamTest, StartExampleBidiStream) {

TestBidirectionalStreamCallback test;

test.AddWriteData("Hello, ");

test.AddWriteData("world!");

@@ -282,6 +306,8 @@ TEST_F(CronetBidirectionalStreamTest, StartExampleBidiStream) {

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, false);

test.BlockForDone();

@@ -296,11 +322,125 @@ TEST_F(CronetBidirectionalStreamTest, StartExampleBidiStream) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, CancelOnRead) {

+TEST_P(CronetBidirectionalStreamTest, SimpleGetWithFlush) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_disable_auto_flush(test.stream, true);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

+ // Flush before start is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "GET",

+ &kTestHeadersArray, true);

+ test.BlockForDone();

+ ASSERT_EQ(std::string(kHelloStatus), test.response_headers[kStatusHeader]);

+ ASSERT_EQ(std::string(kHelloHeaderValue),

+ test.response_headers[kHelloHeaderName]);

+ ASSERT_EQ(TestBidirectionalStreamCallback::ON_SUCCEEDED, test.response_step);

+ ASSERT_EQ(std::string(kHelloBodyValue), base::JoinString(test.read_data, ""));

+ ASSERT_EQ(std::string(kHelloTrailerValue),

+ test.response_trailers[kHelloTrailerName]);

+ // Flush after done is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_destroy(test.stream);

+TEST_P(CronetBidirectionalStreamTest, SimplePostWithFlush) {

+ TestBidirectionalStreamCallback test;

+ test.AddWriteData("Test String", false);

+ test.AddWriteData("1234567890", false);

+ test.AddWriteData("woot!", true);

+ test.stream =

+ cronet_bidirectional_stream_create(engine(), &test, test.callback());

+ DCHECK(test.stream);

+ cronet_bidirectional_stream_disable_auto_flush(test.stream, true);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

+ // Flush before start is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

+ &kTestHeadersArray, false);

+ test.BlockForDone();

+ ASSERT_EQ(std::string(kHelloStatus), test.response_headers[kStatusHeader]);

+ ASSERT_EQ(std::string(kHelloHeaderValue),

+ test.response_headers[kHelloHeaderName]);

+ ASSERT_EQ(TestBidirectionalStreamCallback::ON_SUCCEEDED, test.response_step);

+ ASSERT_EQ(std::string(kHelloBodyValue), base::JoinString(test.read_data, ""));

+ ASSERT_EQ(std::string(kHelloTrailerValue),

+ test.response_trailers[kHelloTrailerName]);

+ // Flush after done is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_destroy(test.stream);

+TEST_P(CronetBidirectionalStreamTest, SimplePostWithFlushTwice) {

+ TestBidirectionalStreamCallback test;

+ test.AddWriteData("Test String", false);

+ test.AddWriteData("1234567890", false);

+ test.AddWriteData("woot!", true);

+ test.AddWriteData("Test String", false);

+ test.AddWriteData("1234567890", false);

+ test.AddWriteData("woot!", true);

+ test.stream =

+ cronet_bidirectional_stream_create(engine(), &test, test.callback());

+ DCHECK(test.stream);

+ cronet_bidirectional_stream_disable_auto_flush(test.stream, true);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

+ // Flush before start is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

+ &kTestHeadersArray, false);

+ test.BlockForDone();

+ ASSERT_EQ(std::string(kHelloStatus), test.response_headers[kStatusHeader]);

+ ASSERT_EQ(std::string(kHelloHeaderValue),

+ test.response_headers[kHelloHeaderName]);

+ ASSERT_EQ(TestBidirectionalStreamCallback::ON_SUCCEEDED, test.response_step);

+ ASSERT_EQ(std::string(kHelloBodyValue), base::JoinString(test.read_data, ""));

+ ASSERT_EQ(std::string(kHelloTrailerValue),

+ test.response_trailers[kHelloTrailerName]);

+ // Flush after done is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_destroy(test.stream);

+TEST_P(CronetBidirectionalStreamTest, SimplePostWithFlushAfterOneWrite) {

+ TestBidirectionalStreamCallback test;

+ test.AddWriteData("Test String", false);

+ test.AddWriteData("1234567890", false);

+ test.AddWriteData("woot!", true);

+ test.stream =

+ cronet_bidirectional_stream_create(engine(), &test, test.callback());

+ DCHECK(test.stream);

+ cronet_bidirectional_stream_disable_auto_flush(test.stream, true);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

+ // Flush before start is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

+ &kTestHeadersArray, false);

+ test.BlockForDone();

+ ASSERT_EQ(std::string(kHelloStatus), test.response_headers[kStatusHeader]);

+ ASSERT_EQ(std::string(kHelloHeaderValue),

+ test.response_headers[kHelloHeaderName]);

+ ASSERT_EQ(TestBidirectionalStreamCallback::ON_SUCCEEDED, test.response_step);

+ ASSERT_EQ(std::string(kHelloBodyValue), base::JoinString(test.read_data, ""));

+ ASSERT_EQ(std::string(kHelloTrailerValue),

+ test.response_trailers[kHelloTrailerName]);

+ // Flush after done is ignored.

+ cronet_bidirectional_stream_flush(test.stream);

+ cronet_bidirectional_stream_destroy(test.stream);

+TEST_P(CronetBidirectionalStreamTest, CancelOnRead) {

+ TestBidirectionalStreamCallback test;

+ test.stream =

+ cronet_bidirectional_stream_create(engine(), &test, test.callback());

+ DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

test.cancel_from_step = TestBidirectionalStreamCallback::ON_READ_COMPLETED;

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, true);

@@ -311,11 +451,13 @@ TEST_F(CronetBidirectionalStreamTest, CancelOnRead) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, CancelOnResponse) {

+TEST_P(CronetBidirectionalStreamTest, CancelOnResponse) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

test.cancel_from_step = TestBidirectionalStreamCallback::ON_RESPONSE_STARTED;

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, true);

@@ -326,11 +468,13 @@ TEST_F(CronetBidirectionalStreamTest, CancelOnResponse) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, CancelOnSucceeded) {

+TEST_P(CronetBidirectionalStreamTest, CancelOnSucceeded) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

test.cancel_from_step = TestBidirectionalStreamCallback::ON_SUCCEEDED;

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, true);

@@ -341,11 +485,13 @@ TEST_F(CronetBidirectionalStreamTest, CancelOnSucceeded) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, ReadFailsBeforeRequestStarted) {

+TEST_P(CronetBidirectionalStreamTest, ReadFailsBeforeRequestStarted) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

char read_buffer[1];

cronet_bidirectional_stream_read(test.stream, read_buffer,

sizeof(read_buffer));

@@ -356,7 +502,7 @@ TEST_F(CronetBidirectionalStreamTest, ReadFailsBeforeRequestStarted) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest,

+TEST_P(CronetBidirectionalStreamTest,

StreamFailBeforeReadIsExecutedOnNetworkThread) {

class CustomTestBidirectionalStreamCallback

: public TestBidirectionalStreamCallback {

@@ -378,6 +524,8 @@ TEST_F(CronetBidirectionalStreamTest,

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, false);

test.BlockForDone();

@@ -386,11 +534,13 @@ TEST_F(CronetBidirectionalStreamTest,

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, WriteFailsBeforeRequestStarted) {

+TEST_P(CronetBidirectionalStreamTest, WriteFailsBeforeRequestStarted) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

cronet_bidirectional_stream_write(test.stream, "1", 1, false);

test.BlockForDone();

ASSERT_TRUE(test.read_data.empty());

@@ -399,7 +549,7 @@ TEST_F(CronetBidirectionalStreamTest, WriteFailsBeforeRequestStarted) {

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest,

+TEST_P(CronetBidirectionalStreamTest,

StreamFailBeforeWriteIsExecutedOnNetworkThread) {

class CustomTestBidirectionalStreamCallback

: public TestBidirectionalStreamCallback {

@@ -421,19 +571,24 @@ TEST_F(CronetBidirectionalStreamTest,

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

cronet_bidirectional_stream_start(test.stream, kTestServerUrl, 0, "POST",

&kTestHeadersArray, false);

test.BlockForDone();

ASSERT_EQ(TestBidirectionalStreamCallback::ON_FAILED, test.response_step);

- ASSERT_EQ(net::ERR_QUIC_PROTOCOL_ERROR, test.net_error);

+ ASSERT_TRUE(test.net_error == net::ERR_QUIC_PROTOCOL_ERROR ||

+ test.net_error == net::ERR_QUIC_HANDSHAKE_FAILED);

cronet_bidirectional_stream_destroy(test.stream);

}

-TEST_F(CronetBidirectionalStreamTest, FailedResolution) {

+TEST_P(CronetBidirectionalStreamTest, FailedResolution) {

TestBidirectionalStreamCallback test;

test.stream =

cronet_bidirectional_stream_create(engine(), &test, test.callback());

DCHECK(test.stream);

+ cronet_bidirectional_stream_delay_request_headers_until_flush(test.stream,

+ GetParam());

test.cancel_from_step = TestBidirectionalStreamCallback::ON_FAILED;

cronet_bidirectional_stream_start(test.stream, "https://notfound.example.com",

0, "GET", &kTestHeadersArray, true);

@@ -444,4 +599,8 @@ TEST_F(CronetBidirectionalStreamTest, FailedResolution) {

cronet_bidirectional_stream_destroy(test.stream);

}

+INSTANTIATE_TEST_CASE_P(CronetBidirectionalStreamDelayRequestHeadersUntilFlush,

+ CronetBidirectionalStreamTest,

+ ::testing::Values(true, false));

} // namespace cronet

« components/cronet/ios/cronet_c_for_grpc.h ('K') | « components/cronet/ios/cronet_c_for_grpc.cc ('k') | no next file » | no next file with comments »