Unify SpdyStream::url_ and SpdyStream::url_from_header_block_.

SpdyStream::url_ used to be set to the raw URL; this CL changes that to
SimplifyUrlForRequest(url) (to remove username, password, and ref).  The
way this member is set is that SpdyStreamRequest::StartRequest() is
called with the URL, then SpdySession::TryCreateStream() is called,
which calls SpdySession::CreateStream(), which passes this URL to the
SpdyStream constructor, which initializes the const SpdyStream::url_
member.

SpdyStream::url_from_header_block_ used to be set to the URL passed in
to CreateSpdyHeadersFromHttpRequest() or
CreateSpdyHeadersFromHttpRequestForWebSocket(), then the URL extracted
using GetUrlFromHeaderBlock() (these two steps only keep host, port,
scheme, and path, and might do something else with CONNECT method
requests); this CL removes this member.  This member was set in
SpdyStream::SendRequestHeaders() and
SpdyStream::OnPushPromiseHeadersReceived().

Other than tests and fuzzers, SpdyStreamRequest::StartRequest() and
SpdyStream::SendRequestHeaders() are called in four places.  In
SpdyHttpStream and BidirectionalStreamSpdyImpl,
SpdyStreamRequest::StartRequest() is called with request_info_.url, and
later, CreateSpdyHeadersFromHttpRequest() is called with the same URL,
and the resulting SpdyHeaderBlock is passed in to
SpdyStream::SendRequestHeaders().  WebSocketHttp2HandshakeStream has a
similar pattern except it calls
CreateSpdyHeadersFromHttpRequestForWebSocket() instead of
CreateSpdyHeadersFromHttpRequest().  The final path is
HttpProxyClientSocketWrapper calling SpdyStreamRequest::StartRequest(),
then it creates a SpdyProxyClientSocket instance with endpoint_, which
passes the exact same URL to CreateSpdyHeadersFromHttpRequest() and
calls SpdyStream::SendRequestHeaders() with the resulting
SpdyHeaderBlock.  So this is the exact same dance but performed by two
classes instead of one.

For these four paths, this CL introduces some change: SpdyStream::url_
will have username, password, and ref (anchor) stripped off, but keep
scheme, host, port, path, query.  SpdyStream::url_from_header_block_
used to be scheme, host, port, path only, now url_ is used instead,
which has query in addition.

SpdyStream::url_from_header_block_ used to be set in
SpdyStream::OnPushPromiseHeadersReceived() as well.  There is absolutely
no change on this code path: this is called only in
SpdySession::TryCreatePushedStream().  Now the same method calls
SpdyUtils::GetPromisedUrlFromHeaders() on the headers and passes the
resulting URL to the SpdyStream constructor.  Later it calls
OnPushPromiseHeadersReceived() on this stream and passes in the exact
same headers, and OnPushPromiseHeadersReceived() used to call
GetPromisedUrlFromHeaders() on these headers and set
url_from_header_block_.  The result of this was identical values in
SpdyStream::url_ and SpdyStream::url_from_header_block_.  It is worth
noting that the URL used as a key for pushed streams does not change.

This CL results in only one remaining call site for
GetUrlFromHeaderBlock(), in QuicChromiumClientSession.  This will be
addressed in a follow-up CL.

Change-Id: I82369d94f573ff68b35e472aa06258aab8623a8e
Reviewed-on: https://chromium-review.googlesource.com/945188
Commit-Queue: Bence Béky <bnc@chromium.org>
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540445}

net/spdy/chromium/spdy_http_stream_unittest.cc

