Changes the type for PING id from uint32 to uint64.

Required for HTTP2. This lands server change 60786018 by birenroy. Minor type updates to QUIC/SpdySession & friends were also required. Review URL: https://codereview.chromium.org/154353003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@250767 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Feb 12, 2014 · bd254d5 · bd254d5
1 parent ccb8202
commit bd254d5
Show file tree

Hide file tree

Showing 19 changed files with 81 additions and 34 deletions.
diff --git a/net/quic/quic_headers_stream.cc b/net/quic/quic_headers_stream.cc
@@ -101,7 +101,7 @@ class QuicHeadersStream::SpdyFramerVisitor
     CloseConnection("SPDY SETTINGS frame recevied.");
   }
 
-  virtual void OnPing(uint32 unique_id) OVERRIDE {
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE {
     CloseConnection("SPDY PING frame recevied.");
   }
 

diff --git a/net/quic/quic_headers_stream_test.cc b/net/quic/quic_headers_stream_test.cc
@@ -47,7 +47,7 @@ class MockVisitor : public SpdyFramerVisitorInterface {
                                  SpdyRstStreamStatus status));
   MOCK_METHOD1(OnSettings, void(bool clear_persisted));
   MOCK_METHOD3(OnSetting, void(SpdySettingsIds id, uint8 flags, uint32 value));
-  MOCK_METHOD1(OnPing, void(uint32 unique_id));
+  MOCK_METHOD1(OnPing, void(SpdyPingId unique_id));
   MOCK_METHOD2(OnGoAway, void(SpdyStreamId last_accepted_stream_id,
                               SpdyGoAwayStatus status));
   MOCK_METHOD2(OnHeaders, void(SpdyStreamId stream_id, bool fin));

diff --git a/net/quic/quic_spdy_decompressor.cc b/net/quic/quic_spdy_decompressor.cc
@@ -44,7 +44,7 @@ class SpdyFramerVisitor : public SpdyFramerVisitorInterface {
   virtual void OnSetting(SpdySettingsIds id,
                          uint8 flags,
                          uint32 value) OVERRIDE {}
-  virtual void OnPing(uint32 unique_id) OVERRIDE {}
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE {}
   virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,
                         SpdyGoAwayStatus status) OVERRIDE {}
   virtual void OnHeaders(SpdyStreamId stream_id, bool fin) OVERRIDE {}

diff --git a/net/spdy/buffered_spdy_framer.cc b/net/spdy/buffered_spdy_framer.cc
@@ -186,7 +186,7 @@ void BufferedSpdyFramer::OnSetting(SpdySettingsIds id,
   visitor_->OnSetting(id, flags, value);
 }
 
