SpdySession::GetPushedStream() cleanup.

* Update hugely out-of-date comment for SpdySession::GetPushedStream().
* Drop unused |stream_net_log| argument.
* Do not reset |*stream| upon error.  The only production call site is
  in SpdyHttpStream::InitializeStream(), where |stream_| is already
  guaranteed to be nullptr when GetPushedStream() is called, see DCHECK
  in line 139.

Change-Id: I4b7b5738dd171df9f01fc1bfd7ddec8376985e08
Reviewed-on: https://chromium-review.googlesource.com/c/1257442Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596074}

net/spdy/spdy_http_stream.cc

@@ -142,9 +142,8 @@ int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info,
 
   request_info_ = request_info;
   if (pushed_stream_id_ != kNoPushedStreamFound) {
-    int error =
-        spdy_session_->GetPushedStream(request_info_->url, pushed_stream_id_,
-                                       priority, &stream_, stream_net_log);
+    int error = spdy_session_->GetPushedStream(
+        request_info_->url, pushed_stream_id_, priority, &stream_);
     if (error != OK)
       return error;
 

net/spdy/spdy_session.cc

@@ -920,8 +920,7 @@ SpdySession::~SpdySession() {
 int SpdySession::GetPushedStream(const GURL& url,
                                  spdy::SpdyStreamId pushed_stream_id,
                                  RequestPriority priority,
-                                 SpdyStream** stream,
-                                 const NetLogWithSource& stream_net_log) {
+                                 SpdyStream** stream) {
   CHECK(!in_io_loop_);
   // |pushed_stream_id| must be valid.
   DCHECK_NE(pushed_stream_id, kNoPushedStreamFound);
@@ -930,7 +929,6 @@ int SpdySession::GetPushedStream(const GURL& url,
             pool_->push_promise_index()->FindStream(url, this));
 
   if (availability_state_ == STATE_DRAINING) {
-    *stream = nullptr;
     return ERR_CONNECTION_CLOSED;
   }
 

net/spdy/spdy_session.h

@@ -317,31 +317,21 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   const SpdySessionKey& spdy_session_key() const {
     return spdy_session_key_;
   }
-  // Get a pushed stream for a given |url|.  If the server initiates a
-  // stream, it might already exist for a given path.  The server
-  // might also not have initiated the stream yet, but indicated it
-  // will via X-Associated-Content.  Returns OK if a stream was found
-  // and put into |spdy_stream|, or if one was not found but it is
-  // okay to create a new stream (in which case |spdy_stream| is
-  // reset).  Returns an error (not ERR_IO_PENDING) otherwise, and
-  // resets |spdy_stream|.
-  //
-  // If |pushed_stream_id != kNoPushedStreamFound|, then the pushed stream with
-  // pushed_stream_id is used.  An error is returned if that stream is not
-  // available.
-  //
-  // If |pushed_stream_id == kNoPushedStreamFound|, then any matching pushed
-  // stream that has not been claimed by another request can be used.  This can
-  // happen, for example, with http scheme pushed streams, or if the pushed
-  // stream was received from the server in the meanwhile.
+
+  // Get a pushed stream for a given |url| with stream ID |pushed_stream_id|.
+  // The caller must have already claimed the stream from Http2PushPromiseIndex.
+  // |pushed_stream_id| must not be kNoPushedStreamFound.
   //
-  // If a stream was found and the stream is still open, the priority
-  // of that stream is updated to match |priority|.
+  // Returns ERR_CONNECTION_CLOSED if the connection is being closed.
+  // Returns ERR_SPDY_PUSHED_STREAM_NOT_AVAILABLE if the pushed stream is not
+  //   available any longer, for example, if the server has reset it.
+  // Returns OK if the stream is still available, and returns the stream in
+  //   |*spdy_stream|.  If the stream is still open, updates its priority to
+  //   |priority|.
   int GetPushedStream(const GURL& url,
                       spdy::SpdyStreamId pushed_stream_id,
                       RequestPriority priority,
-                      SpdyStream** spdy_stream,
-                      const NetLogWithSource& stream_net_log);
+                      SpdyStream** spdy_stream);
 
   // Called when the pushed stream should be cancelled. If the pushed stream is
   // not claimed and active, sends RST to the server to cancel the stream.

net/spdy/spdy_session_unittest.cc

@@ -1650,7 +1650,7 @@ TEST_F(SpdySessionTestWithMockTime, ClaimPushedStreamBeforeExpires) {
 
   SpdyStream* spdy_stream2;
   int rv = session_->GetPushedStream(pushed_url, pushed_stream_id, MEDIUM,
-                                     &spdy_stream2, NetLogWithSource());
+                                     &spdy_stream2);
   ASSERT_THAT(rv, IsOk());
   ASSERT_TRUE(spdy_stream2);
 
@@ -5603,7 +5603,7 @@ TEST_F(SpdySessionTest, CancelReservedStreamOnHeadersReceived) {
 
   SpdyStream* pushed_stream;
   int rv = session_->GetPushedStream(pushed_url, pushed_stream_id, IDLE,
-                                     &pushed_stream, NetLogWithSource());
+                                     &pushed_stream);
   ASSERT_THAT(rv, IsOk());
   ASSERT_TRUE(pushed_stream);
   test::StreamDelegateCloseOnHeaders delegate2(pushed_stream->GetWeakPtr());
@@ -5687,7 +5687,7 @@ TEST_F(SpdySessionTest, GetPushedStream) {
   const GURL pushed_url(kPushedUrl);
   SpdyStream* pushed_stream;
   int rv = session_->GetPushedStream(pushed_url, 2 /* pushed_stream_id */, IDLE,
-                                     &pushed_stream, NetLogWithSource());
+                                     &pushed_stream);
   EXPECT_THAT(rv, IsError(ERR_SPDY_PUSHED_STREAM_NOT_AVAILABLE));
 
   // Read PUSH_PROMISE.
@@ -5718,13 +5718,13 @@ TEST_F(SpdySessionTest, GetPushedStream) {
   // GetPushedStream() should return an error if there does not exist a pushed
   // stream with ID |pushed_stream_id|.
   rv = session_->GetPushedStream(pushed_url, 4 /* pushed_stream_id */, IDLE,
-                                 &pushed_stream, NetLogWithSource());
+                                 &pushed_stream);
   EXPECT_THAT(rv, IsError(ERR_SPDY_PUSHED_STREAM_NOT_AVAILABLE));
 
   // GetPushedStream() should return OK and return the pushed stream in
   // |pushed_stream| outparam if |pushed_stream_id| matches.
   rv = session_->GetPushedStream(pushed_url, 2 /* pushed_stream_id */, IDLE,
-                                 &pushed_stream, NetLogWithSource());
+                                 &pushed_stream);
   EXPECT_THAT(rv, IsOk());
   ASSERT_TRUE(pushed_stream);
   test::StreamDelegateCloseOnHeaders delegate2(pushed_stream->GetWeakPtr());

net/spdy/spdy_stream_unittest.cc

@@ -357,7 +357,7 @@ TEST_F(SpdyStreamTest, PushedStream) {
 
   SpdyStream* push_stream;
   EXPECT_THAT(session->GetPushedStream(pushed_url, pushed_stream_id, IDLE,
-                                       &push_stream, NetLogWithSource()),
+                                       &push_stream),
               IsOk());
   ASSERT_TRUE(push_stream);
   EXPECT_EQ(kPushUrl, push_stream->url().spec());