@@ -846,7 +846,7 @@ TEST_F(SpdyHttpStreamTest, SpdyURLTest) {
   EXPECT_THAT(http_stream->SendRequest(headers, &response, callback.callback()),
               IsError(ERR_IO_PENDING));
 
-  EXPECT_EQ(base_url, http_stream->stream()->GetUrlFromHeaders().spec());
+  EXPECT_EQ(base_url, http_stream->stream()->url().spec());
 
   callback.WaitForResult();
 

net/spdy/chromium/spdy_session.cc

@@ -27,6 +27,7 @@
 #include "base/values.h"
 #include "crypto/ec_private_key.h"
 #include "crypto/ec_signature_creator.h"
+#include "net/base/url_util.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/ct_policy_status.h"
@@ -626,10 +627,11 @@ int SpdyStreamRequest::StartRequest(
   DCHECK(!session_);
   DCHECK(!stream_);
   DCHECK(callback_.is_null());
+  DCHECK(url.is_valid()) << url.possibly_invalid_spec();
 
   type_ = type;
   session_ = session;
-  url_ = url;
+  url_ = SimplifyUrlForRequest(url);
   priority_ = priority;
   socket_tag_ = socket_tag;
   net_log_ = net_log;
@@ -1693,7 +1695,7 @@ void SpdySession::TryCreatePushStream(SpdyStreamId stream_id,
   }
 
   // Cross-origin push validation.
-  GURL associated_url(associated_it->second->GetUrlFromHeaders());
+  GURL associated_url(associated_it->second->url());
   if (associated_url.GetOrigin() != gurl.GetOrigin()) {
     if (is_trusted_proxy_) {
       if (!gurl.SchemeIs(url::kHttpScheme)) {

net/spdy/chromium/spdy_session_pool_unittest.cc

@@ -178,13 +178,12 @@ TEST_F(SpdySessionPoolTest, CloseCurrentIdleSessions) {
   CreateNetworkSession();
 
   // Set up session 1
-  const SpdyString kTestHost1("www.example.org");
-  HostPortPair test_host_port_pair1(kTestHost1, 80);
+  const GURL url1("https://www.example.org");
+  HostPortPair test_host_port_pair1(HostPortPair::FromURL(url1));
   SpdySessionKey key1(test_host_port_pair1, ProxyServer::Direct(),
                       PRIVACY_MODE_DISABLED, SocketTag());
   base::WeakPtr<SpdySession> session1 =
       CreateSpdySession(http_session_.get(), key1, NetLogWithSource());
-  GURL url1(kTestHost1);
   base::WeakPtr<SpdyStream> spdy_stream1 = CreateStreamSynchronously(
       SPDY_BIDIRECTIONAL_STREAM, session1, url1, MEDIUM, NetLogWithSource());
   ASSERT_TRUE(spdy_stream1);
@@ -193,13 +192,12 @@ TEST_F(SpdySessionPoolTest, CloseCurrentIdleSessions) {
   StaticSocketDataProvider data2(reads, arraysize(reads), nullptr, 0);
   data2.set_connect_data(connect_data);
   session_deps_.socket_factory->AddSocketDataProvider(&data2);
-  const SpdyString kTestHost2("mail.example.org");
-  HostPortPair test_host_port_pair2(kTestHost2, 80);
+  const GURL url2("https://mail.example.org");
+  HostPortPair test_host_port_pair2(HostPortPair::FromURL(url2));
   SpdySessionKey key2(test_host_port_pair2, ProxyServer::Direct(),
                       PRIVACY_MODE_DISABLED, SocketTag());
   base::WeakPtr<SpdySession> session2 =
       CreateSpdySession(http_session_.get(), key2, NetLogWithSource());
-  GURL url2(kTestHost2);
   base::WeakPtr<SpdyStream> spdy_stream2 = CreateStreamSynchronously(
       SPDY_BIDIRECTIONAL_STREAM, session2, url2, MEDIUM, NetLogWithSource());
   ASSERT_TRUE(spdy_stream2);
@@ -208,13 +206,12 @@ TEST_F(SpdySessionPoolTest, CloseCurrentIdleSessions) {
   StaticSocketDataProvider data3(reads, arraysize(reads), nullptr, 0);
   data3.set_connect_data(connect_data);
   session_deps_.socket_factory->AddSocketDataProvider(&data3);
-  const SpdyString kTestHost3("mail.example.com");
-  HostPortPair test_host_port_pair3(kTestHost3, 80);
+  const GURL url3("https://mail.example.com");
+  HostPortPair test_host_port_pair3(HostPortPair::FromURL(url3));
   SpdySessionKey key3(test_host_port_pair3, ProxyServer::Direct(),
                       PRIVACY_MODE_DISABLED, SocketTag());
   base::WeakPtr<SpdySession> session3 =
       CreateSpdySession(http_session_.get(), key3, NetLogWithSource());
-  GURL url3(kTestHost3);
   base::WeakPtr<SpdyStream> spdy_stream3 = CreateStreamSynchronously(
       SPDY_BIDIRECTIONAL_STREAM, session3, url3, MEDIUM, NetLogWithSource());
   ASSERT_TRUE(spdy_stream3);
@@ -347,7 +344,7 @@ void SpdySessionPoolTest::RunIPPoolingTest(
     SpdySessionKey key;
     AddressList addresses;
   } test_hosts[] = {
-      {"http:://www.example.org", "www.example.org",
+      {"http://www.example.org", "www.example.org",
        "192.0.2.33,192.168.0.1,192.168.0.5"},
       {"http://mail.example.org", "mail.example.org",
        "192.168.0.2,192.168.0.3,192.168.0.5,192.0.2.33"},

net/spdy/chromium/spdy_session_unittest.cc

@@ -4477,7 +4477,7 @@ void SpdySessionTest::RunResumeAfterUnstallTest(
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING,
             stream->SendRequestHeaders(std::move(headers), MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream->url().spec());
 
   stall_function.Run(stream.get());
 
@@ -4614,7 +4614,7 @@ TEST_F(SpdySessionTest, ResumeByPriorityAfterSendWindowSizeIncrease) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream1->SendRequestHeaders(std::move(headers1),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream1->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream1->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, stream1->stream_id());
@@ -4624,7 +4624,7 @@ TEST_F(SpdySessionTest, ResumeByPriorityAfterSendWindowSizeIncrease) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream2->SendRequestHeaders(std::move(headers2),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream2->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream2->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(3u, stream2->stream_id());
@@ -4718,7 +4718,7 @@ TEST_F(SpdySessionTest, ResumeSessionWithStalledStream) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream1->SendRequestHeaders(std::move(headers1),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream1->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream1->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, stream1->stream_id());
@@ -4728,7 +4728,7 @@ TEST_F(SpdySessionTest, ResumeSessionWithStalledStream) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream2->SendRequestHeaders(std::move(headers2),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream2->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream2->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(3u, stream2->stream_id());
@@ -4870,7 +4870,7 @@ TEST_F(SpdySessionTest, SendWindowSizeIncreaseWithDeletedStreams) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream1->SendRequestHeaders(std::move(headers1),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream1->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream1->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, stream1->stream_id());
@@ -4880,7 +4880,7 @@ TEST_F(SpdySessionTest, SendWindowSizeIncreaseWithDeletedStreams) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream2->SendRequestHeaders(std::move(headers2),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream2->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream2->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(3u, stream2->stream_id());
@@ -4890,7 +4890,7 @@ TEST_F(SpdySessionTest, SendWindowSizeIncreaseWithDeletedStreams) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream3->SendRequestHeaders(std::move(headers3),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream3->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream3->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(5u, stream3->stream_id());
@@ -4995,7 +4995,7 @@ TEST_F(SpdySessionTest, SendWindowSizeIncreaseWithDeletedSession) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream1->SendRequestHeaders(std::move(headers1),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream1->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream1->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1u, stream1->stream_id());
@@ -5005,7 +5005,7 @@ TEST_F(SpdySessionTest, SendWindowSizeIncreaseWithDeletedSession) {
       spdy_util_.ConstructPostHeaderBlock(kDefaultUrl, kBodyDataSize));
   EXPECT_EQ(ERR_IO_PENDING, stream2->SendRequestHeaders(std::move(headers2),
                                                         MORE_DATA_TO_SEND));
-  EXPECT_EQ(kDefaultUrl, stream2->GetUrlFromHeaders().spec());
+  EXPECT_EQ(kDefaultUrl, stream2->url().spec());
 
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(3u, stream2->stream_id());

net/spdy/chromium/spdy_stream.cc

@@ -473,7 +473,6 @@ void SpdyStream::OnPushPromiseHeadersReceived(SpdyHeaderBlock headers,
   io_state_ = STATE_RESERVED_REMOTE;
   request_headers_ = std::move(headers);
   request_headers_valid_ = true;
-  url_from_header_block_ = std::move(url);
 }
 
 void SpdyStream::OnDataReceived(std::unique_ptr<SpdyBuffer> buffer) {
@@ -706,7 +705,6 @@ int SpdyStream::SendRequestHeaders(SpdyHeaderBlock request_headers,
   CHECK_EQ(io_state_, STATE_IDLE);
   request_headers_ = std::move(request_headers);
   request_headers_valid_ = true;
-  url_from_header_block_ = GetUrlFromHeaderBlock(request_headers_);
   pending_send_status_ = send_status;
   session_->EnqueueStreamWrite(
       GetWeakPtr(), SpdyFrameType::HEADERS,
@@ -801,7 +799,6 @@ size_t SpdyStream::EstimateMemoryUsage() const {
   // once scoped_refptr support is in.
   return SpdyEstimateMemoryUsage(url_) +
          SpdyEstimateMemoryUsage(request_headers_) +
-         SpdyEstimateMemoryUsage(url_from_header_block_) +
          SpdyEstimateMemoryUsage(pending_recv_data_) +
          SpdyEstimateMemoryUsage(response_headers_);
 }

net/spdy/chromium/spdy_stream.h

@@ -379,10 +379,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
 
   const SpdyHeaderBlock& request_headers() { return request_headers_; }
 
-  // Get the URL from the appropriate stream headers, or the empty
-  // GURL() if it is unknown.
-  const GURL& GetUrlFromHeaders() const { return url_from_header_block_; }
-
   // Returns the estimate of dynamically allocated memory in bytes.
   size_t EstimateMemoryUsage() const;
 
@@ -484,9 +480,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
   bool request_headers_valid_;
   SpdyHeaderBlock request_headers_;
 
-  // The URL from the request headers.
-  GURL url_from_header_block_;
-
   // Data waiting to be sent, and the close state of the local endpoint
   // after the data is fully written.
   scoped_refptr<DrainableIOBuffer> pending_send_data_;