Reduce SpdyHeaderBlock copies with move semantics.

net/quic/quic_http_stream.cc

Index: net/quic/quic_http_stream.cc
diff --git a/net/quic/quic_http_stream.cc b/net/quic/quic_http_stream.cc
index 0bec563cb3b543e549ba2b1dbdf7020d3e413cfa..de1db7c982b3369621de2732b6ca601e3a38116b 100644
--- a/net/quic/quic_http_stream.cc
+++ b/net/quic/quic_http_stream.cc
@@ -4,6 +4,8 @@
 
 #include "net/quic/quic_http_stream.h"
 
+#include <utility>
+
 #include "base/callback_helpers.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_split.h"
@@ -678,11 +680,11 @@ int QuicHttpStream::DoSendHeaders() {
   bool has_upload_data = request_body_stream_ != nullptr;
 
   next_state_ = STATE_SEND_HEADERS_COMPLETE;
-  size_t frame_len =
-      stream_->WriteHeaders(request_headers_, !has_upload_data, nullptr);
+  size_t frame_len = stream_->WriteHeaders(std::move(request_headers_),
+                                           !has_upload_data, nullptr);
   headers_bytes_sent_ += frame_len;
 
-  request_headers_.clear();
+  request_headers_ = SpdyHeaderBlock();

[Ryan Hamilton, 2016/06/23 18:23:18]

Doesn't move do the equivalent of clear()? maybe not? In any case, why assign to
a new object instead of clear()?

[Bence, 2016/06/23 23:36:16]

I believe the convention is that the moved-from object is left in valid but
unspecified state, so if this class had relied on |request_headers_| being empty
after this, then move would not have been sufficient.

However, I do not think clear() was necessary here in the first place.  The only
methods reading |request_headers_| are HandlePromise (called by SendRequest) and
DoSendHeaders, and I believe neither of them can be called by the state machine
after this point.  So I am removing this line here, just like you proposed,
though for a different reason.

[Ryan Hamilton, 2016/06/24 00:24:07]

Ah! Right. So clear() in this case was to ensure that the headers were no longer
consuming memory in the QuicHttpStream. So if the convention is that we don't
know what state request_headers_ is in, then it seems like we should clear() (or
copy from empty as your patch did) to be safe.

[Bence, 2016/06/24 11:01:42]

Done.

   return static_cast<int>(frame_len);
 }