Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(64)

Issues Search
    My Issues | Starred     Open | Closed | All

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: Address Helen's comments. 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
« components/cronet/ios/cronet_bidirectional_stream.cc ('K') | « components/cronet/ios/cronet_c_for_grpc.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..da565d6da92239a448c034370d26dcce4ec6ca75 100644
--- a/components/cronet/ios/test/cronet_bidirectional_stream_test.mm
+++ b/components/cronet/ios/test/cronet_bidirectional_stream_test.mm
@@ -10,6 +10,8 @@
#include "base/logging.h"
#include "base/mac/scoped_nsobject.h"
+#include "base/macros.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 +38,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 +51,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 +88,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 +105,24 @@ class TestBidirectionalStreamCallback {
ON_SUCCEEDED
};
+ struct WriteData {
+ std::string buffer;
+ // If |flush| is true, then cronet_bidirectional_stream_flush() will be
+ // called after writing of the |buffer|.
+ bool flush;
+
+ WriteData(const std::string& buffer, bool flush);
+ ~WriteData();
+
+ DISALLOW_COPY_AND_ASSIGN(WriteData);
+ };
+
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 +174,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 +239,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();
- test->MaybeWriteNextData(stream);
+ if (continue_writing)
+ test->MaybeWriteNextData(stream);
}
static void on_response_trailers_received_callback(
@@ -242,6 +263,7 @@ class TestBidirectionalStreamCallback {
static void on_succeded_callback(cronet_bidirectional_stream* stream) {
TestBidirectionalStreamCallback* test = FromStream(stream);
+ ASSERT_TRUE(test->write_data.empty());
test->MaybeCancel(stream, ON_SUCCEEDED);
test->SignalDone();
}
@@ -274,7 +296,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 +310,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 +326,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 +455,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 +472,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 +489,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 +506,7 @@ TEST_F(CronetBidirectionalStreamTest, ReadFailsBeforeRequestStarted) {
cronet_bidirectional_stream_destroy(test.stream);
}
-TEST_F(CronetBidirectionalStreamTest,
+TEST_P(CronetBidirectionalStreamTest,
StreamFailBeforeReadIsExecutedOnNetworkThread) {
class CustomTestBidirectionalStreamCallback
: public TestBidirectionalStreamCallback {
@@ -378,6 +528,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 +538,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 +553,7 @@ TEST_F(CronetBidirectionalStreamTest, WriteFailsBeforeRequestStarted) {
cronet_bidirectional_stream_destroy(test.stream);
}
-TEST_F(CronetBidirectionalStreamTest,
+TEST_P(CronetBidirectionalStreamTest,
StreamFailBeforeWriteIsExecutedOnNetworkThread) {
class CustomTestBidirectionalStreamCallback
: public TestBidirectionalStreamCallback {
@@ -421,19 +575,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 +603,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_bidirectional_stream.cc ('K') | « components/cronet/ios/cronet_c_for_grpc.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698