From c71047a7b146278ef5d83b9a0a9babbc86e9b382 Mon Sep 17 00:00:00 2001 From: "akalin@chromium.org" Date: Thu, 14 Mar 2013 03:02:33 +0000 Subject: [SPDY] Refactor use of stream delegates in SpdySession tests This is a follow-up to 187148; I missed a few delegates to audit. BUG=178943 Review URL: https://codereview.chromium.org/12623010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187992 0039d316-1c4b-4281-b951-d872f2087c98 --- net/spdy/spdy_session_spdy2_unittest.cc | 101 +++++-------------------------- net/spdy/spdy_session_spdy3_unittest.cc | 103 +++++--------------------------- net/spdy/spdy_stream_test_util.cc | 36 +++++++++++ net/spdy/spdy_stream_test_util.h | 23 +++++++ 4 files changed, 89 insertions(+), 174 deletions(-) (limited to 'net') diff --git a/net/spdy/spdy_session_spdy2_unittest.cc b/net/spdy/spdy_session_spdy2_unittest.cc index 65cc61f..5127ec4 100644 --- a/net/spdy/spdy_session_spdy2_unittest.cc +++ b/net/spdy/spdy_session_spdy2_unittest.cc @@ -17,6 +17,7 @@ #include "net/spdy/spdy_session_pool.h" #include "net/spdy/spdy_session_test_util.h" #include "net/spdy/spdy_stream.h" +#include "net/spdy/spdy_stream_test_util.h" #include "net/spdy/spdy_test_util_common.h" #include "net/spdy/spdy_test_util_spdy2.h" #include "testing/platform_test.h" @@ -36,76 +37,6 @@ base::TimeTicks TheNearFuture() { return base::TimeTicks::Now() + base::TimeDelta::FromSeconds(g_delta_seconds); } -class ClosingDelegate : public SpdyStream::Delegate { - public: - ClosingDelegate(SpdyStream* stream) : stream_(stream) {} - - // SpdyStream::Delegate implementation: - virtual bool OnSendHeadersComplete(int status) OVERRIDE { - return true; - } - virtual int OnSendBody() OVERRIDE { - return OK; - } - virtual int OnSendBodyComplete(int status, bool* eof) OVERRIDE { - return OK; - } - virtual int OnResponseReceived(const SpdyHeaderBlock& response, - base::Time response_time, - int status) OVERRIDE { - return OK; - } - virtual void OnHeadersSent() OVERRIDE {} - virtual int OnDataReceived(const char* data, int length) OVERRIDE { - return OK; - } - virtual void OnDataSent(int length) OVERRIDE {} - virtual void OnClose(int status) OVERRIDE { - stream_->Close(); - } - private: - SpdyStream* stream_; -}; - -class TestSpdyStreamDelegate : public SpdyStream::Delegate { - public: - explicit TestSpdyStreamDelegate(const CompletionCallback& callback) - : callback_(callback) {} - virtual ~TestSpdyStreamDelegate() {} - - virtual bool OnSendHeadersComplete(int status) OVERRIDE { return true; } - - virtual int OnSendBody() OVERRIDE { - return ERR_UNEXPECTED; - } - - virtual int OnSendBodyComplete(int /*status*/, bool* /*eof*/) OVERRIDE { - return ERR_UNEXPECTED; - } - - virtual int OnResponseReceived(const SpdyHeaderBlock& response, - base::Time response_time, - int status) OVERRIDE { - return status; - } - - virtual void OnHeadersSent() OVERRIDE {} - virtual int OnDataReceived(const char* buffer, int bytes) OVERRIDE { - return OK; - } - - virtual void OnDataSent(int length) OVERRIDE {} - - virtual void OnClose(int status) OVERRIDE { - CompletionCallback callback = callback_; - callback_.Reset(); - callback.Run(OK); - } - - private: - CompletionCallback callback_; -}; - } // namespace class SpdySessionSpdy2Test : public PlatformTest { @@ -276,19 +207,19 @@ TEST_F(SpdySessionSpdy2Test, ClientPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); base::TimeTicks before_ping_time = base::TimeTicks::Now(); - session->set_connection_at_risk_of_loss_time(base::TimeDelta::FromSeconds(0)); + session->set_connection_at_risk_of_loss_time( + base::TimeDelta::FromSeconds(-1)); session->set_hung_interval(base::TimeDelta::FromMilliseconds(50)); session->SendPrefacePingIfNoneInFlight(); - EXPECT_EQ(OK, callback1.WaitForResult()); + EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose()); session->CheckPingStatus(before_ping_time); @@ -331,10 +262,9 @@ TEST_F(SpdySessionSpdy2Test, ServerPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); // Flush the SpdySession::OnReadComplete() task. MessageLoop::current()->RunUntilIdle(); @@ -423,10 +353,9 @@ TEST_F(SpdySessionSpdy2Test, FailedPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); session->set_connection_at_risk_of_loss_time(base::TimeDelta::FromSeconds(0)); session->set_hung_interval(base::TimeDelta::FromSeconds(0)); @@ -1160,12 +1089,12 @@ TEST_F(SpdySessionSpdy2Test, CloseSessionWithTwoCreatedStreams) { spdy_stream1->set_spdy_headers(headers.Pass()); EXPECT_TRUE(spdy_stream1->HasUrl()); - ClosingDelegate delegate1(spdy_stream1.get()); + test::ClosingDelegate delegate1(spdy_stream1.get()); spdy_stream1->SetDelegate(&delegate1); spdy_stream2->set_spdy_headers(headers2.Pass()); EXPECT_TRUE(spdy_stream2->HasUrl()); - ClosingDelegate delegate2(spdy_stream2.get()); + test::ClosingDelegate delegate2(spdy_stream2.get()); spdy_stream2->SetDelegate(&delegate2); spdy_stream1->SendRequest(false); diff --git a/net/spdy/spdy_session_spdy3_unittest.cc b/net/spdy/spdy_session_spdy3_unittest.cc index 6daed66..5418528 100644 --- a/net/spdy/spdy_session_spdy3_unittest.cc +++ b/net/spdy/spdy_session_spdy3_unittest.cc @@ -41,77 +41,7 @@ base::TimeTicks TheNearFuture() { return base::TimeTicks::Now() + base::TimeDelta::FromSeconds(g_delta_seconds); } -class ClosingDelegate : public SpdyStream::Delegate { - public: - ClosingDelegate(SpdyStream* stream) : stream_(stream) {} - - // SpdyStream::Delegate implementation: - virtual bool OnSendHeadersComplete(int status) OVERRIDE { - return true; - } - virtual int OnSendBody() OVERRIDE { - return OK; - } - virtual int OnSendBodyComplete(int status, bool* eof) OVERRIDE { - return OK; - } - virtual int OnResponseReceived(const SpdyHeaderBlock& response, - base::Time response_time, - int status) OVERRIDE { - return OK; - } - virtual void OnHeadersSent() OVERRIDE {} - virtual int OnDataReceived(const char* data, int length) OVERRIDE { - return OK; - } - virtual void OnDataSent(int length) OVERRIDE {} - virtual void OnClose(int status) OVERRIDE { - stream_->Close(); - } - private: - SpdyStream* stream_; -}; - -class TestSpdyStreamDelegate : public SpdyStream::Delegate { - public: - explicit TestSpdyStreamDelegate(const CompletionCallback& callback) - : callback_(callback) {} - virtual ~TestSpdyStreamDelegate() {} - - virtual bool OnSendHeadersComplete(int status) OVERRIDE { return true; } - - virtual int OnSendBody() OVERRIDE { - return ERR_UNEXPECTED; - } - - virtual int OnSendBodyComplete(int /*status*/, bool* /*eof*/) OVERRIDE { - return ERR_UNEXPECTED; - } - - virtual int OnResponseReceived(const SpdyHeaderBlock& response, - base::Time response_time, - int status) OVERRIDE { - return status; - } - - virtual void OnHeadersSent() OVERRIDE {} - virtual int OnDataReceived(const char* buffer, int bytes) OVERRIDE { - return OK; - } - - virtual void OnDataSent(int length) OVERRIDE {} - - virtual void OnClose(int status) OVERRIDE { - CompletionCallback callback = callback_; - callback_.Reset(); - callback.Run(OK); - } - - private: - CompletionCallback callback_; -}; - -} // namespace +} // namespace class SpdySessionSpdy3Test : public PlatformTest { protected: @@ -281,20 +211,19 @@ TEST_F(SpdySessionSpdy3Test, ClientPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); base::TimeTicks before_ping_time = base::TimeTicks::Now(); - session->set_connection_at_risk_of_loss_time(base::TimeDelta::FromSeconds(0)); + session->set_connection_at_risk_of_loss_time( + base::TimeDelta::FromSeconds(-1)); session->set_hung_interval(base::TimeDelta::FromMilliseconds(50)); session->SendPrefacePingIfNoneInFlight(); - EXPECT_EQ(OK, callback1.WaitForResult()); + EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose()); session->CheckPingStatus(before_ping_time); @@ -337,10 +266,9 @@ TEST_F(SpdySessionSpdy3Test, ServerPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); // Flush the SpdySession::OnReadComplete() task. MessageLoop::current()->RunUntilIdle(); @@ -433,10 +361,9 @@ TEST_F(SpdySessionSpdy3Test, FailedPing) { scoped_refptr spdy_stream1 = CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog()); ASSERT_TRUE(spdy_stream1.get() != NULL); - TestCompletionCallback callback1; - scoped_ptr delegate( - new TestSpdyStreamDelegate(callback1.callback())); - spdy_stream1->SetDelegate(delegate.get()); + test::StreamDelegateSendImmediate delegate( + spdy_stream1, scoped_ptr(), NULL); + spdy_stream1->SetDelegate(&delegate); session->set_connection_at_risk_of_loss_time(base::TimeDelta::FromSeconds(0)); session->set_hung_interval(base::TimeDelta::FromSeconds(0)); @@ -1177,12 +1104,12 @@ TEST_F(SpdySessionSpdy3Test, CloseSessionWithTwoCreatedStreams) { spdy_stream1->set_spdy_headers(headers.Pass()); EXPECT_TRUE(spdy_stream1->HasUrl()); - ClosingDelegate delegate1(spdy_stream1.get()); + test::ClosingDelegate delegate1(spdy_stream1.get()); spdy_stream1->SetDelegate(&delegate1); spdy_stream2->set_spdy_headers(headers2.Pass()); EXPECT_TRUE(spdy_stream2->HasUrl()); - ClosingDelegate delegate2(spdy_stream2.get()); + test::ClosingDelegate delegate2(spdy_stream2.get()); spdy_stream2->SetDelegate(&delegate2); spdy_stream1->SendRequest(false); diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc index d72e85f..c608dbc 100644 --- a/net/spdy/spdy_stream_test_util.cc +++ b/net/spdy/spdy_stream_test_util.cc @@ -12,6 +12,42 @@ namespace net { namespace test { +ClosingDelegate::ClosingDelegate( + const scoped_refptr& stream) : stream_(stream) {} + +ClosingDelegate::~ClosingDelegate() {} + +bool ClosingDelegate::OnSendHeadersComplete(int status) { + return true; +} + +int ClosingDelegate::OnSendBody() { + return OK; +} + +int ClosingDelegate::OnSendBodyComplete(int status, bool* eof) { + return OK; +} +int ClosingDelegate::OnResponseReceived(const SpdyHeaderBlock& response, + base::Time response_time, + int status) { + return OK; +} + +void ClosingDelegate::OnHeadersSent() {} + +int ClosingDelegate::OnDataReceived(const char* data, int length) { + return OK; +} + +void ClosingDelegate::OnDataSent(int length) {} + +void ClosingDelegate::OnClose(int status) { + if (stream_) + stream_->Close(); + stream_ = NULL; +} + StreamDelegateBase::StreamDelegateBase( const scoped_refptr& stream) : stream_(stream), diff --git a/net/spdy/spdy_stream_test_util.h b/net/spdy/spdy_stream_test_util.h index 297a4ff..b72bd80 100644 --- a/net/spdy/spdy_stream_test_util.h +++ b/net/spdy/spdy_stream_test_util.h @@ -15,6 +15,29 @@ namespace net { namespace test { +// Delegate that calls Close() on |stream_| on OnClose. Used by tests +// to make sure that such an action is harmless. +class ClosingDelegate : public SpdyStream::Delegate { + public: + explicit ClosingDelegate(const scoped_refptr& stream); + virtual ~ClosingDelegate(); + + // SpdyStream::Delegate implementation. + virtual bool OnSendHeadersComplete(int status) OVERRIDE; + virtual int OnSendBody() OVERRIDE; + virtual int OnSendBodyComplete(int status, bool* eof) OVERRIDE; + virtual int OnResponseReceived(const SpdyHeaderBlock& response, + base::Time response_time, + int status) OVERRIDE; + virtual void OnHeadersSent() OVERRIDE; + virtual int OnDataReceived(const char* data, int length) OVERRIDE; + virtual void OnDataSent(int length) OVERRIDE; + virtual void OnClose(int status) OVERRIDE; + + private: + scoped_refptr stream_; +}; + // Base class with shared functionality for test delegate // implementations below. class StreamDelegateBase : public SpdyStream::Delegate { -- cgit v1.1