-void BufferedSpdyFramer::OnPing(uint32 unique_id) {
+void BufferedSpdyFramer::OnPing(SpdyPingId unique_id) {
   visitor_->OnPing(unique_id);
 }
 

diff --git a/net/spdy/buffered_spdy_framer.h b/net/spdy/buffered_spdy_framer.h
@@ -78,7 +78,7 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramerVisitorInterface {
   virtual void OnSetting(SpdySettingsIds id, uint8 flags, uint32 value) = 0;
 
   // Called when a PING frame has been parsed.
-  virtual void OnPing(uint32 unique_id) = 0;
+  virtual void OnPing(SpdyPingId unique_id) = 0;
 
   // Called when a RST_STREAM frame has been parsed.
   virtual void OnRstStream(SpdyStreamId stream_id,
@@ -140,7 +140,7 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
   virtual void OnSettings(bool clear_persisted) OVERRIDE;
   virtual void OnSetting(
       SpdySettingsIds id, uint8 flags, uint32 value) OVERRIDE;
-  virtual void OnPing(uint32 unique_id) OVERRIDE;
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE;
   virtual void OnRstStream(SpdyStreamId stream_id,
                            SpdyRstStreamStatus status) OVERRIDE;
   virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,

diff --git a/net/spdy/buffered_spdy_framer_unittest.cc b/net/spdy/buffered_spdy_framer_unittest.cc
@@ -87,7 +87,7 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface {
     setting_count_++;
   }
 
-  virtual void OnPing(uint32 unique_id) OVERRIDE {}
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE {}
 
   virtual void OnRstStream(SpdyStreamId stream_id,
                            SpdyRstStreamStatus status) OVERRIDE {

diff --git a/net/spdy/mock_spdy_framer_visitor.h b/net/spdy/mock_spdy_framer_visitor.h
@@ -37,7 +37,7 @@ class MockSpdyFramerVisitor : public SpdyFramerVisitorInterface {
                                  SpdyRstStreamStatus status));
   MOCK_METHOD1(OnSettings, void(bool clear_persisted));
   MOCK_METHOD3(OnSetting, void(SpdySettingsIds id, uint8 flags, uint32 value));
-  MOCK_METHOD1(OnPing, void(uint32 unique_id));
+  MOCK_METHOD1(OnPing, void(SpdyPingId unique_id));
   MOCK_METHOD2(OnGoAway, void(SpdyStreamId last_accepted_stream_id,
                               SpdyGoAwayStatus status));
   MOCK_METHOD2(OnHeaders, void(SpdyStreamId stream_id, bool fin));

diff --git a/net/spdy/spdy_frame_builder.h b/net/spdy/spdy_frame_builder.h
@@ -92,6 +92,12 @@ class NET_EXPORT_PRIVATE SpdyFrameBuilder {
     value = htonl(value);
     return WriteBytes(&value, sizeof(value));
   }
+  bool WriteUInt64(uint64 value) {
+    uint32 upper = htonl(value >> 32);
+    uint32 lower = htonl(value);
+    return (WriteBytes(&upper, sizeof(upper)) &&
+            WriteBytes(&lower, sizeof(lower)));
+  }
   // TODO(hkhalil) Rename to WriteStringPiece16().
   bool WriteString(const std::string& value);
   bool WriteStringPiece32(const base::StringPiece& value);

diff --git a/net/spdy/spdy_frame_reader.cc b/net/spdy/spdy_frame_reader.cc
@@ -64,6 +64,24 @@ bool SpdyFrameReader::ReadUInt32(uint32* result) {
   return true;
 }
 
+bool SpdyFrameReader::ReadUInt64(uint64* result) {
+  // Make sure that we have the whole uint64.
+  if (!CanRead(8)) {
+    OnFailure();
+    return false;
+  }
+
+  // Read into result. Network byte order is big-endian.
+  uint64 upper = ntohl(*(reinterpret_cast<const uint32*>(data_ + ofs_)));
+  uint64 lower = ntohl(*(reinterpret_cast<const uint32*>(data_ + ofs_ + 4)));
+  *result = (upper << 32) + lower;
+
+  // Iterate.
+  ofs_ += 8;
+
+  return true;
+}
+
 bool SpdyFrameReader::ReadUInt31(uint32* result) {
   bool success = ReadUInt32(result);
 

diff --git a/net/spdy/spdy_frame_reader.h b/net/spdy/spdy_frame_reader.h
@@ -34,29 +34,34 @@ class NET_EXPORT_PRIVATE SpdyFrameReader {
   ~SpdyFrameReader() {}
 
   // Reads an 8-bit unsigned integer into the given output parameter.
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadUInt8(uint8* result);
 
   // Reads a 16-bit unsigned integer into the given output parameter.
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadUInt16(uint16* result);
 
   // Reads a 32-bit unsigned integer into the given output parameter.
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadUInt32(uint32* result);
 
+  // Reads a 64-bit unsigned integer into the given output parameter.
+  // Forwards the internal iterator on success.
+  // Returns true on success, false otherwise.
+  bool ReadUInt64(uint64* result);
+
   // Reads a 31-bit unsigned integer into the given output parameter. This is
   // equivalent to ReadUInt32() above except that the highest-order bit is
   // discarded.
-  // Forwards the internal iterater (by 4B) on success.
+  // Forwards the internal iterator (by 4B) on success.
   // Returns true on success, false otherwise.
   bool ReadUInt31(uint32* result);
 
   // Reads a 24-bit unsigned integer into the given output parameter.
-  // Forwards the internal iterater (by 3B) on success.
+  // Forwards the internal iterator (by 3B) on success.
   // Returns true on success, false otherwise.
   bool ReadUInt24(uint32* result);
 
@@ -65,7 +70,7 @@ class NET_EXPORT_PRIVATE SpdyFrameReader {
   // NOTE: Does not copy but rather references strings in the underlying buffer.
   // This should be kept in mind when handling memory management!
   //
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadStringPiece16(base::StringPiece* result);
 
@@ -74,13 +79,13 @@ class NET_EXPORT_PRIVATE SpdyFrameReader {
   // NOTE: Does not copy but rather references strings in the underlying buffer.
   // This should be kept in mind when handling memory management!
   //
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadStringPiece32(base::StringPiece* result);
 
   // Reads a given number of bytes into the given buffer. The buffer
   // must be of adequate size.
-  // Forwards the internal iterater on success.
+  // Forwards the internal iterator on success.
   // Returns true on success, false otherwise.
   bool ReadBytes(void* result, size_t size);
 

diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
@@ -232,9 +232,16 @@ size_t SpdyFramer::GetSettingsMinimumSize() const {
 }
 
 size_t SpdyFramer::GetPingSize() const {
-  // Size, in bytes, of this PING frame. Calculated as:
-  // control frame header + 4 (id)
-  return GetControlFrameHeaderSize() + 4;
+  // Size, in bytes, of this PING frame.
+  if (spdy_version_ < 4) {
+    // Calculated as:
+    // control frame header + 4 (id)
+    return GetControlFrameHeaderSize() + 4;
+  } else {
+    // Calculated as:
+    // control frame header + 8 (id)
+    return GetControlFrameHeaderSize() + 8;
+  }
 }
 
 size_t SpdyFramer::GetGoAwayMinimumSize() const {
@@ -1474,7 +1481,14 @@ size_t SpdyFramer::ProcessControlFramePayload(const char* data, size_t len) {
         break;
       case PING: {
           SpdyPingId id = 0;
-          bool successful_read = reader.ReadUInt32(&id);
+          bool successful_read = true;
+          if (spdy_version_ < 4) {
+            uint32 id32 = 0;
+            successful_read = reader.ReadUInt32(&id32);
+            id = id32;
+          } else {
+            successful_read = reader.ReadUInt64(&id);
+          }
           DCHECK(successful_read);
           DCHECK(reader.IsDoneReading());
           visitor_->OnPing(id);
@@ -1938,10 +1952,11 @@ SpdySerializedFrame* SpdyFramer::SerializePing(const SpdyPingIR& ping) const {
   SpdyFrameBuilder builder(GetPingSize());
   if (spdy_version_ < 4) {
     builder.WriteControlFrameHeader(*this, PING, kNoFlags);
+    builder.WriteUInt32(static_cast<uint32>(ping.id()));
   } else {
     builder.WriteFramePrefix(*this, PING, 0, 0);
+    builder.WriteUInt64(ping.id());
   }
-  builder.WriteUInt32(ping.id());
   DCHECK_EQ(GetPingSize(), builder.length());
   return builder.take();
 }

diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
@@ -185,7 +185,7 @@ class NET_EXPORT_PRIVATE SpdyFramerVisitorInterface {
   virtual void OnSetting(SpdySettingsIds id, uint8 flags, uint32 value) = 0;
 
   // Called when a PING frame has been parsed.
-  virtual void OnPing(uint32 unique_id) = 0;
+  virtual void OnPing(SpdyPingId unique_id) = 0;
 
   // Called when a GOAWAY frame has been parsed.
   virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,

diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
@@ -151,7 +151,7 @@ class SpdyFramerTestUtil {
                            uint32 value) OVERRIDE {
       LOG(FATAL);
     }
-    virtual void OnPing(uint32 unique_id) OVERRIDE {
+    virtual void OnPing(uint64 unique_id) OVERRIDE {
       LOG(FATAL);
     }
     virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,
@@ -343,7 +343,7 @@ class TestSpdyVisitor : public SpdyFramerVisitorInterface,
     setting_count_++;
   }
 
-  virtual void OnPing(uint32 unique_id) OVERRIDE {
+  virtual void OnPing(uint64 unique_id) OVERRIDE {
     DLOG(FATAL);
   }
 
@@ -2350,14 +2350,17 @@ TEST_P(SpdyFramerTest, CreatePingFrame) {
       0x12, 0x34, 0x56, 0x78,
     };
     const unsigned char kV4FrameData[] = {
-      0x00, 0x0c, 0x06, 0x00,
+      0x00, 0x10, 0x06, 0x00,
       0x00, 0x00, 0x00, 0x00,
       0x12, 0x34, 0x56, 0x78,
+      0x9a, 0xbc, 0xde, 0xff,
     };
-    scoped_ptr<SpdyFrame> frame(framer.SerializePing(SpdyPingIR(0x12345678u)));
+    scoped_ptr<SpdyFrame> frame;
     if (IsSpdy4()) {
+      frame.reset(framer.SerializePing(SpdyPingIR(0x123456789abcdeffull)));
       CompareFrame(kDescription, *frame, kV4FrameData, arraysize(kV4FrameData));
     } else {
+      frame.reset(framer.SerializePing(SpdyPingIR(0x12345678ull)));
       CompareFrame(kDescription, *frame, kV3FrameData, arraysize(kV3FrameData));
     }
   }
@@ -3428,7 +3431,7 @@ TEST_P(SpdyFramerTest, SizesTest) {
     EXPECT_EQ(8u, framer.GetSynReplyMinimumSize());
     EXPECT_EQ(12u, framer.GetRstStreamMinimumSize());
     EXPECT_EQ(12u, framer.GetSettingsMinimumSize());
-    EXPECT_EQ(12u, framer.GetPingSize());
+    EXPECT_EQ(16u, framer.GetPingSize());
     EXPECT_EQ(16u, framer.GetGoAwayMinimumSize());
     EXPECT_EQ(8u, framer.GetHeadersMinimumSize());
     EXPECT_EQ(12u, framer.GetWindowUpdateSize());

diff --git a/net/spdy/spdy_protocol.h b/net/spdy/spdy_protocol.h
@@ -365,7 +365,7 @@ typedef uint8 SpdyPriority;
 
 typedef std::map<std::string, std::string> SpdyNameValueBlock;
 
-typedef uint32 SpdyPingId;
+typedef uint64 SpdyPingId;
 
 class SpdyFrame;
 typedef SpdyFrame SpdySerializedFrame;

diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
@@ -205,7 +205,7 @@ base::Value* NetLogSpdyRstCallback(SpdyStreamId stream_id,
   return dict;
 }
 
-base::Value* NetLogSpdyPingCallback(uint32 unique_id,
+base::Value* NetLogSpdyPingCallback(SpdyPingId unique_id,
                                     const char* type,
                                     NetLog::LogLevel /* log_level */) {
   base::DictionaryValue* dict = new base::DictionaryValue();
@@ -2390,7 +2390,7 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
   MaybeFinishGoingAway();
 }
 
-void SpdySession::OnPing(uint32 unique_id) {
+void SpdySession::OnPing(SpdyPingId unique_id) {
   CHECK(in_io_loop_);
 
   if (availability_state_ == STATE_CLOSED)

diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
@@ -784,7 +784,7 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   virtual void OnError(SpdyFramer::SpdyError error_code) OVERRIDE;
   virtual void OnStreamError(SpdyStreamId stream_id,
                              const std::string& description) OVERRIDE;
-  virtual void OnPing(uint32 unique_id) OVERRIDE;
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE;
   virtual void OnRstStream(SpdyStreamId stream_id,
                            SpdyRstStreamStatus status) OVERRIDE;
   virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,

diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
@@ -237,7 +237,7 @@ class PriorityGetter : public BufferedSpdyFramerVisitorInterface {
   virtual void OnSettings(bool clear_persisted) OVERRIDE {}
   virtual void OnSetting(
       SpdySettingsIds id, uint8 flags, uint32 value) OVERRIDE {}
-  virtual void OnPing(uint32 unique_id) OVERRIDE {}
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE {}
   virtual void OnRstStream(SpdyStreamId stream_id,
                            SpdyRstStreamStatus status) OVERRIDE {}
   virtual void OnGoAway(SpdyStreamId last_accepted_stream_id,

diff --git a/net/tools/flip_server/spdy_interface.h b/net/tools/flip_server/spdy_interface.h
@@ -107,7 +107,7 @@ class SpdySM : public BufferedSpdyFramerVisitorInterface, public SMInterface {
                          uint32 value) OVERRIDE {}
 
   // Called when a PING frame has been parsed.
-  virtual void OnPing(uint32 unique_id) OVERRIDE {}
+  virtual void OnPing(SpdyPingId unique_id) OVERRIDE {}
 
   // Called when a RST_STREAM frame has been parsed.
   virtual void OnRstStream(SpdyStreamId stream_id,

diff --git a/net/tools/flip_server/spdy_interface_test.cc b/net/tools/flip_server/spdy_interface_test.cc
@@ -60,7 +60,7 @@ class SpdyFramerVisitor : public BufferedSpdyFramerVisitorInterface {
                                        bool));
   MOCK_METHOD1(OnSettings, void(bool clear_persisted));
   MOCK_METHOD3(OnSetting, void(SpdySettingsIds, uint8, uint32));
-  MOCK_METHOD1(OnPing, void(uint32 unique_id));
+  MOCK_METHOD1(OnPing, void(SpdyPingId unique_id));
   MOCK_METHOD2(OnRstStream, void(SpdyStreamId, SpdyRstStreamStatus));
   MOCK_METHOD2(OnGoAway, void(SpdyStreamId, SpdyGoAwayStatus));
   MOCK_METHOD2(OnWindowUpdate, void(SpdyStreamId, uint32));