[SPDY] Make sure final SpdyStream references are held by SpdySession
This will make it much easier to avoid use-after-free bugs when we
make SpdyStream not ref-counted.
Expose only WeakPtr<SpdyStream> objects outside of SpdySession
and SpdyWriteQueue.
Introduce SpdySession::DeleteStreamRefs, which takes a pointer to the last
scoped_refptr to that stream and serves as its destructor.
Fix a lot of dangling SpdyStream references in tests.
Remove SpdyStream::cancelled(), as a stream now guaranteed to be
deleted when cancelled.
Split SpdyStreamRequest::OnRequestComplete() into
OnRequestComplete{Success,Failure}().
BUG=178943
Review URL: https://chromiumcodereview.appspot.com/14812007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@199688 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc
index 3c38c155..03b43ed3 100644
--- a/net/http/http_proxy_client_socket_pool.cc
+++ b/net/http/http_proxy_client_socket_pool.cc
@@ -330,7 +330,7 @@
return result;
next_state_ = STATE_HTTP_PROXY_CONNECT_COMPLETE;
- scoped_refptr<SpdyStream> stream = spdy_stream_request_.ReleaseStream();
+ base::WeakPtr<SpdyStream> stream = spdy_stream_request_.ReleaseStream();
DCHECK(stream);
// |transport_socket_| will set itself as |stream|'s delegate.
transport_socket_.reset(
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 33fe47d..84e30b5f 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -43,15 +43,18 @@
more_read_data_pending_(false),
direct_(direct) {}
-void SpdyHttpStream::InitializeWithExistingStream(SpdyStream* spdy_stream) {
+void SpdyHttpStream::InitializeWithExistingStream(
+ const base::WeakPtr<SpdyStream>& spdy_stream) {
stream_ = spdy_stream;
stream_->SetDelegate(this);
response_headers_received_ = true;
}
SpdyHttpStream::~SpdyHttpStream() {
- if (stream_)
+ if (stream_) {
stream_->DetachDelegate();
+ DCHECK(!stream_);
+ }
}
int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info,
@@ -104,9 +107,6 @@
int SpdyHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
CHECK(!callback.is_null());
- if (stream_)
- CHECK(!stream_->cancelled());
-
if (stream_closed_)
return closed_stream_status_;
@@ -241,7 +241,6 @@
}
CHECK(!callback.is_null());
- CHECK(!stream_->cancelled());
CHECK(response);
// SendRequest can be called in two cases.
@@ -276,9 +275,10 @@
void SpdyHttpStream::Cancel() {
callback_.Reset();
- if (stream_)
+ if (stream_) {
stream_->Cancel();
- DCHECK(!stream_.get());
+ DCHECK(!stream_);
+ }
}
void SpdyHttpStream::OnStreamCreated(
@@ -451,7 +451,7 @@
closed_stream_status_ = status;
closed_stream_id_ = stream_->stream_id();
}
- stream_ = NULL;
+ stream_.reset();
bool invoked_callback = false;
if (status == net::OK) {
// We need to complete any pending buffered read now.
@@ -504,9 +504,6 @@
if (stream_status != OK)
return false;
- if (stream_)
- DCHECK(!stream_->cancelled());
-
// When more_read_data_pending_ is true, it means that more data has
// arrived since we started waiting. Wait a little longer and continue
// to buffer.
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index 2af82f0..bfb5076 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -38,7 +38,8 @@
// SpdyStream. In particular, this must be called instead of
// InitializeStream() if a NULL SpdySession was passed into the
// constructor.
- void InitializeWithExistingStream(SpdyStream* spdy_stream);
+ void InitializeWithExistingStream(
+ const base::WeakPtr<SpdyStream>& spdy_stream);
SpdyStream* stream() { return stream_.get(); }
@@ -116,7 +117,7 @@
const scoped_refptr<SpdySession> spdy_session_;
SpdyStreamRequest stream_request_;
- scoped_refptr<SpdyStream> stream_;
+ base::WeakPtr<SpdyStream> stream_;
bool stream_closed_;
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index 84552d2..2fcb899 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -23,7 +23,7 @@
namespace net {
SpdyProxyClientSocket::SpdyProxyClientSocket(
- SpdyStream* spdy_stream,
+ const base::WeakPtr<SpdyStream>& spdy_stream,
const std::string& user_agent,
const HostPortPair& endpoint,
const GURL& url,
@@ -142,7 +142,7 @@
// This will cause OnClose to be invoked, which takes care of
// cleaning up all the internal state.
spdy_stream_->Cancel();
- DCHECK(!spdy_stream_.get());
+ DCHECK(!spdy_stream_);
}
}
@@ -432,8 +432,8 @@
// SpdyHttpStream so that any subsequent SpdyFrames are processed in
// the context of the HttpStream, not the socket.
DCHECK(spdy_stream_);
- SpdyStream* stream = spdy_stream_;
- spdy_stream_ = NULL;
+ base::WeakPtr<SpdyStream> stream = spdy_stream_;
+ spdy_stream_.reset();
response_stream_.reset(new SpdyHttpStream(NULL, false));
response_stream_->InitializeWithExistingStream(stream);
next_state_ = STATE_DISCONNECTED;
@@ -546,9 +546,8 @@
}
void SpdyProxyClientSocket::OnClose(int status) {
- DCHECK(spdy_stream_);
was_ever_used_ = spdy_stream_->WasEverUsed();
- spdy_stream_ = NULL;
+ spdy_stream_.reset();
bool connecting = next_state_ != STATE_DISCONNECTED &&
next_state_ < STATE_OPEN;
diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
index e25e1e3..f5b1baa 100644
--- a/net/spdy/spdy_proxy_client_socket.h
+++ b/net/spdy/spdy_proxy_client_socket.h
@@ -42,7 +42,7 @@
// CONNECT frame for |endpoint|. After the SYN_REPLY is received,
// any data read/written to the socket will be transferred in data
// frames. This object will set itself as |spdy_stream|'s delegate.
- SpdyProxyClientSocket(SpdyStream* spdy_stream,
+ SpdyProxyClientSocket(const base::WeakPtr<SpdyStream>& spdy_stream,
const std::string& user_agent,
const HostPortPair& endpoint,
const GURL& url,
@@ -132,7 +132,7 @@
State next_state_;
// Pointer to the SPDY Stream that this sits on top of.
- scoped_refptr<SpdyStream> spdy_stream_;
+ base::WeakPtr<SpdyStream> spdy_stream_;
// Stores the callback to the layer above, called on completing Read() or
// Connect().
diff --git a/net/spdy/spdy_proxy_client_socket_spdy2_unittest.cc b/net/spdy/spdy_proxy_client_socket_spdy2_unittest.cc
index 0e2d2b1d..509838a 100644
--- a/net/spdy/spdy_proxy_client_socket_spdy2_unittest.cc
+++ b/net/spdy/spdy_proxy_client_socket_spdy2_unittest.cc
@@ -122,7 +122,6 @@
SpdySessionDependencies session_deps_;
MockConnect connect_data_;
scoped_refptr<SpdySession> spdy_session_;
- scoped_refptr<SpdyStream> spdy_stream_;
BufferedSpdyFramer framer_;
std::string user_agent_;
@@ -145,7 +144,6 @@
session_deps_(kProtoSPDY2),
connect_data_(SYNCHRONOUS, OK),
spdy_session_(NULL),
- spdy_stream_(NULL),
framer_(2, false),
user_agent_(kUserAgent),
url_(kRequestUrl),
@@ -203,13 +201,14 @@
spdy_session_->InitializeWithSocket(connection.release(), false, OK);
// Create the SPDY Stream.
- spdy_stream_ =
- CreateStreamSynchronously(spdy_session_, url_, LOWEST, net_log_.bound());
- ASSERT_TRUE(spdy_stream_.get() != NULL);
+ base::WeakPtr<SpdyStream> spdy_stream(
+ CreateStreamSynchronously(
+ spdy_session_, url_, LOWEST, net_log_.bound()));
+ ASSERT_TRUE(spdy_stream.get() != NULL);
// Create the SpdyProxyClientSocket.
sock_.reset(
- new SpdyProxyClientSocket(spdy_stream_, user_agent_,
+ new SpdyProxyClientSocket(spdy_stream, user_agent_,
endpoint_host_port_pair_, url_,
proxy_host_port_, net_log_.bound(),
session_->http_auth_cache(),
diff --git a/net/spdy/spdy_proxy_client_socket_spdy3_unittest.cc b/net/spdy/spdy_proxy_client_socket_spdy3_unittest.cc
index 0251f81..0ce54b7 100644
--- a/net/spdy/spdy_proxy_client_socket_spdy3_unittest.cc
+++ b/net/spdy/spdy_proxy_client_socket_spdy3_unittest.cc
@@ -122,7 +122,6 @@
SpdySessionDependencies session_deps_;
MockConnect connect_data_;
scoped_refptr<SpdySession> spdy_session_;
- scoped_refptr<SpdyStream> spdy_stream_;
BufferedSpdyFramer framer_;
std::string user_agent_;
@@ -145,7 +144,6 @@
session_deps_(kProtoSPDY3),
connect_data_(SYNCHRONOUS, OK),
spdy_session_(NULL),
- spdy_stream_(NULL),
framer_(3, false),
user_agent_(kUserAgent),
url_(kRequestUrl),
@@ -203,13 +201,14 @@
spdy_session_->InitializeWithSocket(connection.release(), false, OK);
// Create the SPDY Stream.
- spdy_stream_ =
- CreateStreamSynchronously(spdy_session_, url_, LOWEST, net_log_.bound());
- ASSERT_TRUE(spdy_stream_.get() != NULL);
+ base::WeakPtr<SpdyStream> spdy_stream(
+ CreateStreamSynchronously(
+ spdy_session_, url_, LOWEST, net_log_.bound()));
+ ASSERT_TRUE(spdy_stream.get() != NULL);
// Create the SpdyProxyClientSocket.
sock_.reset(
- new SpdyProxyClientSocket(spdy_stream_, user_agent_,
+ new SpdyProxyClientSocket(spdy_stream, user_agent_,
endpoint_host_port_pair_, url_,
proxy_host_port_, net_log_.bound(),
session_->http_auth_cache(),
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index e0b48cf..cf697bf 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -254,7 +254,7 @@
net_log_ = net_log;
callback_ = callback;
- scoped_refptr<SpdyStream> stream;
+ base::WeakPtr<SpdyStream> stream;
int rv = session->TryCreateStream(this, &stream);
if (rv == OK) {
Reset();
@@ -269,28 +269,39 @@
Reset();
}
-scoped_refptr<SpdyStream> SpdyStreamRequest::ReleaseStream() {
+base::WeakPtr<SpdyStream> SpdyStreamRequest::ReleaseStream() {
DCHECK(!session_.get());
- scoped_refptr<SpdyStream> stream = stream_;
- DCHECK(stream.get());
+ base::WeakPtr<SpdyStream> stream = stream_;
+ DCHECK(stream);
Reset();
return stream;
}
-void SpdyStreamRequest::OnRequestComplete(
- const scoped_refptr<SpdyStream>& stream,
- int rv) {
- DCHECK(session_.get());
+void SpdyStreamRequest::OnRequestCompleteSuccess(
+ base::WeakPtr<SpdyStream>* stream) {
+ DCHECK(session_);
+ DCHECK(!stream_);
DCHECK(!callback_.is_null());
CompletionCallback callback = callback_;
Reset();
- stream_ = stream;
+ DCHECK(*stream);
+ stream_ = *stream;
+ callback.Run(OK);
+}
+
+void SpdyStreamRequest::OnRequestCompleteFailure(int rv) {
+ DCHECK(session_);
+ DCHECK(!stream_);
+ DCHECK(!callback_.is_null());
+ CompletionCallback callback = callback_;
+ Reset();
+ DCHECK_NE(rv, OK);
callback.Run(rv);
}
void SpdyStreamRequest::Reset() {
session_ = NULL;
- stream_ = NULL;
+ stream_.reset();
url_ = GURL();
priority_ = MINIMUM_PRIORITY;
net_log_ = BoundNetLog();
@@ -496,11 +507,11 @@
int SpdySession::GetPushStream(
const GURL& url,
- scoped_refptr<SpdyStream>* stream,
+ base::WeakPtr<SpdyStream>* stream,
const BoundNetLog& stream_net_log) {
CHECK_NE(state_, STATE_CLOSED);
- *stream = NULL;
+ stream->reset();
// Don't allow access to secure push streams over an unauthenticated, but
// encrypted SSL socket.
@@ -525,7 +536,7 @@
}
int SpdySession::TryCreateStream(SpdyStreamRequest* request,
- scoped_refptr<SpdyStream>* stream) {
+ base::WeakPtr<SpdyStream>* stream) {
if (!max_concurrent_streams_ ||
(active_streams_.size() + created_streams_.size() <
max_concurrent_streams_)) {
@@ -539,7 +550,7 @@
}
int SpdySession::CreateStream(const SpdyStreamRequest& request,
- scoped_refptr<SpdyStream>* stream) {
+ base::WeakPtr<SpdyStream>* stream) {
DCHECK_GE(request.priority(), MINIMUM_PRIORITY);
DCHECK_LT(request.priority(), NUM_PRIORITIES);
@@ -572,11 +583,14 @@
}
const std::string& path = request.url().PathForRequest();
- *stream = new SpdyStream(this, path, request.priority(),
- stream_initial_send_window_size_,
- stream_initial_recv_window_size_,
- false, request.net_log());
- created_streams_.insert(*stream);
+ scoped_refptr<SpdyStream> new_stream(
+ new SpdyStream(this, path, request.priority(),
+ stream_initial_send_window_size_,
+ stream_initial_recv_window_size_,
+ false, request.net_log()));
+ created_streams_.insert(new_stream);
+
+ *stream = new_stream->GetWeakPtr();
UMA_HISTOGRAM_CUSTOM_COUNTS(
"Net.SpdyPriorityCount",
@@ -896,11 +910,13 @@
DeleteStream(stream_id, status);
}
-void SpdySession::CloseCreatedStream(SpdyStream* stream, int status) {
+void SpdySession::CloseCreatedStream(
+ const base::WeakPtr<SpdyStream>& stream, int status) {
DCHECK_EQ(0u, stream->stream_id());
- created_streams_.erase(scoped_refptr<SpdyStream>(stream));
- stream->OnClose(status);
- ProcessPendingStreamRequests();
+ scoped_refptr<SpdyStream> last_ref(stream.get());
+ created_streams_.erase(last_ref);
+
+ DeleteStreamRefs(&last_ref, status);
}
void SpdySession::ResetStream(SpdyStreamId stream_id,
@@ -1075,7 +1091,7 @@
if (in_flight_write_->GetRemainingSize() == 0) {
// It is possible that the stream was cancelled while we were
// writing to the socket.
- if (in_flight_write_stream_ && !in_flight_write_stream_->cancelled()) {
+ if (in_flight_write_stream_) {
DCHECK_GT(in_flight_write_frame_size_, 0u);
in_flight_write_stream_->OnFrameWriteComplete(
in_flight_write_frame_type_,
@@ -1137,19 +1153,14 @@
if (!write_queue_.Dequeue(&frame_type, &producer, &stream))
break;
- // It is possible that a stream had data to write, but a
- // WINDOW_UPDATE frame has been received which made that
- // stream no longer writable.
- // TODO(rch): consider handling that case by removing the
- // stream from the writable queue?
- if (stream.get() && stream->cancelled())
- continue;
+ if (stream)
+ DCHECK(!stream->closed());
// Activate the stream only when sending the SYN_STREAM frame to
// guarantee monotonically-increasing stream IDs.
if (frame_type == SYN_STREAM) {
- if (stream.get() && stream->stream_id() == 0) {
- ActivateStream(stream);
+ if (stream && stream->stream_id() == 0) {
+ ActivateStream(stream.get());
} else {
NOTREACHED();
continue;
@@ -1204,26 +1215,24 @@
queue.swap(pending_create_stream_queues_[i]);
for (PendingStreamRequestQueue::const_iterator it = queue.begin();
it != queue.end(); ++it) {
- (*it)->OnRequestComplete(NULL, ERR_ABORTED);
+ (*it)->OnRequestCompleteFailure(ERR_ABORTED);
}
}
ActiveStreamMap::iterator it =
active_streams_.lower_bound(last_good_stream_id + 1);
while (it != active_streams_.end()) {
- const scoped_refptr<SpdyStream>& stream = it->second;
- ++it;
- LogAbandonedStream(stream, status);
- DeleteStream(stream->stream_id(), status);
+ SpdyStreamId stream_id = it->first;
streams_abandoned_count_++;
+ LogAbandonedStream(it->second, status);
+ ++it;
+ DeleteStream(stream_id, status);
}
while (!created_streams_.empty()) {
CreatedStreamSet::iterator it = created_streams_.begin();
- const scoped_refptr<SpdyStream> stream = *it;
- created_streams_.erase(it);
- LogAbandonedStream(stream, status);
- stream->OnClose(status);
+ LogAbandonedStream(*it, status);
+ CloseCreatedStream((*it)->GetWeakPtr(), status);
}
write_queue_.RemovePendingWritesForStreamsAfter(last_good_stream_id);
@@ -1411,8 +1420,7 @@
if (status != OK) {
for (PushedStreamMap::iterator it = unclaimed_pushed_streams_.begin();
it != unclaimed_pushed_streams_.end(); ++it) {
- scoped_refptr<SpdyStream> curr = it->second.first;
- if (id == curr->stream_id()) {
+ if (id == it->second.first->stream_id()) {
unclaimed_pushed_streams_.erase(it);
break;
}
@@ -1424,15 +1432,36 @@
if (it == active_streams_.end())
return;
- const scoped_refptr<SpdyStream> stream(it->second);
+ scoped_refptr<SpdyStream> last_ref;
+ last_ref.swap(it->second);
active_streams_.erase(it);
- DCHECK(stream);
+ DCHECK(last_ref);
- write_queue_.RemovePendingWritesForStream(stream);
+ DeleteStreamRefs(&last_ref, status);
+}
- // If this is an active stream, call the callback.
- stream->OnClose(status);
+void SpdySession::DeleteStreamRefs(scoped_refptr<SpdyStream>* last_ref,
+ int status) {
+ if (in_flight_write_stream_ == *last_ref) {
+ // If we're deleting the stream for the in-flight write, we still
+ // need to let the write complete, so we clear
+ // |in_flight_write_stream_| and let the write finish on its own
+ // without notifying |in_flight_write_stream_|.
+ in_flight_write_stream_ = NULL;
+ }
+
+ write_queue_.RemovePendingWritesForStream(*last_ref);
+
+ (*last_ref)->OnClose(status);
+
ProcessPendingStreamRequests();
+
+ // Nothing else should be holding a reference to |stream| after this
+ // point.
+ CHECK((*last_ref)->HasOneRef());
+
+ // May release the last reference to |this|.
+ *last_ref = NULL;
}
void SpdySession::RemoveFromPool() {
@@ -1443,19 +1472,19 @@
}
}
-scoped_refptr<SpdyStream> SpdySession::GetActivePushStream(
+base::WeakPtr<SpdyStream> SpdySession::GetActivePushStream(
const std::string& path) {
base::StatsCounter used_push_streams("spdy.claimed_push_streams");
PushedStreamMap::iterator it = unclaimed_pushed_streams_.find(path);
if (it != unclaimed_pushed_streams_.end()) {
net_log_.AddEvent(NetLog::TYPE_SPDY_STREAM_ADOPTED_PUSH_STREAM);
- scoped_refptr<SpdyStream> stream = it->second.first;
+ base::WeakPtr<SpdyStream> stream = it->second.first->GetWeakPtr();
unclaimed_pushed_streams_.erase(it);
used_push_streams.Increment();
return stream;
}
- return NULL;
+ return base::WeakPtr<SpdyStream>();
}
bool SpdySession::GetSSLInfo(SSLInfo* ssl_info,
@@ -1491,10 +1520,11 @@
void SpdySession::OnStreamError(SpdyStreamId stream_id,
const std::string& description) {
ActiveStreamMap::const_iterator it = active_streams_.find(stream_id);
- if (it != active_streams_.end()) {
- ResetStream(stream_id, it->second->priority(),
- RST_STREAM_PROTOCOL_ERROR, description);
- }
+ // We still want to reset the stream even if we don't know anything
+ // about it.
+ RequestPriority priority =
+ (it == active_streams_.end()) ? IDLE : it->second->priority();
+ ResetStream(stream_id, priority, RST_STREAM_PROTOCOL_ERROR, description);
}
void SpdySession::OnStreamFrameData(SpdyStreamId stream_id,
@@ -1572,12 +1602,13 @@
bool SpdySession::Respond(const SpdyHeaderBlock& headers,
- const scoped_refptr<SpdyStream> stream) {
+ const scoped_refptr<SpdyStream>& stream) {
int rv = OK;
+ SpdyStreamId stream_id = stream->stream_id();
+ // May invalidate |stream|.
rv = stream->OnResponseReceived(headers);
if (rv < 0) {
DCHECK_NE(rv, ERR_IO_PENDING);
- const SpdyStreamId stream_id = stream->stream_id();
DeleteStream(stream_id, rv);
return false;
}
@@ -1636,7 +1667,9 @@
const std::string& url = gurl.spec();
// Verify we have a valid stream association.
- if (!IsStreamActive(associated_stream_id)) {
+ ActiveStreamMap::iterator associated_it =
+ active_streams_.find(associated_stream_id);
+ if (associated_it == active_streams_.end()) {
ResetStream(stream_id, request_priority, RST_STREAM_INVALID_STREAM,
base::StringPrintf(
"Received OnSyn with inactive associated stream %d",
@@ -1656,9 +1689,7 @@
associated_stream_id));
}
} else {
- scoped_refptr<SpdyStream> associated_stream =
- active_streams_[associated_stream_id];
- GURL associated_url(associated_stream->GetUrl());
+ GURL associated_url(associated_it->second->GetUrl());
if (associated_url.GetOrigin() != gurl.GetOrigin()) {
ResetStream(stream_id, request_priority, RST_STREAM_REFUSED_STREAM,
base::StringPrintf(
@@ -1669,8 +1700,7 @@
}
// There should not be an existing pushed stream with the same path.
- PushedStreamMap::iterator it = unclaimed_pushed_streams_.find(url);
- if (it != unclaimed_pushed_streams_.end()) {
+ if (unclaimed_pushed_streams_.find(url) != unclaimed_pushed_streams_.end()) {
ResetStream(stream_id, request_priority, RST_STREAM_PROTOCOL_ERROR,
"Received duplicate pushed stream with url: " + url);
return;
@@ -1688,12 +1718,18 @@
std::pair<scoped_refptr<SpdyStream>, base::TimeTicks> (
stream, time_func_());
-
- ActivateStream(stream);
+ ActivateStream(stream.get());
stream->set_response_received();
+ stream = NULL;
+
+ ActiveStreamMap::iterator it = active_streams_.find(stream_id);
+ if (it == active_streams_.end()) {
+ NOTREACHED();
+ return;
+ }
// Parse the headers.
- if (!Respond(headers, stream))
+ if (!Respond(headers, it->second))
return;
base::StatsCounter push_requests("spdy.pushed_streams");
@@ -1719,13 +1755,13 @@
// DeleteStream() will invalidate the current iterator, so move to next.
++it;
if (minimum_freshness > creation_time) {
- DeleteStream(stream->stream_id(), ERR_INVALID_SPDY_STREAM);
base::StatsCounter abandoned_push_streams(
"spdy.abandoned_push_streams");
base::StatsCounter abandoned_streams("spdy.abandoned_streams");
abandoned_push_streams.Increment();
abandoned_streams.Increment();
streams_abandoned_count_++;
+ DeleteStream(stream->stream_id(), ERR_INVALID_SPDY_STREAM);
}
}
next_unclaimed_push_stream_sweep_time_ = time_func_() +
@@ -1743,14 +1779,14 @@
stream_id, 0));
}
- if (!IsStreamActive(stream_id)) {
+ ActiveStreamMap::iterator it = active_streams_.find(stream_id);
+ if (it == active_streams_.end()) {
// NOTE: it may just be that the stream was cancelled.
return;
}
- scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
+ const scoped_refptr<SpdyStream>& stream = it->second;
CHECK_EQ(stream->stream_id(), stream_id);
- CHECK(!stream->cancelled());
if (stream->response_received()) {
stream->LogStreamError(ERR_SYN_REPLY_NOT_RECEIVED,
@@ -1775,20 +1811,18 @@
stream_id, 0));
}
- if (!IsStreamActive(stream_id)) {
+ ActiveStreamMap::iterator it = active_streams_.find(stream_id);
+ if (it == active_streams_.end()) {
// NOTE: it may just be that the stream was cancelled.
LOG(WARNING) << "Received HEADERS for invalid stream " << stream_id;
return;
}
- scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
- CHECK_EQ(stream->stream_id(), stream_id);
- CHECK(!stream->cancelled());
+ CHECK_EQ(it->second->stream_id(), stream_id);
- int rv = stream->OnHeaders(headers);
+ int rv = it->second->OnHeaders(headers);
if (rv < 0) {
DCHECK_NE(rv, ERR_IO_PENDING);
- const SpdyStreamId stream_id = stream->stream_id();
DeleteStream(stream_id, rv);
}
}
@@ -1801,23 +1835,23 @@
base::Bind(&NetLogSpdyRstCallback,
stream_id, status, &description));
- if (!IsStreamActive(stream_id)) {
+ ActiveStreamMap::iterator it = active_streams_.find(stream_id);
+ if (it == active_streams_.end()) {
// NOTE: it may just be that the stream was cancelled.
LOG(WARNING) << "Received RST for invalid stream" << stream_id;
return;
}
- scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
- CHECK_EQ(stream->stream_id(), stream_id);
- CHECK(!stream->cancelled());
+
+ CHECK_EQ(it->second->stream_id(), stream_id);
if (status == 0) {
- stream->OnDataReceived(scoped_ptr<SpdyBuffer>());
+ it->second->OnDataReceived(scoped_ptr<SpdyBuffer>());
} else if (status == RST_STREAM_REFUSED_STREAM) {
DeleteStream(stream_id, ERR_SPDY_SERVER_REFUSED_STREAM);
} else {
RecordProtocolErrorHistogram(
PROTOCOL_ERROR_RST_STREAM_FOR_NON_ACTIVE_STREAM);
- stream->LogStreamError(
+ it->second->LogStreamError(
ERR_SPDY_PROTOCOL_ERROR,
base::StringPrintf("SPDY stream closed with status: %d", status));
// TODO(mbelshe): Map from Spdy-protocol errors to something sensical.
@@ -1921,7 +1955,6 @@
}
CHECK_EQ(it->second->stream_id(), stream_id);
- CHECK(!it->second->cancelled());
it->second->IncreaseSendWindowSize(static_cast<int32>(delta_window_size));
}
}
@@ -2231,11 +2264,17 @@
if (it == pending_stream_request_completions_.end())
return;
- scoped_refptr<SpdyStream> stream;
+ base::WeakPtr<SpdyStream> stream;
int rv = CreateStream(*pending_request, &stream);
pending_stream_request_completions_.erase(it);
- pending_request->OnRequestComplete(stream, rv);
+ if (rv == OK) {
+ DCHECK(stream);
+ pending_request->OnRequestCompleteSuccess(&stream);
+ } else {
+ DCHECK(!stream);
+ pending_request->OnRequestCompleteFailure(rv);
+ }
}
SSLClientSocket* SpdySession::GetSSLClientSocket() const {
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 8995371..5e22074 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -142,16 +142,19 @@
// caller. Must be called at most once after StartRequest() returns
// OK or |callback| is called with OK. The caller must immediately
// set a delegate for the returned stream (except for test code).
- scoped_refptr<SpdyStream> ReleaseStream();
+ base::WeakPtr<SpdyStream> ReleaseStream();
private:
friend class SpdySession;
- // Called by |session_| when the stream attempt is
- // finished. |stream| is non-NULL exactly when |rv| is OK. Also
- // called with a NULL stream and ERR_ABORTED if |session_| is
- // destroyed while the stream attempt is still pending.
- void OnRequestComplete(const scoped_refptr<SpdyStream>& stream, int rv);
+ // Called by |session_| when the stream attempt has finished
+ // successfully.
+ void OnRequestCompleteSuccess(base::WeakPtr<SpdyStream>* stream);
+
+ // Called by |session_| when the stream attempt has finished with an
+ // error. Also called with ERR_ABORTED if |session_| is destroyed
+ // while the stream attempt is still pending.
+ void OnRequestCompleteFailure(int rv);
// Accessors called by |session_|.
const GURL& url() const { return url_; }
@@ -161,7 +164,7 @@
void Reset();
scoped_refptr<SpdySession> session_;
- scoped_refptr<SpdyStream> stream_;
+ base::WeakPtr<SpdyStream> stream_;
GURL url_;
RequestPriority priority_;
BoundNetLog net_log_;
@@ -222,7 +225,7 @@
// ERR_IO_PENDING) otherwise.
int GetPushStream(
const GURL& url,
- scoped_refptr<SpdyStream>* spdy_stream,
+ base::WeakPtr<SpdyStream>* spdy_stream,
const BoundNetLog& stream_net_log);
// Used by SpdySessionPool to initialize with a pre-existing SSL socket. For
@@ -282,17 +285,20 @@
int len,
SpdyDataFlags flags);
- // Close a stream.
+ // If there is an active stream with the given ID, close it. There
+ // must be no external references to that active stream.
void CloseStream(SpdyStreamId stream_id, int status);
// Close a stream that has been created but is not yet active.
- void CloseCreatedStream(SpdyStream* stream, int status);
+ // There must be no external references to that active stream.
+ void CloseCreatedStream(const base::WeakPtr<SpdyStream>& stream, int status);
- // Reset a stream by sending a RST_STREAM frame with the given
- // status code at the given priority, which should be the stream's
- // priority. Also closes the stream. Was not piggybacked to
- // CloseStream since not all of the calls to CloseStream necessitate
- // sending a RST_STREAM.
+ // If there is an active stream with the given ID, reset it by by
+ // sending a RST_STREAM frame with the given status code at the
+ // given priority, which should be the stream's priority, and
+ // deleting it. There must be no external references to that active
+ // stream. Was not piggybacked to CloseStream since not all of the
+ // calls to CloseStream necessitate sending a RST_STREAM.
void ResetStream(SpdyStreamId stream_id,
RequestPriority priority,
SpdyRstStreamStatus status,
@@ -480,7 +486,7 @@
typedef std::deque<SpdyStreamRequest*> PendingStreamRequestQueue;
typedef std::set<SpdyStreamRequest*> PendingStreamRequestCompletionSet;
- typedef std::map<int, scoped_refptr<SpdyStream> > ActiveStreamMap;
+ typedef std::map<SpdyStreamId, scoped_refptr<SpdyStream> > ActiveStreamMap;
typedef std::map<std::string,
std::pair<scoped_refptr<SpdyStream>, base::TimeTicks> > PushedStreamMap;
@@ -499,16 +505,16 @@
// Called by SpdyStreamRequest to start a request to create a
// stream. If OK is returned, then |stream| will be filled in with a
// valid stream. If ERR_IO_PENDING is returned, then
- // |request->OnRequestComplete()| will be called when the stream is
- // created (unless it is cancelled). Otherwise, no stream is created
- // and the error is returned.
+ // |request->OnRequestComplete{Success,Failure}()| will be called
+ // when the stream is created (unless it is cancelled). Otherwise,
+ // no stream is created and the error is returned.
int TryCreateStream(SpdyStreamRequest* request,
- scoped_refptr<SpdyStream>* stream);
+ base::WeakPtr<SpdyStream>* stream);
// Actually create a stream into |stream|. Returns OK if successful;
// otherwise, returns an error and |stream| is not filled.
int CreateStream(const SpdyStreamRequest& request,
- scoped_refptr<SpdyStream>* stream);
+ base::WeakPtr<SpdyStream>* stream);
// Called by SpdyStreamRequest to remove |request| from the stream
// creation queue.
@@ -594,20 +600,31 @@
// Track active streams in the active stream list.
void ActivateStream(SpdyStream* stream);
+
+ // If there is an active stream with the given ID, delete it. There
+ // must be no external references to that active stream. Also note
+ // that that active stream may hold the last reference to this
+ // object.
void DeleteStream(SpdyStreamId id, int status);
+ // Remove all internal references to the stream pointed to by
+ // |last_ref|, which must be the last reference to that stream and
+ // is set to NULL. Also note that the given stream may hold the last
+ // reference to this object.
+ void DeleteStreamRefs(scoped_refptr<SpdyStream>* last_ref, int status);
+
// Removes this session from the session pool.
void RemoveFromPool();
// Check if we have a pending pushed-stream for this url
// Returns the stream if found (and returns it from the pending
// list), returns NULL otherwise.
- scoped_refptr<SpdyStream> GetActivePushStream(const std::string& url);
+ base::WeakPtr<SpdyStream> GetActivePushStream(const std::string& url);
// Calls OnResponseReceived().
// Returns true if successful.
bool Respond(const SpdyHeaderBlock& headers,
- const scoped_refptr<SpdyStream> stream);
+ const scoped_refptr<SpdyStream>& stream);
void RecordPingRTTHistogram(base::TimeDelta duration);
void RecordHistograms();
diff --git a/net/spdy/spdy_session_spdy2_unittest.cc b/net/spdy/spdy_session_spdy2_unittest.cc
index af0af3d..686949b3 100644
--- a/net/spdy/spdy_session_spdy2_unittest.cc
+++ b/net/spdy/spdy_session_spdy2_unittest.cc
@@ -139,10 +139,10 @@
EXPECT_EQ(2, session->GetProtocolVersion());
GURL url("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
@@ -180,8 +180,7 @@
scoped_refptr<SpdySession> session2 = GetSession(pair_);
spdy_stream1->Close();
- spdy_stream1 = NULL;
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Delete the first session.
session = NULL;
@@ -189,6 +188,7 @@
// Delete the second session.
spdy_session_pool_->Remove(session2);
session2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
}
TEST_F(SpdySessionSpdy2Test, ClientPing) {
@@ -217,7 +217,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -272,7 +272,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -286,6 +286,7 @@
// Delete the session.
session = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
}
TEST_F(SpdySessionSpdy2Test, DeleteExpiredPushStreams) {
@@ -315,6 +316,7 @@
false, session->net_log_));
stream->set_spdy_headers(request_headers.Pass());
session->ActivateStream(stream);
+ stream = NULL;
SpdyHeaderBlock headers;
headers["url"] = "http://www.google.com/a.dat";
@@ -366,7 +368,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -400,6 +402,7 @@
// Delete the first session.
session = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
}
TEST_F(SpdySessionSpdy2Test, CloseIdleSessions) {
@@ -424,7 +427,7 @@
InitializeSession(
http_session_.get(), session1.get(), test_host_port_pair1));
GURL url1(kTestHost1);
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session1, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
@@ -439,7 +442,7 @@
InitializeSession(
http_session_.get(), session2.get(), test_host_port_pair2));
GURL url2(kTestHost2);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session2, url2, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
@@ -454,7 +457,7 @@
InitializeSession(
http_session_.get(), session3.get(), test_host_port_pair3));
GURL url3(kTestHost3);
- scoped_refptr<SpdyStream> spdy_stream3 =
+ base::WeakPtr<SpdyStream> spdy_stream3 =
CreateStreamSynchronously(session3, url3, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream3.get() != NULL);
@@ -478,7 +481,9 @@
// Make sessions 1 and 3 inactive, but keep them open.
// Session 2 still open and active
session1->CloseCreatedStream(spdy_stream1, OK);
+ EXPECT_EQ(NULL, spdy_stream1.get());
session3->CloseCreatedStream(spdy_stream3, OK);
+ EXPECT_EQ(NULL, spdy_stream3.get());
EXPECT_FALSE(session1->is_active());
EXPECT_FALSE(session1->IsClosed());
EXPECT_TRUE(session2->is_active());
@@ -502,6 +507,7 @@
// Make 2 not active
session2->CloseCreatedStream(spdy_stream2, OK);
+ EXPECT_EQ(NULL, spdy_stream2.get());
EXPECT_FALSE(session2->is_active());
EXPECT_FALSE(session2->IsClosed());
@@ -553,22 +559,17 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
- StreamReleaserCallback stream_releaser(session, spdy_stream1);
-
+ StreamReleaserCallback stream_releaser;
SpdyStreamRequest request;
ASSERT_EQ(ERR_IO_PENDING,
request.StartRequest(session, test_url_, MEDIUM,
BoundNetLog(),
stream_releaser.MakeCallback(&request)));
- // Make sure |stream_releaser| holds the last refs.
- session = NULL;
- spdy_stream1 = NULL;
-
EXPECT_EQ(OK, stream_releaser.WaitForResult());
}
@@ -617,19 +618,17 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
- StreamReleaserCallback stream_releaser(session, spdy_stream1);
-
+ StreamReleaserCallback stream_releaser;
SpdyStreamRequest request;
ASSERT_EQ(ERR_IO_PENDING,
request.StartRequest(session, test_url_, MEDIUM,
BoundNetLog(),
stream_releaser.MakeCallback(&request)));
- spdy_stream1 = NULL;
EXPECT_EQ(OK, stream_releaser.WaitForResult());
// Make sure that persisted data is cleared.
@@ -674,7 +673,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
@@ -690,7 +689,7 @@
// Release the first one, this will allow the second to be created.
spdy_stream1->Cancel();
- spdy_stream1 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
request.CancelRequest();
callback.reset();
@@ -955,18 +954,20 @@
break;
case SPDY_POOL_CLOSE_IDLE_SESSIONS:
GURL url(test_hosts[0].url);
- scoped_refptr<SpdyStream> spdy_stream =
+ base::WeakPtr<SpdyStream> spdy_stream =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
GURL url1(test_hosts[1].url);
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session1, url1, MEDIUM, BoundNetLog());
GURL url2(test_hosts[2].url);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session2, url2, MEDIUM, BoundNetLog());
// Close streams to make spdy_session and spdy_session1 inactive.
session->CloseCreatedStream(spdy_stream, OK);
+ EXPECT_EQ(NULL, spdy_stream.get());
session1->CloseCreatedStream(spdy_stream1, OK);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Check spdy_session and spdy_session1 are not closed.
EXPECT_FALSE(session->is_active());
@@ -988,10 +989,8 @@
EXPECT_TRUE(session2->is_active());
EXPECT_FALSE(session2->IsClosed());
- spdy_stream = NULL;
- spdy_stream1 = NULL;
spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
spdy_session_pool->Remove(session2);
session2 = NULL;
break;
@@ -1121,15 +1120,19 @@
GURL url("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
+ spdy_stream2->SetDelegate(&delegate2);
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
@@ -1150,14 +1153,10 @@
spdy_stream2->SendRequest(false);
MessageLoop::current()->RunUntilIdle();
- EXPECT_EQ(3u, spdy_stream1->stream_id());
- EXPECT_EQ(1u, spdy_stream2->stream_id());
-
- spdy_stream1->Cancel();
- spdy_stream1 = NULL;
-
- spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(NULL, spdy_stream2.get());
+ EXPECT_EQ(3u, delegate1.stream_id());
+ EXPECT_EQ(1u, delegate2.stream_id());
}
TEST_F(SpdySessionSpdy2Test, CancelStream) {
@@ -1192,16 +1191,20 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
GURL url2("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url2, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
+ spdy_stream2->SetDelegate(&delegate2);
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
@@ -1224,17 +1227,17 @@
EXPECT_EQ(0u, spdy_stream1->stream_id());
spdy_stream1->Cancel();
+ EXPECT_EQ(NULL, spdy_stream1.get());
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(1);
- EXPECT_EQ(0u, spdy_stream1->stream_id());
- EXPECT_EQ(1u, spdy_stream2->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
+ EXPECT_EQ(1u, delegate2.stream_id());
- spdy_stream1 = NULL;
spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
}
TEST_F(SpdySessionSpdy2Test, CloseSessionWithTwoCreatedStreams) {
@@ -1265,13 +1268,13 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
GURL url2("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url2, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
@@ -1287,12 +1290,12 @@
spdy_stream1->set_spdy_headers(headers.Pass());
EXPECT_TRUE(spdy_stream1->HasUrl());
- test::ClosingDelegate delegate1(spdy_stream1.get());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
spdy_stream1->SetDelegate(&delegate1);
spdy_stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(spdy_stream2->HasUrl());
- test::ClosingDelegate delegate2(spdy_stream2.get());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->SendRequest(false);
@@ -1305,11 +1308,11 @@
// Ensure we don't crash while closing the session.
session->CloseSessionOnError(ERR_ABORTED, true, std::string());
- EXPECT_TRUE(spdy_stream1->closed());
- EXPECT_TRUE(spdy_stream2->closed());
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(NULL, spdy_stream2.get());
- spdy_stream1 = NULL;
- spdy_stream2 = NULL;
+ EXPECT_TRUE(delegate1.StreamIsClosed());
+ EXPECT_TRUE(delegate2.StreamIsClosed());
}
TEST_F(SpdySessionSpdy2Test, VerifyDomainAuthentication) {
@@ -1509,10 +1512,12 @@
data.RunFor(1);
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
TestCompletionCallback callback2;
GURL url2("http://www.google.com");
@@ -1549,33 +1554,42 @@
spdy_stream1->SendRequest(false);
// Run until 1st stream is closed and 2nd one is opened.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(3);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ // Pump loop for SpdySession::ProcessPendingStreamRequests().
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(2u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
- scoped_refptr<SpdyStream> stream2 = request2.ReleaseStream();
+ base::WeakPtr<SpdyStream> stream2 = request2.ReleaseStream();
+ test::StreamDelegateDoNothing delegate2(stream2);
+ stream2->SetDelegate(&delegate2);
stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(stream2->HasUrl());
stream2->SendRequest(false);
// Run until 2nd stream is closed.
- EXPECT_EQ(0u, stream2->stream_id());
+ EXPECT_EQ(0u, delegate2.stream_id());
data.RunFor(3);
- EXPECT_EQ(3u, stream2->stream_id());
+ EXPECT_EQ(NULL, stream2.get());
+ EXPECT_EQ(3u, delegate2.stream_id());
EXPECT_EQ(1u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
- scoped_refptr<SpdyStream> stream3 = request3.ReleaseStream();
+ base::WeakPtr<SpdyStream> stream3 = request3.ReleaseStream();
ASSERT_TRUE(stream3.get() != NULL);
+ test::StreamDelegateDoNothing delegate3(stream3);
+ stream3->SetDelegate(&delegate3);
stream3->set_spdy_headers(headers3.Pass());
EXPECT_TRUE(stream3->HasUrl());
stream3->SendRequest(false);
- EXPECT_EQ(0u, stream3->stream_id());
+ EXPECT_EQ(0u, delegate3.stream_id());
data.RunFor(4);
- EXPECT_EQ(5u, stream3->stream_id());
+ EXPECT_EQ(NULL, stream3.get());
+ EXPECT_EQ(5u, delegate3.stream_id());
EXPECT_EQ(0u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
}
@@ -1608,7 +1622,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -1635,19 +1649,23 @@
// Cancel the first stream, this will allow the second stream to be created.
EXPECT_TRUE(spdy_stream1.get() != NULL);
spdy_stream1->Cancel();
- spdy_stream1 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
callback2.WaitForResult();
EXPECT_EQ(2u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
// Cancel the second stream, this will allow the third stream to be created.
- request2.ReleaseStream()->Cancel();
+ base::WeakPtr<SpdyStream> spdy_stream2 = request2.ReleaseStream();
+ spdy_stream2->Cancel();
+ EXPECT_EQ(NULL, spdy_stream2.get());
EXPECT_EQ(1u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
// Cancel the third stream.
- request3.ReleaseStream()->Cancel();
+ base::WeakPtr<SpdyStream> spdy_stream3 = request3.ReleaseStream();
+ spdy_stream3->Cancel();
+ EXPECT_EQ(NULL, spdy_stream3.get());
EXPECT_EQ(0u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
}
@@ -1765,10 +1783,12 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
@@ -1785,13 +1805,14 @@
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has not posted a task.
data.RunFor(4);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is zero, which indicates DoRead read
// all the available data.
@@ -1855,10 +1876,12 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
@@ -1875,13 +1898,14 @@
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has posted a task.
data.RunFor(6);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is 1, which indicates DoRead has
// posted only one task and thus yielded though there is data available for it
@@ -1968,10 +1992,12 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
@@ -1988,13 +2014,14 @@
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has posted a task.
data.RunFor(12);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is 1, which indicates DoRead has
// posted only one task and thus yielded though there is data available for
@@ -2043,8 +2070,9 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
+ session = NULL;
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -2064,19 +2092,13 @@
data.RunFor(1);
EXPECT_EQ(1u, spdy_stream1->stream_id());
- // Drop the reference to the session.
- session = NULL;
+ // Only references to SpdySession are held by DoLoop and
+ // SpdySessionPool. If DoLoop doesn't hold the reference, we get a
+ // crash if SpdySession is deleted from the SpdySessionPool.
// Run until GoAway.
- data.RunFor(2);
-
- // Drop the reference to the stream which deletes its reference to the
- // SpdySession. Only references to SpdySession are held by DoLoop and
- // SpdySessionPool. If DoLoop doesn't hold the reference, we get a crash if
- // SpdySession is deleted from the SpdySessionPool.
- spdy_stream1 = NULL;
-
- data.RunFor(2);
+ data.RunFor(4);
+ EXPECT_EQ(NULL, spdy_stream1.get());
EXPECT_TRUE(data.at_write_eof());
EXPECT_TRUE(data.at_read_eof());
}
diff --git a/net/spdy/spdy_session_spdy3_unittest.cc b/net/spdy/spdy_session_spdy3_unittest.cc
index 05190f1..6c7313a 100644
--- a/net/spdy/spdy_session_spdy3_unittest.cc
+++ b/net/spdy/spdy_session_spdy3_unittest.cc
@@ -223,10 +223,10 @@
EXPECT_EQ(3, session->GetProtocolVersion());
GURL url("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
@@ -264,8 +264,7 @@
scoped_refptr<SpdySession> session2 = GetSession(pair_);
spdy_stream1->Close();
- spdy_stream1 = NULL;
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Delete the first session.
session = NULL;
@@ -273,6 +272,7 @@
// Delete the second session.
spdy_session_pool_->Remove(session2);
session2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
}
TEST_F(SpdySessionSpdy3Test, ClientPing) {
@@ -301,7 +301,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -356,7 +356,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -370,6 +370,7 @@
// Delete the session.
session = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
}
TEST_F(SpdySessionSpdy3Test, DeleteExpiredPushStreams) {
@@ -399,6 +400,7 @@
false, session->net_log_));
stream->set_spdy_headers(request_headers.Pass());
session->ActivateStream(stream);
+ stream = NULL;
SpdyHeaderBlock headers;
headers[":scheme"] = "http";
@@ -454,7 +456,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
test::StreamDelegateSendImmediate delegate(
@@ -488,6 +490,7 @@
// Delete the first session.
session = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
}
TEST_F(SpdySessionSpdy3Test, CloseIdleSessions) {
@@ -512,7 +515,7 @@
InitializeSession(
http_session_.get(), session1.get(), test_host_port_pair1));
GURL url1(kTestHost1);
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session1, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
@@ -527,7 +530,7 @@
InitializeSession(
http_session_.get(), session2.get(), test_host_port_pair2));
GURL url2(kTestHost2);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session2, url2, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
@@ -542,7 +545,7 @@
InitializeSession(
http_session_.get(), session3.get(), test_host_port_pair3));
GURL url3(kTestHost3);
- scoped_refptr<SpdyStream> spdy_stream3 =
+ base::WeakPtr<SpdyStream> spdy_stream3 =
CreateStreamSynchronously(session3, url3, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream3.get() != NULL);
@@ -566,7 +569,9 @@
// Make sessions 1 and 3 inactive, but keep them open.
// Session 2 still open and active
session1->CloseCreatedStream(spdy_stream1, OK);
+ EXPECT_EQ(NULL, spdy_stream1.get());
session3->CloseCreatedStream(spdy_stream3, OK);
+ EXPECT_EQ(NULL, spdy_stream3.get());
EXPECT_FALSE(session1->is_active());
EXPECT_FALSE(session1->IsClosed());
EXPECT_TRUE(session2->is_active());
@@ -590,6 +595,7 @@
// Make 2 not active
session2->CloseCreatedStream(spdy_stream2, OK);
+ EXPECT_EQ(NULL, spdy_stream2.get());
EXPECT_FALSE(session2->is_active());
EXPECT_FALSE(session2->IsClosed());
@@ -641,21 +647,17 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
- StreamReleaserCallback stream_releaser(session, spdy_stream1);
-
+ StreamReleaserCallback stream_releaser;
SpdyStreamRequest request;
ASSERT_EQ(ERR_IO_PENDING,
request.StartRequest(session, test_url_, MEDIUM,
BoundNetLog(),
stream_releaser.MakeCallback(&request)));
-
- // Make sure |stream_releaser| holds the last refs.
session = NULL;
- spdy_stream1 = NULL;
EXPECT_EQ(OK, stream_releaser.WaitForResult());
}
@@ -705,11 +707,11 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
- StreamReleaserCallback stream_releaser(session, spdy_stream1);
+ StreamReleaserCallback stream_releaser;
SpdyStreamRequest request;
ASSERT_EQ(ERR_IO_PENDING,
@@ -717,7 +719,6 @@
BoundNetLog(),
stream_releaser.MakeCallback(&request)));
- spdy_stream1 = NULL;
EXPECT_EQ(OK, stream_releaser.WaitForResult());
// Make sure that persisted data is cleared.
@@ -726,8 +727,6 @@
// Make sure session's max_concurrent_streams is 2.
EXPECT_EQ(2u, session->max_concurrent_streams());
-
- session = NULL;
}
// Start with max concurrent streams set to 1. Request two streams. When the
@@ -762,7 +761,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
// Create 2 streams. First will succeed. Second will be pending.
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
@@ -778,7 +777,7 @@
// Release the first one, this will allow the second to be created.
spdy_stream1->Cancel();
- spdy_stream1 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
request.CancelRequest();
callback.reset();
@@ -1048,18 +1047,20 @@
break;
case SPDY_POOL_CLOSE_IDLE_SESSIONS:
GURL url(test_hosts[0].url);
- scoped_refptr<SpdyStream> spdy_stream =
+ base::WeakPtr<SpdyStream> spdy_stream =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
GURL url1(test_hosts[1].url);
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session1, url1, MEDIUM, BoundNetLog());
GURL url2(test_hosts[2].url);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session2, url2, MEDIUM, BoundNetLog());
// Close streams to make spdy_session and spdy_session1 inactive.
session->CloseCreatedStream(spdy_stream, OK);
+ EXPECT_EQ(NULL, spdy_stream.get());
session1->CloseCreatedStream(spdy_stream1, OK);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Check spdy_session and spdy_session1 are not closed.
EXPECT_FALSE(session->is_active());
@@ -1081,10 +1082,10 @@
EXPECT_TRUE(session2->is_active());
EXPECT_FALSE(session2->IsClosed());
- spdy_stream = NULL;
- spdy_stream1 = NULL;
spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream.get());
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(NULL, spdy_stream2.get());
spdy_session_pool->Remove(session2);
session2 = NULL;
break;
@@ -1265,15 +1266,19 @@
GURL url("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
+ spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url.spec()));
@@ -1287,14 +1292,10 @@
spdy_stream2->SendRequest(false);
MessageLoop::current()->RunUntilIdle();
- EXPECT_EQ(3u, spdy_stream1->stream_id());
- EXPECT_EQ(1u, spdy_stream2->stream_id());
-
- spdy_stream1->Cancel();
- spdy_stream1 = NULL;
-
- spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(NULL, spdy_stream2.get());
+ EXPECT_EQ(3u, delegate1.stream_id());
+ EXPECT_EQ(1u, delegate2.stream_id());
}
TEST_F(SpdySessionSpdy3Test, CancelStream) {
@@ -1329,16 +1330,20 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
GURL url2("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url2, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
+ spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url1.spec()));
@@ -1354,17 +1359,17 @@
EXPECT_EQ(0u, spdy_stream1->stream_id());
spdy_stream1->Cancel();
+ EXPECT_EQ(NULL, spdy_stream1.get());
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(1);
- EXPECT_EQ(0u, spdy_stream1->stream_id());
- EXPECT_EQ(1u, spdy_stream2->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
+ EXPECT_EQ(1u, delegate2.stream_id());
- spdy_stream1 = NULL;
spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
}
TEST_F(SpdySessionSpdy3Test, CloseSessionWithTwoCreatedStreams) {
@@ -1395,13 +1400,13 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, HIGHEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
GURL url2("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url2, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(0u, spdy_stream2->stream_id());
@@ -1409,13 +1414,13 @@
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url1.spec()));
EXPECT_TRUE(spdy_stream1->HasUrl());
- test::ClosingDelegate delegate1(spdy_stream1.get());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
spdy_stream1->SetDelegate(&delegate1);
spdy_stream2->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url2.spec()));
EXPECT_TRUE(spdy_stream2->HasUrl());
- test::ClosingDelegate delegate2(spdy_stream2.get());
+ test::StreamDelegateDoNothing delegate2(spdy_stream2);
spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->SendRequest(false);
@@ -1428,11 +1433,12 @@
// Ensure we don't crash while closing the session.
session->CloseSessionOnError(ERR_ABORTED, true, std::string());
- EXPECT_TRUE(spdy_stream1->closed());
- EXPECT_TRUE(spdy_stream2->closed());
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(NULL, spdy_stream2.get());
- spdy_stream1 = NULL;
- spdy_stream2 = NULL;
+ EXPECT_TRUE(delegate1.StreamIsClosed());
+ EXPECT_TRUE(delegate2.StreamIsClosed());
+
session = NULL;
}
@@ -1633,10 +1639,12 @@
data.RunFor(1);
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
TestCompletionCallback callback2;
GURL url2("http://www.google.com");
@@ -1663,34 +1671,41 @@
spdy_stream1->SendRequest(false);
// Run until 1st stream is closed and 2nd one is opened.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(3);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ // Pump loop for SpdySession::ProcessPendingStreamRequests().
+ MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(NULL, spdy_stream1.get());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(2u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
- scoped_refptr<SpdyStream> stream2 = request2.ReleaseStream();
- stream2->set_spdy_headers(
- spdy_util_.ConstructGetHeaderBlock(url2.spec()));
+ base::WeakPtr<SpdyStream> stream2 = request2.ReleaseStream();
+ test::StreamDelegateDoNothing delegate2(stream2);
+ stream2->SetDelegate(&delegate2);
+ stream2->set_spdy_headers(spdy_util_.ConstructGetHeaderBlock(url2.spec()));
EXPECT_TRUE(stream2->HasUrl());
stream2->SendRequest(false);
// Run until 2nd stream is closed.
- EXPECT_EQ(0u, stream2->stream_id());
+ EXPECT_EQ(0u, delegate2.stream_id());
data.RunFor(3);
- EXPECT_EQ(3u, stream2->stream_id());
+ EXPECT_EQ(NULL, stream2.get());
+ EXPECT_EQ(3u, delegate2.stream_id());
EXPECT_EQ(1u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
- scoped_refptr<SpdyStream> stream3 = request3.ReleaseStream();
- stream3->set_spdy_headers(
- spdy_util_.ConstructGetHeaderBlock(url3.spec()));
+ base::WeakPtr<SpdyStream> stream3 = request3.ReleaseStream();
+ test::StreamDelegateDoNothing delegate3(stream3);
+ stream3->SetDelegate(&delegate3);
+ stream3->set_spdy_headers(spdy_util_.ConstructGetHeaderBlock(url3.spec()));
EXPECT_TRUE(stream3->HasUrl());
stream3->SendRequest(false);
- EXPECT_EQ(0u, stream3->stream_id());
+ EXPECT_EQ(0u, delegate3.stream_id());
data.RunFor(4);
- EXPECT_EQ(5u, stream3->stream_id());
+ EXPECT_EQ(NULL, stream3.get());
+ EXPECT_EQ(5u, delegate3.stream_id());
EXPECT_EQ(0u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
}
@@ -1723,7 +1738,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, LOWEST, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -1750,19 +1765,23 @@
// Cancel the first stream, this will allow the second stream to be created.
EXPECT_TRUE(spdy_stream1.get() != NULL);
spdy_stream1->Cancel();
- spdy_stream1 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
callback2.WaitForResult();
EXPECT_EQ(2u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
// Cancel the second stream, this will allow the third stream to be created.
- request2.ReleaseStream()->Cancel();
+ base::WeakPtr<SpdyStream> spdy_stream2 = request2.ReleaseStream();
+ spdy_stream2->Cancel();
+ EXPECT_EQ(NULL, spdy_stream2.get());
EXPECT_EQ(1u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
// Cancel the third stream.
- request3.ReleaseStream()->Cancel();
+ base::WeakPtr<SpdyStream> spdy_stream3 = request3.ReleaseStream();
+ spdy_stream3->Cancel();
+ EXPECT_EQ(NULL, spdy_stream3.get());
EXPECT_EQ(0u, session->num_active_streams() + session->num_created_streams());
EXPECT_EQ(0u, session->pending_create_stream_queues(LOWEST));
}
@@ -1919,7 +1938,7 @@
CreateDeterministicNetworkSession();
scoped_refptr<SpdySession> session = CreateInitializedSession();
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
TestCompletionCallback callback1;
@@ -1932,14 +1951,14 @@
// Release the first one, this will allow the second to be created.
spdy_stream1->Cancel();
- spdy_stream1 = NULL;
+ EXPECT_EQ(NULL, spdy_stream1.get());
- scoped_refptr<SpdyStream> spdy_stream2 =
+ base::WeakPtr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, test_url_, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream2.get() != NULL);
EXPECT_EQ(spdy_stream2->send_window_size(), window_size);
spdy_stream2->Cancel();
- spdy_stream2 = NULL;
+ EXPECT_EQ(NULL, spdy_stream2.get());
}
// Test that SpdySession::DoRead reads data from the socket without yielding.
@@ -1996,10 +2015,12 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url1.spec()));
@@ -2010,13 +2031,14 @@
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has not posted a task.
data.RunFor(4);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is zero, which indicates DoRead read
// all the available data.
@@ -2080,27 +2102,31 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url1.spec()));
EXPECT_TRUE(spdy_stream1->HasUrl());
spdy_stream1->SendRequest(false);
+ spdy_stream1.reset();
// Set up the TaskObserver to verify SpdySession::DoRead posts a task.
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has posted a task.
data.RunFor(6);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is 1, which indicates DoRead has
// posted only one task and thus yielded though there is data available for it
@@ -2187,10 +2213,12 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
+ test::StreamDelegateDoNothing delegate1(spdy_stream1);
+ spdy_stream1->SetDelegate(&delegate1);
spdy_stream1->set_spdy_headers(
spdy_util_.ConstructGetHeaderBlock(url1.spec()));
@@ -2201,13 +2229,14 @@
SpdySessionTestTaskObserver observer("spdy_session.cc", "DoRead");
// Run until 1st read.
- EXPECT_EQ(0u, spdy_stream1->stream_id());
+ EXPECT_EQ(0u, delegate1.stream_id());
data.RunFor(2);
- EXPECT_EQ(1u, spdy_stream1->stream_id());
+ EXPECT_EQ(1u, delegate1.stream_id());
EXPECT_EQ(0u, observer.executed_count());
// Read all the data and verify SpdySession::DoRead has posted a task.
data.RunFor(12);
+ EXPECT_EQ(NULL, spdy_stream1.get());
// Verify task observer's executed_count is 1, which indicates DoRead has
// posted only one task and thus yielded though there is data available for
@@ -2256,8 +2285,9 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url1("http://www.google.com");
- scoped_refptr<SpdyStream> spdy_stream1 =
+ base::WeakPtr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url1, MEDIUM, BoundNetLog());
+ session = NULL;
ASSERT_TRUE(spdy_stream1.get() != NULL);
EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -2271,19 +2301,13 @@
data.RunFor(1);
EXPECT_EQ(1u, spdy_stream1->stream_id());
- // Drop the reference to the session.
- session = NULL;
+ // Only references to SpdySession are held by DoLoop and
+ // SpdySessionPool. If DoLoop doesn't hold the reference, we get a
+ // crash if SpdySession is deleted from the SpdySessionPool.
// Run until GoAway.
- data.RunFor(2);
-
- // Drop the reference to the stream which deletes its reference to the
- // SpdySession. Only references to SpdySession are held by DoLoop and
- // SpdySessionPool. If DoLoop doesn't hold the reference, we get a crash if
- // SpdySession is deleted from the SpdySessionPool.
- spdy_stream1 = NULL;
-
- data.RunFor(2);
+ data.RunFor(4);
+ EXPECT_EQ(NULL, spdy_stream1.get());
EXPECT_TRUE(data.at_write_eof());
EXPECT_TRUE(data.at_read_eof());
}
@@ -2540,7 +2564,7 @@
// A delegate that drops any received data.
class DropReceivedDataDelegate : public test::StreamDelegateSendImmediate {
public:
- DropReceivedDataDelegate(const scoped_refptr<SpdyStream>& stream,
+ DropReceivedDataDelegate(const base::WeakPtr<SpdyStream>& stream,
base::StringPiece data)
: StreamDelegateSendImmediate(
stream, scoped_ptr<SpdyHeaderBlock>(), data) {}
@@ -2607,7 +2631,7 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url(kStreamUrl);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
EXPECT_EQ(0u, stream->stream_id());
@@ -2632,6 +2656,7 @@
EXPECT_EQ(msg_data_size, session->session_unacked_recv_window_bytes_);
stream->Close();
+ EXPECT_EQ(NULL, stream.get());
EXPECT_EQ(OK, delegate.WaitForClose());
@@ -2684,13 +2709,13 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url(kStreamUrl);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
EXPECT_EQ(0u, stream->stream_id());
test::StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), msg_data);
+ stream, scoped_ptr<SpdyHeaderBlock>(), msg_data);
stream->SetDelegate(&delegate);
stream->set_spdy_headers(
@@ -2714,6 +2739,7 @@
// Closing the stream should increase the session's send window.
stream->Close();
+ EXPECT_EQ(NULL, stream.get());
EXPECT_EQ(kSpdySessionInitialWindowSize, session->session_send_window_size_);
@@ -2774,13 +2800,13 @@
scoped_refptr<SpdySession> session = CreateInitializedSession();
GURL url(kStreamUrl);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
EXPECT_EQ(0u, stream->stream_id());
test::StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), msg_data);
+ stream, scoped_ptr<SpdyHeaderBlock>(), msg_data);
stream->SetDelegate(&delegate);
stream->set_spdy_headers(
@@ -2839,6 +2865,7 @@
EXPECT_EQ(msg_data_size, session->session_unacked_recv_window_bytes_);
stream->Close();
+ EXPECT_EQ(NULL, stream.get());
EXPECT_EQ(OK, delegate.WaitForClose());
@@ -2895,11 +2922,11 @@
EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION,
session->flow_control_state());
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
- test::StreamDelegateWithBody delegate(stream.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate(stream, kBodyDataStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -3051,11 +3078,11 @@
EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION,
session->flow_control_state());
- scoped_refptr<SpdyStream> stream1 =
+ base::WeakPtr<SpdyStream> stream1 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream1.get() != NULL);
- test::StreamDelegateWithBody delegate1(stream1.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate1(stream1, kBodyDataStringPiece);
stream1->SetDelegate(&delegate1);
EXPECT_FALSE(stream1->HasUrl());
@@ -3070,11 +3097,11 @@
data.RunFor(3);
EXPECT_EQ(1u, stream1->stream_id());
- scoped_refptr<SpdyStream> stream2 =
+ base::WeakPtr<SpdyStream> stream2 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());
ASSERT_TRUE(stream2.get() != NULL);
- test::StreamDelegateWithBody delegate2(stream2.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate2(stream2, kBodyDataStringPiece);
stream2->SetDelegate(&delegate2);
EXPECT_FALSE(stream2->HasUrl());
@@ -3138,13 +3165,13 @@
// Delegate that closes a given stream after sending its body.
class StreamClosingDelegate : public test::StreamDelegateWithBody {
public:
- StreamClosingDelegate(const scoped_refptr<SpdyStream>& stream,
+ StreamClosingDelegate(const base::WeakPtr<SpdyStream>& stream,
base::StringPiece data)
: StreamDelegateWithBody(stream, data) {}
virtual ~StreamClosingDelegate() {}
- void set_stream_to_close(const scoped_refptr<SpdyStream>& stream_to_close) {
+ void set_stream_to_close(const base::WeakPtr<SpdyStream>& stream_to_close) {
stream_to_close_ = stream_to_close;
}
@@ -3152,13 +3179,13 @@
int rv = test::StreamDelegateWithBody::OnSendBody();
if (stream_to_close_) {
stream_to_close_->Close();
- stream_to_close_ = NULL;
+ EXPECT_EQ(NULL, stream_to_close_.get());
}
return rv;
}
private:
- scoped_refptr<SpdyStream> stream_to_close_;
+ base::WeakPtr<SpdyStream> stream_to_close_;
};
// Cause a stall by reducing the flow control send window to
@@ -3217,11 +3244,11 @@
EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION,
session->flow_control_state());
- scoped_refptr<SpdyStream> stream1 =
+ base::WeakPtr<SpdyStream> stream1 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream1.get() != NULL);
- test::StreamDelegateWithBody delegate1(stream1.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate1(stream1, kBodyDataStringPiece);
stream1->SetDelegate(&delegate1);
EXPECT_FALSE(stream1->HasUrl());
@@ -3236,11 +3263,11 @@
data.RunFor(3);
EXPECT_EQ(1u, stream1->stream_id());
- scoped_refptr<SpdyStream> stream2 =
+ base::WeakPtr<SpdyStream> stream2 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream2.get() != NULL);
- StreamClosingDelegate delegate2(stream2.get(), kBodyDataStringPiece);
+ StreamClosingDelegate delegate2(stream2, kBodyDataStringPiece);
stream2->SetDelegate(&delegate2);
EXPECT_FALSE(stream2->HasUrl());
@@ -3255,11 +3282,11 @@
data.RunFor(2);
EXPECT_EQ(3u, stream2->stream_id());
- scoped_refptr<SpdyStream> stream3 =
+ base::WeakPtr<SpdyStream> stream3 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream3.get() != NULL);
- test::StreamDelegateWithBody delegate3(stream3.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate3(stream3, kBodyDataStringPiece);
stream3->SetDelegate(&delegate3);
EXPECT_FALSE(stream3->HasUrl());
@@ -3293,8 +3320,8 @@
SpdyStreamId stream_id3 = stream3->stream_id();
// Close stream1 preemptively.
- stream1 = NULL;
session->CloseStream(stream_id1, ERR_CONNECTION_CLOSED);
+ EXPECT_EQ(NULL, stream1.get());
EXPECT_FALSE(session->IsStreamActive(stream_id1));
EXPECT_TRUE(session->IsStreamActive(stream_id2));
@@ -3302,8 +3329,8 @@
// Unstall stream2, which should then close stream3.
delegate2.set_stream_to_close(stream3);
- stream3 = NULL;
UnstallSessionSend(session, kBodyDataSize);
+ EXPECT_EQ(NULL, stream3.get());
EXPECT_FALSE(stream2->send_stalled_by_flow_control());
EXPECT_FALSE(session->IsStreamActive(stream_id1));
@@ -3311,6 +3338,7 @@
EXPECT_FALSE(session->IsStreamActive(stream_id3));
data.RunFor(3);
+ EXPECT_EQ(NULL, stream2.get());
EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate1.WaitForClose());
EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate2.WaitForClose());
@@ -3338,7 +3366,7 @@
// Delegate that closes a given session after sending its body.
class SessionClosingDelegate : public test::StreamDelegateWithBody {
public:
- SessionClosingDelegate(const scoped_refptr<SpdyStream>& stream,
+ SessionClosingDelegate(const base::WeakPtr<SpdyStream>& stream,
base::StringPiece data)
: StreamDelegateWithBody(stream, data) {}
@@ -3414,11 +3442,11 @@
EXPECT_EQ(SpdySession::FLOW_CONTROL_STREAM_AND_SESSION,
session->flow_control_state());
- scoped_refptr<SpdyStream> stream1 =
+ base::WeakPtr<SpdyStream> stream1 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream1.get() != NULL);
- SessionClosingDelegate delegate1(stream1.get(), kBodyDataStringPiece);
+ SessionClosingDelegate delegate1(stream1, kBodyDataStringPiece);
stream1->SetDelegate(&delegate1);
EXPECT_FALSE(stream1->HasUrl());
@@ -3433,11 +3461,11 @@
data.RunFor(3);
EXPECT_EQ(1u, stream1->stream_id());
- scoped_refptr<SpdyStream> stream2 =
+ base::WeakPtr<SpdyStream> stream2 =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream2.get() != NULL);
- test::StreamDelegateWithBody delegate2(stream2.get(), kBodyDataStringPiece);
+ test::StreamDelegateWithBody delegate2(stream2, kBodyDataStringPiece);
stream2->SetDelegate(&delegate2);
EXPECT_FALSE(stream2->HasUrl());
@@ -3467,8 +3495,6 @@
// Unstall stream1, which should then close the session.
delegate1.set_session_to_close(session);
- stream1 = NULL;
- stream2 = NULL;
UnstallSessionSend(session, kBodyDataSize);
session = NULL;
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 5a38a89a..a4ba04c 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -132,7 +132,6 @@
response_(new SpdyHeaderBlock),
io_state_(STATE_NONE),
response_status_(OK),
- cancelled_(false),
has_upload_data_(false),
net_log_(net_log),
send_bytes_(0),
@@ -153,14 +152,16 @@
if (pushed_) {
CHECK(response_received());
MessageLoop::current()->PostTask(
- FROM_HERE, base::Bind(&SpdyStream::PushedStreamReplayData, this));
+ FROM_HERE,
+ base::Bind(&SpdyStream::PushedStreamReplayData,
+ weak_ptr_factory_.GetWeakPtr()));
} else {
continue_buffering_data_ = false;
}
}
void SpdyStream::PushedStreamReplayData() {
- if (cancelled_ || !delegate_)
+ if (!delegate_)
return;
continue_buffering_data_ = false;
@@ -211,7 +212,6 @@
scoped_ptr<SpdyFrame> SpdyStream::ProduceHeaderFrame(
scoped_ptr<SpdyHeaderBlock> header_block) {
- CHECK(!cancelled());
// We must need to write stream data.
// Until the headers have been completely sent, we can not be sure
// that our stream_id is correct.
@@ -226,9 +226,9 @@
}
void SpdyStream::DetachDelegate() {
+ DCHECK(!closed());
delegate_ = NULL;
- if (!closed())
- Cancel();
+ Cancel();
}
const SpdyHeaderBlock& SpdyStream::spdy_headers() const {
@@ -438,7 +438,6 @@
if (ContainsUpperAscii(it->first)) {
session_->ResetStream(stream_id_, priority_, RST_STREAM_PROTOCOL_ERROR,
"Upper case characters in header: " + it->first);
- response_status_ = ERR_SPDY_PROTOCOL_ERROR;
return ERR_SPDY_PROTOCOL_ERROR;
}
}
@@ -474,7 +473,6 @@
if (ContainsUpperAscii(it->first)) {
session_->ResetStream(stream_id_, priority_, RST_STREAM_PROTOCOL_ERROR,
"Upper case characters in header: " + it->first);
- response_status_ = ERR_SPDY_PROTOCOL_ERROR;
return ERR_SPDY_PROTOCOL_ERROR;
}
@@ -561,7 +559,7 @@
NOTREACHED();
return;
}
- if (cancelled() || closed())
+ if (closed())
return;
just_completed_frame_type_ = frame_type;
just_completed_frame_size_ = frame_size;
@@ -588,22 +586,20 @@
}
void SpdyStream::Cancel() {
- if (cancelled())
- return;
-
- cancelled_ = true;
- if (session_->IsStreamActive(stream_id_))
+ if (stream_id_ != 0) {
session_->ResetStream(stream_id_, priority_,
RST_STREAM_CANCEL, std::string());
- else if (stream_id_ == 0)
- session_->CloseCreatedStream(this, RST_STREAM_CANCEL);
+ } else {
+ session_->CloseCreatedStream(GetWeakPtr(), RST_STREAM_CANCEL);
+ }
}
void SpdyStream::Close() {
- if (stream_id_ != 0)
+ if (stream_id_ != 0) {
session_->CloseStream(stream_id_, OK);
- else
- session_->CloseCreatedStream(this, OK);
+ } else {
+ session_->CloseCreatedStream(GetWeakPtr(), OK);
+ }
}
int SpdyStream::SendRequest(bool has_upload_data) {
@@ -643,7 +639,6 @@
// that our stream_id is correct.
DCHECK_GT(io_state_, STATE_SEND_HEADERS_COMPLETE);
CHECK_GT(stream_id_, 0u);
- CHECK(!cancelled());
scoped_ptr<SpdyBuffer> data_buffer(session_->CreateDataBuffer(
stream_id_, data, length, flags));
@@ -698,6 +693,10 @@
}
}
+base::WeakPtr<SpdyStream> SpdyStream::GetWeakPtr() {
+ return weak_ptr_factory_.GetWeakPtr();
+}
+
bool SpdyStream::HasUrl() const {
if (pushed_)
return response_received();
@@ -869,7 +868,6 @@
}
int SpdyStream::DoSendHeaders() {
- CHECK(!cancelled_);
io_state_ = STATE_SEND_HEADERS_COMPLETE;
session_->EnqueueStreamWrite(
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index d3ef69f..7e8c5758 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -119,8 +119,8 @@
void SetDelegate(Delegate* delegate);
Delegate* GetDelegate() { return delegate_; }
- // Detach delegate from the stream. It will cancel the stream if it was not
- // cancelled yet. It is safe to call multiple times.
+ // Detach the delegate from the stream, which must not yet be
+ // closed, and cancel it.
void DetachDelegate();
// Is this stream a pushed stream from the server.
@@ -264,10 +264,19 @@
// Called by the SpdySession to log stream related errors.
void LogStreamError(int status, const std::string& description);
+ // If this stream is active, reset it, and close it otherwise. In
+ // either case the stream is deleted.
void Cancel();
+
+ // Close this stream without sending a RST_STREAM and delete
+ // it.
void Close();
- bool cancelled() const { return cancelled_; }
+
+ // Returns whether or not this stream is closed. Note that the only
+ // time a stream is closed and not deleted is in its delegate's
+ // OnClose() method.
bool closed() const { return io_state_ == STATE_DONE; }
+
// TODO(satorux): This is only for testing. We should be able to remove
// this once crbug.com/113107 is addressed.
bool body_sent() const { return io_state_ > STATE_SEND_BODY_COMPLETE; }
@@ -302,6 +311,9 @@
// called only when the stream is still open.
void PossiblyResumeIfSendStalled();
+ // Must be used only by the SpdySession.
+ base::WeakPtr<SpdyStream> GetWeakPtr();
+
bool is_idle() const {
return io_state_ == STATE_OPEN || io_state_ == STATE_DONE;
}
@@ -418,7 +430,6 @@
// Not valid until the stream is closed.
int response_status_;
- bool cancelled_;
bool has_upload_data_;
BoundNetLog net_log_;
diff --git a/net/spdy/spdy_stream_spdy2_unittest.cc b/net/spdy/spdy_stream_spdy2_unittest.cc
index 0e14c4b8..7d5c1ac 100644
--- a/net/spdy/spdy_stream_spdy2_unittest.cc
+++ b/net/spdy/spdy_stream_spdy2_unittest.cc
@@ -116,12 +116,12 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
+ stream, scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -186,7 +186,7 @@
HostPortPair host_port_pair("server.example.com", 80);
InitializeSpdySession(session, host_port_pair);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, HIGHEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
scoped_ptr<SpdyHeaderBlock> message_headers(new SpdyHeaderBlock);
@@ -195,7 +195,7 @@
(*message_headers)["fin"] = "1";
StreamDelegateSendImmediate delegate(
- stream.get(), message_headers.Pass(), base::StringPiece("hello!", 6));
+ stream, message_headers.Pass(), base::StringPiece("hello!", 6));
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -308,12 +308,12 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, log.bound());
ASSERT_TRUE(stream.get() != NULL);
StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
+ stream, scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -327,7 +327,7 @@
EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose());
- const SpdyStreamId stream_id = stream->stream_id();
+ const SpdyStreamId stream_id = delegate.stream_id();
EXPECT_TRUE(delegate.send_headers_completed());
EXPECT_EQ("200", delegate.GetResponseHeaderValue("status"));
diff --git a/net/spdy/spdy_stream_spdy3_unittest.cc b/net/spdy/spdy_stream_spdy3_unittest.cc
index 643cfd8..cb08736 100644
--- a/net/spdy/spdy_stream_spdy3_unittest.cc
+++ b/net/spdy/spdy_stream_spdy3_unittest.cc
@@ -113,12 +113,12 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
+ stream, scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -183,7 +183,7 @@
HostPortPair host_port_pair("server.example.com", 80);
InitializeSpdySession(session, host_port_pair);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, HIGHEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
TestCompletionCallback callback;
@@ -193,7 +193,7 @@
(*message_headers)[":fin"] = "1";
StreamDelegateSendImmediate delegate(
- stream.get(), message_headers.Pass(), base::StringPiece("hello!", 6));
+ stream, message_headers.Pass(), base::StringPiece("hello!", 6));
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -309,12 +309,12 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, log.bound());
ASSERT_TRUE(stream.get() != NULL);
StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
+ stream, scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -328,7 +328,7 @@
EXPECT_EQ(ERR_CONNECTION_CLOSED, delegate.WaitForClose());
- const SpdyStreamId stream_id = stream->stream_id();
+ const SpdyStreamId stream_id = delegate.stream_id();
EXPECT_TRUE(delegate.send_headers_completed());
EXPECT_EQ("200", delegate.GetResponseHeaderValue(":status"));
@@ -385,12 +385,12 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, log.bound());
ASSERT_TRUE(stream.get() != NULL);
StreamDelegateSendImmediate delegate(
- stream.get(), scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
+ stream, scoped_ptr<SpdyHeaderBlock>(), kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -443,11 +443,11 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
- StreamDelegateWithBody delegate(stream.get(), kPostBodyStringPiece);
+ StreamDelegateWithBody delegate(stream, kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
@@ -527,11 +527,11 @@
InitializeSpdySession(session, host_port_pair_);
- scoped_refptr<SpdyStream> stream =
+ base::WeakPtr<SpdyStream> stream =
CreateStreamSynchronously(session, url, LOWEST, BoundNetLog());
ASSERT_TRUE(stream.get() != NULL);
- StreamDelegateWithBody delegate(stream.get(), kPostBodyStringPiece);
+ StreamDelegateWithBody delegate(stream, kPostBodyStringPiece);
stream->SetDelegate(&delegate);
EXPECT_FALSE(stream->HasUrl());
diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc
index 4af3382..76597ab2 100644
--- a/net/spdy/spdy_stream_test_util.cc
+++ b/net/spdy/spdy_stream_test_util.cc
@@ -16,7 +16,7 @@
namespace test {
ClosingDelegate::ClosingDelegate(
- const scoped_refptr<SpdyStream>& stream) : stream_(stream) {}
+ const base::WeakPtr<SpdyStream>& stream) : stream_(stream) {}
ClosingDelegate::~ClosingDelegate() {}
@@ -47,14 +47,15 @@
void ClosingDelegate::OnDataSent(size_t bytes_sent) {}
void ClosingDelegate::OnClose(int status) {
- if (stream_)
- stream_->Close();
- stream_ = NULL;
+ DCHECK(stream_);
+ stream_->Close();
+ DCHECK(!stream_);
}
StreamDelegateBase::StreamDelegateBase(
- const scoped_refptr<SpdyStream>& stream)
+ const base::WeakPtr<SpdyStream>& stream)
: stream_(stream),
+ stream_id_(0),
send_headers_completed_(false),
headers_sent_(0),
data_sent_(0) {
@@ -64,6 +65,8 @@
}
SpdySendStatus StreamDelegateBase::OnSendHeadersComplete() {
+ stream_id_ = stream_->stream_id();
+ EXPECT_NE(stream_id_, 0u);
send_headers_completed_ = true;
return NO_MORE_DATA_TO_SEND;
}
@@ -93,7 +96,8 @@
void StreamDelegateBase::OnClose(int status) {
if (!stream_)
return;
- stream_ = NULL;
+ stream_id_ = stream_->stream_id();
+ stream_.reset();
callback_.callback().Run(status);
}
@@ -120,8 +124,23 @@
return (it == response_.end()) ? std::string() : it->second;
}
+StreamDelegateDoNothing::StreamDelegateDoNothing(
+ const base::WeakPtr<SpdyStream>& stream)
+ : StreamDelegateBase(stream) {}
+
+StreamDelegateDoNothing::~StreamDelegateDoNothing() {
+}
+
+int StreamDelegateDoNothing::OnSendBody() {
+ return OK;
+}
+SpdySendStatus StreamDelegateDoNothing::OnSendBodyComplete(
+ size_t /*bytes_sent*/) {
+ return NO_MORE_DATA_TO_SEND;
+}
+
StreamDelegateSendImmediate::StreamDelegateSendImmediate(
- const scoped_refptr<SpdyStream>& stream,
+ const base::WeakPtr<SpdyStream>& stream,
scoped_ptr<SpdyHeaderBlock> headers,
base::StringPiece data)
: StreamDelegateBase(stream),
@@ -158,7 +177,7 @@
}
StreamDelegateWithBody::StreamDelegateWithBody(
- const scoped_refptr<SpdyStream>& stream,
+ const base::WeakPtr<SpdyStream>& stream,
base::StringPiece data)
: StreamDelegateBase(stream),
buf_(new DrainableIOBuffer(new StringIOBuffer(data.as_string()),
diff --git a/net/spdy/spdy_stream_test_util.h b/net/spdy/spdy_stream_test_util.h
index 1e59ad8..baf92a1 100644
--- a/net/spdy/spdy_stream_test_util.h
+++ b/net/spdy/spdy_stream_test_util.h
@@ -22,7 +22,7 @@
// to make sure that such an action is harmless.
class ClosingDelegate : public SpdyStream::Delegate {
public:
- explicit ClosingDelegate(const scoped_refptr<SpdyStream>& stream);
+ explicit ClosingDelegate(const base::WeakPtr<SpdyStream>& stream);
virtual ~ClosingDelegate();
// SpdyStream::Delegate implementation.
@@ -37,15 +37,18 @@
virtual void OnDataSent(size_t bytes_sent) OVERRIDE;
virtual void OnClose(int status) OVERRIDE;
+ // Returns whether or not the stream is closed.
+ bool StreamIsClosed() const { return !stream_; }
+
private:
- scoped_refptr<SpdyStream> stream_;
+ base::WeakPtr<SpdyStream> stream_;
};
// Base class with shared functionality for test delegate
// implementations below.
class StreamDelegateBase : public SpdyStream::Delegate {
public:
- explicit StreamDelegateBase(const scoped_refptr<SpdyStream>& stream);
+ explicit StreamDelegateBase(const base::WeakPtr<SpdyStream>& stream);
virtual ~StreamDelegateBase();
virtual SpdySendStatus OnSendHeadersComplete() OVERRIDE;
@@ -67,16 +70,24 @@
// a string.
std::string TakeReceivedData();
+ // Returns whether or not the stream is closed.
+ bool StreamIsClosed() const { return !stream_; }
+
+ // Returns the stream's ID. If called when the stream is closed,
+ // returns the stream's ID when it was open.
+ SpdyStreamId stream_id() const { return stream_id_; }
+
std::string GetResponseHeaderValue(const std::string& name) const;
bool send_headers_completed() const { return send_headers_completed_; }
int headers_sent() const { return headers_sent_; }
int data_sent() const { return data_sent_; }
protected:
- const scoped_refptr<SpdyStream>& stream() { return stream_; }
+ const base::WeakPtr<SpdyStream>& stream() { return stream_; }
private:
- scoped_refptr<SpdyStream> stream_;
+ base::WeakPtr<SpdyStream> stream_;
+ SpdyStreamId stream_id_;
TestCompletionCallback callback_;
bool send_headers_completed_;
SpdyHeaderBlock response_;
@@ -85,11 +96,22 @@
int data_sent_;
};
+// Test delegate that does nothing. Used to capture data about the
+// stream, e.g. its id when it was open.
+class StreamDelegateDoNothing : public StreamDelegateBase {
+ public:
+ StreamDelegateDoNothing(const base::WeakPtr<SpdyStream>& stream);
+ virtual ~StreamDelegateDoNothing();
+
+ virtual int OnSendBody() OVERRIDE;
+ virtual SpdySendStatus OnSendBodyComplete(size_t bytes_sent) OVERRIDE;
+};
+
// Test delegate that sends data immediately in OnResponseReceived().
class StreamDelegateSendImmediate : public StreamDelegateBase {
public:
// Both |headers| and |buf| can be NULL.
- StreamDelegateSendImmediate(const scoped_refptr<SpdyStream>& stream,
+ StreamDelegateSendImmediate(const base::WeakPtr<SpdyStream>& stream,
scoped_ptr<SpdyHeaderBlock> headers,
base::StringPiece data);
virtual ~StreamDelegateSendImmediate();
@@ -108,7 +130,7 @@
// Test delegate that sends body data.
class StreamDelegateWithBody : public StreamDelegateBase {
public:
- StreamDelegateWithBody(const scoped_refptr<SpdyStream>& stream,
+ StreamDelegateWithBody(const base::WeakPtr<SpdyStream>& stream,
base::StringPiece data);
virtual ~StreamDelegateWithBody();
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index 4f17b65..047f264 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -276,7 +276,7 @@
return true;
}
-scoped_refptr<SpdyStream> CreateStreamSynchronously(
+base::WeakPtr<SpdyStream> CreateStreamSynchronously(
const scoped_refptr<SpdySession>& session,
const GURL& url,
RequestPriority priority,
@@ -284,14 +284,11 @@
SpdyStreamRequest stream_request;
int rv = stream_request.StartRequest(session, url, priority, net_log,
CompletionCallback());
- return (rv == OK) ? stream_request.ReleaseStream() : NULL;
+ return
+ (rv == OK) ? stream_request.ReleaseStream() : base::WeakPtr<SpdyStream>();
}
-StreamReleaserCallback::StreamReleaserCallback(
- SpdySession* session,
- SpdyStream* first_stream)
- : session_(session),
- first_stream_(first_stream) {}
+StreamReleaserCallback::StreamReleaserCallback() {}
StreamReleaserCallback::~StreamReleaserCallback() {}
@@ -304,10 +301,6 @@
void StreamReleaserCallback::OnComplete(
SpdyStreamRequest* request, int result) {
- session_->CloseSessionOnError(ERR_FAILED, false, "On complete.");
- session_ = NULL;
- first_stream_->Cancel();
- first_stream_ = NULL;
request->ReleaseStream()->Cancel();
SetResult(result);
}
diff --git a/net/spdy/spdy_test_util_common.h b/net/spdy/spdy_test_util_common.h
index 5531a85f..1af36c81 100644
--- a/net/spdy/spdy_test_util_common.h
+++ b/net/spdy/spdy_test_util_common.h
@@ -130,30 +130,25 @@
// Tries to create a stream in |session| synchronously. Returns NULL
// on failure.
-scoped_refptr<SpdyStream> CreateStreamSynchronously(
+base::WeakPtr<SpdyStream> CreateStreamSynchronously(
const scoped_refptr<SpdySession>& session,
const GURL& url,
RequestPriority priority,
const BoundNetLog& net_log);
-// Helper class used by some tests to release two streams as soon as
-// one is created.
+// Helper class used by some tests to release a stream as soon as it's
+// created.
class StreamReleaserCallback : public TestCompletionCallbackBase {
public:
- StreamReleaserCallback(SpdySession* session,
- SpdyStream* first_stream);
+ StreamReleaserCallback();
virtual ~StreamReleaserCallback();
- // Returns a callback that releases |request|'s stream as well as
- // |first_stream|.
+ // Returns a callback that releases |request|'s stream.
CompletionCallback MakeCallback(SpdyStreamRequest* request);
private:
void OnComplete(SpdyStreamRequest* request, int result);
-
- scoped_refptr<SpdySession> session_;
- scoped_refptr<SpdyStream> first_stream_;
};
const size_t kSpdyCredentialSlotUnused = 0;
diff --git a/net/spdy/spdy_websocket_stream.cc b/net/spdy/spdy_websocket_stream.cc
index 3440af7..d503fa2 100644
--- a/net/spdy/spdy_websocket_stream.cc
+++ b/net/spdy/spdy_websocket_stream.cc
@@ -20,7 +20,6 @@
SpdyWebSocketStream::SpdyWebSocketStream(
SpdySession* spdy_session, Delegate* delegate)
: weak_ptr_factory_(this),
- stream_(NULL),
spdy_session_(spdy_session),
delegate_(delegate) {
DCHECK(spdy_session_);
@@ -28,17 +27,8 @@
}
SpdyWebSocketStream::~SpdyWebSocketStream() {
- if (stream_) {
- // If Close() has not already been called, DetachDelegate() will send a
- // SPDY RST_STREAM. Deleting SpdyWebSocketStream is good enough to initiate
- // graceful shutdown, so we call Close() to avoid sending a RST_STREAM.
- // For safe, we should eliminate |delegate_| for OnClose() calback.
- delegate_ = NULL;
- stream_->Close();
- // The call to Close() should call into OnClose(), which should
- // set |stream_| to NULL.
- DCHECK(!stream_.get());
- }
+ delegate_ = NULL;
+ Close();
}
int SpdyWebSocketStream::InitializeStream(const GURL& url,
@@ -84,8 +74,10 @@
}
void SpdyWebSocketStream::Close() {
- if (stream_)
+ if (stream_) {
stream_->Close();
+ DCHECK(!stream_);
+ }
}
SpdySendStatus SpdyWebSocketStream::OnSendHeadersComplete() {
@@ -128,7 +120,7 @@
}
void SpdyWebSocketStream::OnClose(int status) {
- stream_ = NULL;
+ stream_.reset();
// Destruction without Close() call OnClose() with delegate_ being NULL.
if (!delegate_)
diff --git a/net/spdy/spdy_websocket_stream.h b/net/spdy/spdy_websocket_stream.h
index 02f6c7c..a589ae0 100644
--- a/net/spdy/spdy_websocket_stream.h
+++ b/net/spdy/spdy_websocket_stream.h
@@ -95,7 +95,7 @@
base::WeakPtrFactory<SpdyWebSocketStream> weak_ptr_factory_;
SpdyStreamRequest stream_request_;
- scoped_refptr<SpdyStream> stream_;
+ base::WeakPtr<SpdyStream> stream_;
scoped_refptr<SpdySession> spdy_session_;
Delegate* delegate_;
