SPDY Framer: Refactorings.

Pick up cl/48817545

Remove OnSynStreamCompressed from SpdyFramerVisitorInterface
in favour of SpdyFramerDebugVisitorInterface.

BUG=242152

Review URL: https://chromiumcodereview.appspot.com/16360010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210870 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 80 insertions, 77 deletions

diff --git a/net/quic/quic_spdy_decompressor.cc b/net/quic/quic_spdy_decompressor.cc
index acfcd0a..a083ca3 100644
--- a/net/quic/quic_spdy_decompressor.cc
+++ b/net/quic/quic_spdy_decompressor.cc
@@ -55,8 +55,6 @@ class SpdyFramerVisitor : public SpdyFramerVisitorInterface {
                         SpdyGoAwayStatus status) OVERRIDE {}
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) OVERRIDE {}
-  virtual void OnSynStreamCompressed(size_t uncompressed_size,
-                                     size_t compressed_size)  OVERRIDE {}
   void set_visitor(QuicSpdyDecompressor::Visitor* visitor) {
     DCHECK(visitor);
     visitor_ = visitor;
diff --git a/net/spdy/buffered_spdy_framer.cc b/net/spdy/buffered_spdy_framer.cc
index e62316c..8111925 100644
--- a/net/spdy/buffered_spdy_framer.cc
+++ b/net/spdy/buffered_spdy_framer.cc
@@ -29,6 +29,11 @@ void BufferedSpdyFramer::set_visitor(
   spdy_framer_.set_visitor(this);
 }
 
+void BufferedSpdyFramer::set_debug_visitor(
+    SpdyFramerDebugVisitorInterface* debug_visitor) {
+  spdy_framer_.set_debug_visitor(debug_visitor);
+}
+
 void BufferedSpdyFramer::OnError(SpdyFramer* spdy_framer) {
   DCHECK(spdy_framer);
   visitor_->OnError(spdy_framer->error_code());
@@ -187,12 +192,6 @@ void BufferedSpdyFramer::OnWindowUpdate(SpdyStreamId stream_id,
   visitor_->OnWindowUpdate(stream_id, delta_window_size);
 }
 
-void BufferedSpdyFramer::OnSynStreamCompressed(
-    size_t uncompressed_size,
-    size_t compressed_size) {
-  visitor_->OnSynStreamCompressed(uncompressed_size, compressed_size);
-}
-
 int BufferedSpdyFramer::protocol_version() {
   return spdy_framer_.protocol_version();
 }
diff --git a/net/spdy/buffered_spdy_framer.h b/net/spdy/buffered_spdy_framer.h
index a355843..91de594 100644
--- a/net/spdy/buffered_spdy_framer.h
+++ b/net/spdy/buffered_spdy_framer.h
@@ -82,12 +82,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramerVisitorInterface {
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) = 0;
 
-  // Called after a control frame has been compressed to allow the visitor
-  // to record compression statistics.
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) = 0;
-
  protected:
   virtual ~BufferedSpdyFramerVisitorInterface() {}
 
@@ -108,6 +102,11 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
   // visitor will be used.
   void set_visitor(BufferedSpdyFramerVisitorInterface* visitor);
 
+  // Set debug callbacks to be called from the framer. The debug visitor is
+  // completely optional and need not be set in order for normal operation.
+  // If this is called multiple times, only the last visitor will be used.
+  void set_debug_visitor(SpdyFramerDebugVisitorInterface* debug_visitor);
+
   // SpdyFramerVisitorInterface
   virtual void OnError(SpdyFramer* spdy_framer) OVERRIDE;
   virtual void OnSynStream(SpdyStreamId stream_id,
@@ -141,12 +140,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
                                  size_t length,
                                  bool fin) OVERRIDE;
 
-  // Called after a syn stream control frame has been compressed to
-  // allow the visitor to record compression statistics.
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE;
-
   // SpdyFramer methods.
   size_t ProcessInput(const char* data, size_t len);
   int protocol_version();
@@ -202,6 +195,10 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
     return spdy_framer_.GetControlFrameHeaderSize();
   }
 
+  size_t GetSynStreamMinimumSize() const {
+    return spdy_framer_.GetSynStreamMinimumSize();
+  }
+
   size_t GetFrameMinimumSize() const {
     return spdy_framer_.GetFrameMinimumSize();
   }
