Create frame iterator for frame types except HEADER, PUSH_PROMISE and DATA. This helps use unified …

net/spdy/core/spdy_framer.cc

Index: net/spdy/core/spdy_framer.cc
diff --git a/net/spdy/core/spdy_framer.cc b/net/spdy/core/spdy_framer.cc
index ce6cfef6f399f263c1a6680572b0a9ee7115caa1..0dc97c4da2885a6d7d26db0a801cbd19db466a0a 100644
--- a/net/spdy/core/spdy_framer.cc
+++ b/net/spdy/core/spdy_framer.cc
@@ -1674,7 +1674,7 @@ SpdyFramer::SpdyFrameIterator::SpdyFrameIterator(SpdyFramer* framer)
 
 SpdyFramer::SpdyFrameIterator::~SpdyFrameIterator() {}
 
-bool SpdyFramer::SpdyFrameIterator::NextFrame(ZeroCopyOutputBuffer* output) {
+size_t SpdyFramer::SpdyFrameIterator::NextFrame(ZeroCopyOutputBuffer* output) {
   SpdyFrameWithHeaderBlockIR* frame_ir = GetIR();
   if (frame_ir == nullptr) {
     LOG(WARNING) << "frame_ir doesn't exist.";
@@ -1712,7 +1712,9 @@ bool SpdyFramer::SpdyFrameIterator::NextFrame(ZeroCopyOutputBuffer* output) {
   if (is_first_frame_) {
     is_first_frame_ = false;
     frame_ir->set_end_headers(!has_next_frame_);
-    return SerializeGivenEncoding(*encoding, output);
+    size_t free_bytes_before = output->BytesFree();
+    bool ok = SerializeGivenEncoding(*encoding, output);
+    return ok ? free_bytes_before - output->BytesFree() : 0;
   } else {
     SpdyContinuationIR continuation_ir(frame_ir->stream_id());
     continuation_ir.set_end_headers(!has_next_frame_);
@@ -1774,6 +1776,55 @@ bool SpdyFramer::SpdyPushPromiseFrameIterator::SerializeGivenEncoding(
                                                         encoding, output);
 }
 
+SpdyFramer::SpdyControlFrameIterator::SpdyControlFrameIterator(
+    SpdyFramer* framer,
+    std::unique_ptr<SpdyFrameIR> frame_ir)
+    : framer_(framer), frame_ir_(std::move(frame_ir)) {}
+
+SpdyFramer::SpdyControlFrameIterator::~SpdyControlFrameIterator() {}
+
+size_t SpdyFramer::SpdyControlFrameIterator::NextFrame(
+    ZeroCopyOutputBuffer* output) {
+  size_t size_written = framer_->SerializeFrame(*frame_ir_, output);
+  frame_ir_.reset();
+  return size_written;
+}
+
+bool SpdyFramer::SpdyControlFrameIterator::HasNextFrame() const {
+  return frame_ir_ != nullptr;
+}
+
+// TODO(yasong): remove all the down_casts.

[Lei Zhang, 2017/05/04 21:22:46]

I don't see any down_casts?

[yasong, 2017/05/09 20:42:06]

Good catch! This code is merged from google3, and down_cast is changed to
static_cast, but this comment is missed.

+std::unique_ptr<SpdyFrameSequence> SpdyFramer::CreateIterator(
+    SpdyFramer* framer,
+    std::unique_ptr<SpdyFrameIR> frame_ir) {
+  std::unique_ptr<SpdyFrameSequence> result = nullptr;

[Lei Zhang, 2017/05/04 21:22:46]

You don't have to explicitly initialize std::unique_ptrs, but if that's the norm
in SPDY code, then ok.

[Bence, 2017/05/09 18:27:51]

Acknowledged.

[yasong, 2017/05/09 20:42:06]

Will be Fixed by cl/155545539.

+  switch (frame_ir->frame_type()) {
+    case SpdyFrameType::DATA: {
+      DLOG(ERROR) << "Data should use a different path to write";
+      result = nullptr;

[Lei Zhang, 2017/05/04 21:22:46]

Or further set them to nullptr when they already are.

[Bence, 2017/05/09 18:27:52]

Acknowledged.

[yasong, 2017/05/09 20:42:06]

Fixed by 155536594.

+      break;
+    }
+    case SpdyFrameType::HEADERS: {
+      result = base::MakeUnique<SpdyHeaderFrameIterator>(
+          framer,
+          base::WrapUnique(static_cast<SpdyHeadersIR*>(frame_ir.get())));

[Lei Zhang, 2017/05/04 21:22:46]

Wait, how does this work? You can creating a new unique_ptr out of the raw
pointer in an existing unique_ptr? Doesn't this create double ownership?

[Bence, 2017/05/09 18:27:51]

Oops, good catch!  Thank you.

[yasong, 2017/05/09 20:42:06]

Fixed by 155536594.

+      break;
+    }
+    case SpdyFrameType::PUSH_PROMISE: {
+      result = base::MakeUnique<SpdyPushPromiseFrameIterator>(
+          framer,
+          base::WrapUnique(static_cast<SpdyPushPromiseIR*>(frame_ir.get())));
+      break;
+    }
+    default: {
+      result = base::MakeUnique<SpdyControlFrameIterator>(framer,
+                                                          std::move(frame_ir));
+    }
+  }
+  return result;
+}
+
 void SpdyFramer::SerializeDataBuilderHelper(const SpdyDataIR& data_ir,
                                             uint8_t* flags,
                                             int* num_padding_fields,
@@ -2706,7 +2757,7 @@ class FrameSerializationVisitorWithOutput : public SpdyFrameVisitor {
       : framer_(framer), output_(output), result_(false) {}
   ~FrameSerializationVisitorWithOutput() override {}
 
-  bool Result() { return result_; }
+  size_t Result() { return result_; }
 
   void VisitData(const SpdyDataIR& data) override {
     result_ = framer_->SerializeData(data, output_);
@@ -2750,11 +2801,12 @@ class FrameSerializationVisitorWithOutput : public SpdyFrameVisitor {
 
 }  // namespace
 
-bool SpdyFramer::SerializeFrame(const SpdyFrameIR& frame,
-                                ZeroCopyOutputBuffer* output) {
+size_t SpdyFramer::SerializeFrame(const SpdyFrameIR& frame,
+                                  ZeroCopyOutputBuffer* output) {
   FrameSerializationVisitorWithOutput visitor(this, output);
+  size_t free_bytes_before = output->BytesFree();
   frame.Visit(&visitor);
-  return visitor.Result();
+  return visitor.Result() ? free_bytes_before - output->BytesFree() : 0;
 }
 
 size_t SpdyFramer::GetNumberRequiredContinuationFrames(size_t size) {