diff --git a/net/spdy/buffered_spdy_framer_unittest.cc b/net/spdy/buffered_spdy_framer_unittest.cc
index 921072f..4a7bd97 100644
--- a/net/spdy/buffered_spdy_framer_unittest.cc
+++ b/net/spdy/buffered_spdy_framer_unittest.cc
@@ -92,11 +92,6 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface {
                         SpdyGoAwayStatus status) OVERRIDE {
   }
 
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE {
-  }
-
   bool OnCredentialFrameData(const char*, size_t) {
     LOG(FATAL) << "Unexpected OnCredentialFrameData call.";
     return false;
diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index ed53b38..aeb42893 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -823,12 +823,6 @@ void SpdyFramer::ProcessControlFrameHeader(uint16 control_frame_type_field) {
     remaining_control_header_ = frame_size_without_variable_data -
         current_frame_buffer_length_;
 
-    if (debug_visitor_) {
-      debug_visitor_->OnReceiveCompressedFrame(current_frame_stream_id_,
-                                               current_frame_type_,
-                                               current_frame_length_);
-    }
-
     CHANGE_STATE(SPDY_CONTROL_FRAME_BEFORE_HEADER_BLOCK);
     return;
   }
@@ -1107,6 +1101,12 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
           }
 
           DCHECK(reader.IsDoneReading());
+          if (debug_visitor_) {
+            debug_visitor_->OnReceiveCompressedFrame(
+                current_frame_stream_id_,
+                current_frame_type_,
+                current_frame_length_);
+          }
           visitor_->OnSynStream(
               current_frame_stream_id_,
               associated_to_stream_id,
@@ -1131,6 +1131,12 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
             reader.Seek(2);
           }
           DCHECK(reader.IsDoneReading());
+          if (debug_visitor_) {
+            debug_visitor_->OnReceiveCompressedFrame(
+                current_frame_stream_id_,
+                current_frame_type_,
+                current_frame_length_);
+          }
           if (current_frame_type_ == SYN_REPLY) {
             visitor_->OnSynReply(
                 current_frame_stream_id_,
@@ -1583,9 +1589,6 @@ SpdySerializedFrame* SpdyFramer::SerializeSynStream(
   DCHECK_EQ(GetSynStreamMinimumSize(), builder.length());
   SerializeNameValueBlock(&builder, syn_stream);
 
-  if (visitor_)
-    visitor_->OnSynStreamCompressed(size, builder.length());
-
   if (debug_visitor_) {
     const size_t payload_len = GetSerializedLength(
         protocol_version(), &(syn_stream.name_value_block()));
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index caea743..783dd7c 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -219,14 +219,6 @@ class NET_EXPORT_PRIVATE SpdyFramerVisitorInterface {
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) = 0;
 
-  // Called after a control frame has been compressed to allow the visitor
-  // to record compression statistics.
-  //
-  // TODO(akalin): Upstream this function.
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) = 0;
-
   // Called when a BLOCKED frame has been parsed.
   virtual void OnBlocked(SpdyStreamId stream_id) {}
 };
@@ -237,7 +229,7 @@ class NET_EXPORT_PRIVATE SpdyFramerVisitorInterface {
 // operates.
 //
 // Most SPDY implementations need not bother with this interface at all.
-class SpdyFramerDebugVisitorInterface {
+class NET_EXPORT_PRIVATE SpdyFramerDebugVisitorInterface {
  public:
   virtual ~SpdyFramerDebugVisitorInterface() {}
 
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index b8919a9..0260210 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -62,12 +62,21 @@ class MockVisitor : public SpdyFramerVisitorInterface {
                               SpdyGoAwayStatus status));
   MOCK_METHOD2(OnWindowUpdate, void(SpdyStreamId stream_id,
                                     uint32 delta_window_size));
-  MOCK_METHOD2(OnSynStreamCompressed,
-               void(size_t uncompressed_length,
-                    size_t compressed_length));
   MOCK_METHOD1(OnBlocked, void(SpdyStreamId stream_id));
 };
 
+class MockDebugVisitor : public SpdyFramerDebugVisitorInterface {
+ public:
+  MOCK_METHOD4(OnSendCompressedFrame, void(SpdyStreamId stream_id,
+                                           SpdyFrameType type,
+                                           size_t payload_len,
+                                           size_t frame_len));
+
+  MOCK_METHOD3(OnReceiveCompressedFrame, void(SpdyStreamId stream_id,
+                                              SpdyFrameType type,
+                                              size_t frame_len));
+};
+
 class SpdyFramerTestUtil {
  public:
   // Decompress a single frame using the decompression context held by
@@ -215,10 +224,6 @@ class SpdyFramerTestUtil {
                            uint32 value) OVERRIDE {
       LOG(FATAL);
     }
-    virtual void OnSynStreamCompressed(
-        size_t uncompressed_size,
-        size_t compressed_size) OVERRIDE {
-    }
     virtual void OnPing(uint32 unique_id) OVERRIDE {
       LOG(FATAL);
     }
@@ -361,11 +366,6 @@ class TestSpdyVisitor : public SpdyFramerVisitorInterface,
     setting_count_++;
   }
 
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE {
-  }
-
   virtual void OnPing(uint32 unique_id) OVERRIDE {
     DLOG(FATAL);
   }
@@ -3769,10 +3769,12 @@ TEST_P(SpdyFramerTest, SynStreamFrameFlags) {
     SCOPED_TRACE(testing::Message() << "Flags " << flags);
 
     testing::StrictMock<test::MockVisitor> visitor;
+    testing::StrictMock<test::MockDebugVisitor> debug_visitor;
     SpdyFramer framer(spdy_version_);
     framer.set_visitor(&visitor);
+    framer.set_debug_visitor(&debug_visitor);
 
-    EXPECT_CALL(visitor, OnSynStreamCompressed(_, _));
+    EXPECT_CALL(debug_visitor, OnSendCompressedFrame(8, SYN_STREAM, _, _));
 
     SpdyHeaderBlock headers;
     headers["foo"] = "bar";
@@ -3783,6 +3785,7 @@ TEST_P(SpdyFramerTest, SynStreamFrameFlags) {
     if (flags & ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) {
       EXPECT_CALL(visitor, OnError(_));
     } else {
+      EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(8, SYN_STREAM, _));
       EXPECT_CALL(visitor, OnSynStream(8, 3, 1, 0, flags & CONTROL_FLAG_FIN,
                                        flags & CONTROL_FLAG_UNIDIRECTIONAL));
       EXPECT_CALL(visitor, OnControlFrameHeaderData(8, _, _))
@@ -4064,10 +4067,13 @@ TEST_P(SpdyFramerTest, EmptySynStream) {
   SpdyHeaderBlock headers;
 
   testing::StrictMock<test::MockVisitor> visitor;
+  testing::StrictMock<test::MockDebugVisitor> debug_visitor;
   SpdyFramer framer(spdy_version_);
   framer.set_visitor(&visitor);
+  framer.set_debug_visitor(&debug_visitor);
+
+  EXPECT_CALL(debug_visitor, OnSendCompressedFrame(1, SYN_STREAM, _, _));
 
-  EXPECT_CALL(visitor, OnSynStreamCompressed(_, _));
   scoped_ptr<SpdyFrame>
       frame(framer.CreateSynStream(1, 0, 1, 0, CONTROL_FLAG_NONE, true,
                                    &headers));
@@ -4084,6 +4090,7 @@ TEST_P(SpdyFramerTest, EmptySynStream) {
         spdy_version_);
   }
 
+  EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(1, SYN_STREAM, _));
   EXPECT_CALL(visitor, OnSynStream(1, 0, 1, 0, false, false));
   EXPECT_CALL(visitor, OnControlFrameHeaderData(1, NULL, 0));
 
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 2e2ae85..a5a9037 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -488,6 +488,7 @@ Error SpdySession::InitializeWithSocket(
   buffered_spdy_framer_.reset(
       new BufferedSpdyFramer(NPNToSpdyVersion(protocol), enable_compression_));
   buffered_spdy_framer_->set_visitor(this);
+  buffered_spdy_framer_->set_debug_visitor(this);
   UMA_HISTOGRAM_ENUMERATION("Net.SpdyVersion", protocol, kProtoMaximumVersion);
 
   net_log_.AddEvent(
@@ -1677,15 +1678,25 @@ void SpdySession::OnSetting(SpdySettingsIds id,
                  id, static_cast<SpdySettingsFlags>(flags), value));
 }
 
-void SpdySession::OnSynStreamCompressed(
-    size_t uncompressed_size,
-    size_t compressed_size) {
-  // Make sure we avoid early decimal truncation.
-  int compression_pct = 100 - (100 * compressed_size) / uncompressed_size;
-  UMA_HISTOGRAM_PERCENTAGE("Net.SpdySynStreamCompressionPercentage",
-                           compression_pct);
-}
+void SpdySession::OnSendCompressedFrame(
+    SpdyStreamId stream_id,
+    SpdyFrameType type,
+    size_t payload_len,
+    size_t frame_len) {
+  if (type != SYN_STREAM)
+    return;
 
+  DCHECK(buffered_spdy_framer_.get());
+  size_t compressed_len =
+      frame_len - buffered_spdy_framer_->GetSynStreamMinimumSize();
+
+  if (payload_len) {
+    // Make sure we avoid early decimal truncation.
+    int compression_pct = 100 - (100 * compressed_len) / payload_len;
+    UMA_HISTOGRAM_PERCENTAGE("Net.SpdySynStreamCompressionPercentage",
+                             compression_pct);
+  }
+}
 
 int SpdySession::OnInitialResponseHeadersReceived(
     const SpdyHeaderBlock& response_headers,
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 20ce147..7cabac0 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -29,6 +29,7 @@
 #include "net/spdy/buffered_spdy_framer.h"
 #include "net/spdy/spdy_buffer.h"
 #include "net/spdy/spdy_credential_state.h"
+#include "net/spdy/spdy_framer.h"
 #include "net/spdy/spdy_header_block.h"
 #include "net/spdy/spdy_protocol.h"
 #include "net/spdy/spdy_session_pool.h"
@@ -180,6 +181,7 @@ class NET_EXPORT_PRIVATE SpdyStreamRequest {
 
 class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
                                public BufferedSpdyFramerVisitorInterface,
+                               public SpdyFramerDebugVisitorInterface,
                                public LayeredPool {
  public:
   // TODO(akalin): Use base::TickClock when it becomes available.
@@ -697,9 +699,6 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
       SpdySettingsIds id, uint8 flags, uint32 value) OVERRIDE;
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) OVERRIDE;
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE;
   virtual void OnSynStream(SpdyStreamId stream_id,
                            SpdyStreamId associated_stream_id,
                            SpdyPriority priority,
@@ -716,6 +715,17 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
       bool fin,
       const SpdyHeaderBlock& headers) OVERRIDE;
 
+  // SpdyFramerDebugVisitorInterface
+  virtual void OnSendCompressedFrame(
+      SpdyStreamId stream_id,
+      SpdyFrameType type,
+      size_t payload_len,
+      size_t frame_len) OVERRIDE;
+  virtual void OnReceiveCompressedFrame(
+      SpdyStreamId stream_id,
+      SpdyFrameType type,
+      size_t frame_len) OVERRIDE {}
+
   // Called when bytes are consumed from a SpdyBuffer for a DATA frame
   // that is to be written or is being written. Increases the send
   // window size accordingly if some or all of the SpdyBuffer is being
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index 1574c87..1c0322d 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -244,9 +244,6 @@ class PriorityGetter : public BufferedSpdyFramerVisitorInterface {
                         SpdyGoAwayStatus status) OVERRIDE {}
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) OVERRIDE {}
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE {}
 
  private:
   SpdyPriority priority_;
diff --git a/net/tools/flip_server/spdy_interface.h b/net/tools/flip_server/spdy_interface.h
index 16df842..184907d 100644
--- a/net/tools/flip_server/spdy_interface.h
+++ b/net/tools/flip_server/spdy_interface.h
@@ -115,12 +115,6 @@ class SpdySM : public BufferedSpdyFramerVisitorInterface,
   virtual void OnWindowUpdate(SpdyStreamId stream_id,
                               uint32 delta_window_size) OVERRIDE {}
 
-  // Called after a control frame has been compressed to allow the visitor
-  // to record compression statistics.
-  virtual void OnSynStreamCompressed(
-      size_t uncompressed_size,
-      size_t compressed_size) OVERRIDE {}
-
  public:
   virtual size_t ProcessReadInput(const char* data, size_t len) OVERRIDE;
   virtual size_t ProcessWriteInput(const char* data, size_t len) OVERRIDE;