Skip to content

Commit

Permalink
SPDY cleanup: remove credential slot.
Browse files Browse the repository at this point in the history
Credential slot field has been removed from SYN_STREAM frame.

This lands server change 59587408 by mlavan.

Also update affected Chromium portions of QUIC & SPDY.

Review URL: https://codereview.chromium.org/141953004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247849 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jgraettinger@chromium.org committed Jan 30, 2014
1 parent 0cc93b4 commit dcaebbf
Show file tree
Hide file tree
Showing 20 changed files with 79 additions and 165 deletions.
6 changes: 0 additions & 6 deletions net/quic/quic_headers_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class QuicHeadersStream::SpdyFramerVisitor
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) OVERRIDE {
if (!stream_->IsConnected()) {
Expand All @@ -42,11 +41,6 @@ class QuicHeadersStream::SpdyFramerVisitor
return;
}

if (credential_slot != 0) {
CloseConnection("credential_slot != 0");
return;
}

if (unidirectional != 0) {
CloseConnection("unidirectional != 0");
return;
Expand Down
5 changes: 2 additions & 3 deletions net/quic/quic_headers_stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,9 @@ class MockVisitor : public SpdyFramerVisitorInterface {
MOCK_METHOD3(OnControlFrameHeaderData, bool(SpdyStreamId stream_id,
const char* header_data,
size_t len));
MOCK_METHOD6(OnSynStream, void(SpdyStreamId stream_id,
MOCK_METHOD5(OnSynStream, void(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 slot,
bool fin,
bool unidirectional));
MOCK_METHOD2(OnSynReply, void(SpdyStreamId stream_id, bool fin));
Expand Down Expand Up @@ -124,7 +123,7 @@ class QuicHeadersStreamTest : public ::testing::TestWithParam<bool> {
if (type == SYN_STREAM) {
EXPECT_CALL(visitor_, OnSynStream(stream_id, kNoAssociatedStream, 0,
// priority,
_, fin, kNotUnidirectional));
fin, kNotUnidirectional));
} else {
EXPECT_CALL(visitor_, OnSynReply(stream_id, fin));
}
Expand Down
1 change: 0 additions & 1 deletion net/quic/quic_spdy_decompressor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class SpdyFramerVisitor : public SpdyFramerVisitorInterface {
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) OVERRIDE {}
virtual void OnSynReply(SpdyStreamId stream_id, bool fin) OVERRIDE {}
Expand Down
5 changes: 0 additions & 5 deletions net/spdy/buffered_spdy_framer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ void BufferedSpdyFramer::OnError(SpdyFramer* spdy_framer) {
void BufferedSpdyFramer::OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) {
frames_received_++;
Expand All @@ -72,7 +71,6 @@ void BufferedSpdyFramer::OnSynStream(SpdyStreamId stream_id,
control_frame_fields_->stream_id = stream_id;
control_frame_fields_->associated_stream_id = associated_stream_id;
control_frame_fields_->priority = priority;
control_frame_fields_->credential_slot = credential_slot;
control_frame_fields_->fin = fin;
control_frame_fields_->unidirectional = unidirectional;

Expand Down Expand Up @@ -128,7 +126,6 @@ bool BufferedSpdyFramer::OnControlFrameHeaderData(SpdyStreamId stream_id,
visitor_->OnSynStream(control_frame_fields_->stream_id,
control_frame_fields_->associated_stream_id,
control_frame_fields_->priority,
control_frame_fields_->credential_slot,
control_frame_fields_->fin,
control_frame_fields_->unidirectional,
headers);
Expand Down Expand Up @@ -246,13 +243,11 @@ SpdyFrame* BufferedSpdyFramer::CreateSynStream(
SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
SpdyControlFlags flags,
const SpdyHeaderBlock* headers) {
SpdySynStreamIR syn_stream(stream_id);
syn_stream.set_associated_to_stream_id(associated_stream_id);
syn_stream.set_priority(priority);
syn_stream.set_slot(credential_slot);
syn_stream.set_fin((flags & CONTROL_FLAG_FIN) != 0);
syn_stream.set_unidirectional((flags & CONTROL_FLAG_UNIDIRECTIONAL) != 0);
// TODO(hkhalil): Avoid copy here.
Expand Down
3 changes: 0 additions & 3 deletions net/spdy/buffered_spdy_framer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramerVisitorInterface {
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional,
const SpdyHeaderBlock& headers) = 0;
Expand Down Expand Up @@ -127,7 +126,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) OVERRIDE;
virtual void OnSynReply(SpdyStreamId stream_id, bool fin) OVERRIDE;
Expand Down Expand Up @@ -166,7 +164,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
SpdyFrame* CreateSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
SpdyControlFlags flags,
const SpdyHeaderBlock* headers);
SpdyFrame* CreateSynReply(SpdyStreamId stream_id,
Expand Down
2 changes: 0 additions & 2 deletions net/spdy/buffered_spdy_framer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class TestBufferedSpdyVisitor : public BufferedSpdyFramerVisitorInterface {
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional,
const SpdyHeaderBlock& headers) OVERRIDE {
Expand Down Expand Up @@ -221,7 +220,6 @@ TEST_P(BufferedSpdyFramerTest, ReadSynStreamHeaderBlock) {
framer.CreateSynStream(1, // stream_id
0, // associated_stream_id
1, // priority
0, // credential_slot
CONTROL_FLAG_NONE,
&headers));
EXPECT_TRUE(control_frame.get() != NULL);
Expand Down
3 changes: 1 addition & 2 deletions net/spdy/mock_spdy_framer_visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ class MockSpdyFramerVisitor : public SpdyFramerVisitorInterface {
MOCK_METHOD3(OnControlFrameHeaderData, bool(SpdyStreamId stream_id,
const char* header_data,
size_t len));
MOCK_METHOD6(OnSynStream, void(SpdyStreamId stream_id,
MOCK_METHOD5(OnSynStream, void(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 slot,
bool fin,
bool unidirectional));
MOCK_METHOD2(OnSynReply, void(SpdyStreamId stream_id, bool fin));
Expand Down
17 changes: 5 additions & 12 deletions net/spdy/spdy_framer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ size_t SpdyFramer::GetSynStreamMinimumSize() const {
// name-value block.
if (spdy_version_ < 4) {
// Calculated as:
// control frame header + 2 * 4 (stream IDs) + 1 (priority) + 1 (slot)
// control frame header + 2 * 4 (stream IDs) + 1 (priority)
// + 1 (unused, was credential slot)
return GetControlFrameHeaderSize() + 10;
} else {
// Calculated as:
Expand Down Expand Up @@ -1162,14 +1163,8 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
priority = priority >> 5;
}

uint8 slot = 0;
if (protocol_version() < 3) {
// SPDY 2 had an unused byte here. Seek past it.
reader.Seek(1);
} else {
successful_read = reader.ReadUInt8(&slot);
DCHECK(successful_read);
}
// Seek past unused byte; used to be credential slot in SPDY 3.
reader.Seek(1);

DCHECK(reader.IsDoneReading());
if (debug_visitor_) {
Expand All @@ -1182,7 +1177,6 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
current_frame_stream_id_,
associated_to_stream_id,
priority,
slot,
(current_frame_flags_ & CONTROL_FLAG_FIN) != 0,
(current_frame_flags_ & CONTROL_FLAG_UNIDIRECTIONAL) != 0);
}
Expand Down Expand Up @@ -1244,7 +1238,6 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
current_frame_stream_id_,
0, // associated_to_stream_id
priority,
0, // TODO(hkhalil): handle slot for SPDY 4+?
current_frame_flags_ & CONTROL_FLAG_FIN,
false); // unidirectional
} else {
Expand Down Expand Up @@ -1798,7 +1791,7 @@ SpdySerializedFrame* SpdyFramer::SerializeSynStream(
builder.WriteUInt32(syn_stream.stream_id());
builder.WriteUInt32(syn_stream.associated_to_stream_id());
builder.WriteUInt8(priority << ((spdy_version_ < 3) ? 6 : 5));
builder.WriteUInt8(syn_stream.slot());
builder.WriteUInt8(0); // Unused byte where credential slot used to be.
} else {
builder.WriteFramePrefix(*this,
HEADERS,
Expand Down
1 change: 0 additions & 1 deletion net/spdy/spdy_framer.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ class NET_EXPORT_PRIVATE SpdyFramerVisitorInterface {
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) = 0;

Expand Down
77 changes: 5 additions & 72 deletions net/spdy/spdy_framer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,13 @@ class SpdyFramerTestUtil {
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 slot,
bool fin,
bool unidirectional) OVERRIDE {
SpdyFramer framer(version_);
framer.set_enable_compression(false);
SpdySynStreamIR syn_stream(stream_id);
syn_stream.set_associated_to_stream_id(associated_stream_id);
syn_stream.set_priority(priority);
syn_stream.set_slot(slot);
syn_stream.set_fin(fin);
syn_stream.set_unidirectional(unidirectional);
scoped_ptr<SpdyFrame> frame(framer.SerializeSynStream(syn_stream));
Expand Down Expand Up @@ -309,7 +307,6 @@ class TestSpdyVisitor : public SpdyFramerVisitorInterface,
virtual void OnSynStream(SpdyStreamId stream_id,
SpdyStreamId associated_stream_id,
SpdyPriority priority,
uint8 credential_slot,
bool fin,
bool unidirectional) OVERRIDE {
syn_frame_count_++;
Expand Down Expand Up @@ -616,68 +613,6 @@ TEST_P(SpdyFramerTest, UndersizedHeaderBlockInBuffer) {
&new_headers));
}

TEST_P(SpdyFramerTest, OutOfOrderHeaders) {
SpdyFramer framer(spdy_version_);
framer.set_enable_compression(false);

// Frame builder with plentiful buffer size.
SpdyFrameBuilder frame(1024);
if (spdy_version_ < 4) {
frame.WriteControlFrameHeader(framer, SYN_STREAM, CONTROL_FLAG_NONE);
frame.WriteUInt32(3); // stream_id
} else {
frame.WriteFramePrefix(framer, SYN_STREAM, CONTROL_FLAG_NONE, 3);
}

frame.WriteUInt32(0); // Associated stream id
frame.WriteUInt16(0); // Priority.

if (IsSpdy2()) {
frame.WriteUInt16(2); // Number of headers.
frame.WriteString("gamma");
frame.WriteString("gamma");
frame.WriteString("alpha");
frame.WriteString("alpha");
} else {
frame.WriteUInt32(2); // Number of headers.
frame.WriteStringPiece32("gamma");
frame.WriteStringPiece32("gamma");
frame.WriteStringPiece32("alpha");
frame.WriteStringPiece32("alpha");
}
// write the length
frame.RewriteLength(framer);

SpdyHeaderBlock new_headers;
scoped_ptr<SpdyFrame> control_frame(frame.take());
base::StringPiece serialized_headers =
GetSerializedHeaders(control_frame.get(), framer);
EXPECT_TRUE(framer.ParseHeaderBlockInBuffer(serialized_headers.data(),
serialized_headers.size(),
&new_headers));
}

// Test that if we receive a SYN_STREAM with stream ID zero, we signal an error
// (but don't crash).
TEST_P(SpdyFramerTest, SynStreamWithStreamIdZero) {
testing::StrictMock<test::MockSpdyFramerVisitor> visitor;
SpdyFramer framer(spdy_version_);
framer.set_visitor(&visitor);

SpdySynStreamIR syn_stream(0);
syn_stream.set_priority(1);
syn_stream.SetHeader("alpha", "beta");
scoped_ptr<SpdyFrame> frame(framer.SerializeSynStream(syn_stream));
ASSERT_TRUE(frame.get() != NULL);

// We shouldn't have to read the whole frame before we signal an error.
EXPECT_CALL(visitor, OnError(testing::Eq(&framer)));
EXPECT_GT(frame->size(), framer.ProcessInput(frame->data(), frame->size()));
EXPECT_TRUE(framer.HasError());
EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME, framer.error_code())
<< SpdyFramer::ErrorCodeToString(framer.error_code());
}

// Test that if we receive a SYN_REPLY with stream ID zero, we signal an error
// (but don't crash).
TEST_P(SpdyFramerTest, SynReplyWithStreamIdZero) {
Expand Down Expand Up @@ -1687,10 +1622,9 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) {
framer.set_enable_compression(false);

{
const char kDescription[] = "SYN_STREAM frame, lowest pri, slot 2, no FIN";
const char kDescription[] = "SYN_STREAM frame, lowest pri, no FIN";

const unsigned char kPri = IsSpdy2() ? 0xC0 : 0xE0;
const unsigned char kCre = IsSpdy2() ? 0 : 2;
const unsigned char kV2FrameData[] = {
0x80, spdy_version_ch_, 0x00, 0x01,
0x00, 0x00, 0x00, 0x20,
Expand All @@ -1708,7 +1642,7 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) {
0x00, 0x00, 0x00, 0x2a,
0x00, 0x00, 0x00, 0x01,
0x00, 0x00, 0x00, 0x00,
kPri, kCre, 0x00, 0x00,
kPri, 0x00, 0x00, 0x00,
0x00, 0x02, 0x00, 0x00,
0x00, 0x03, 'b', 'a',
'r', 0x00, 0x00, 0x00,
Expand All @@ -1733,7 +1667,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) {
};
SpdySynStreamIR syn_stream(1);
syn_stream.set_priority(framer.GetLowestPriority());
syn_stream.set_slot(kCre);
syn_stream.SetHeader("bar", "foo");
syn_stream.SetHeader("foo", "bar");
scoped_ptr<SpdyFrame> frame(framer.SerializeSynStream(syn_stream));
Expand Down Expand Up @@ -3732,10 +3665,10 @@ TEST_P(SpdyFramerTest, SynStreamFrameFlags) {
} else {
EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(8, SYN_STREAM, _));
if (IsSpdy4()) {
EXPECT_CALL(visitor, OnSynStream(8, 0, 1, 0, flags & CONTROL_FLAG_FIN,
EXPECT_CALL(visitor, OnSynStream(8, 0, 1, flags & CONTROL_FLAG_FIN,
false));
} else {
EXPECT_CALL(visitor, OnSynStream(8, 3, 1, 0, flags & CONTROL_FLAG_FIN,
EXPECT_CALL(visitor, OnSynStream(8, 3, 1, flags & CONTROL_FLAG_FIN,
flags & CONTROL_FLAG_UNIDIRECTIONAL));
}
EXPECT_CALL(visitor, OnControlFrameHeaderData(8, _, _))
Expand Down Expand Up @@ -4087,7 +4020,7 @@ TEST_P(SpdyFramerTest, EmptySynStream) {
}

EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(1, SYN_STREAM, _));
EXPECT_CALL(visitor, OnSynStream(1, 0, 1, 0, false, false));
EXPECT_CALL(visitor, OnSynStream(1, 0, 1, false, false));
EXPECT_CALL(visitor, OnControlFrameHeaderData(1, NULL, 0));

framer.ProcessInput(frame->data(), framer.GetSynStreamMinimumSize());
Expand Down
6 changes: 0 additions & 6 deletions net/spdy/spdy_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,6 @@ enum SpdyGoAwayStatus {
// number between 0 and 3.
typedef uint8 SpdyPriority;

typedef uint8 SpdyCredentialSlot;

typedef std::map<std::string, std::string> SpdyNameValueBlock;

typedef uint32 SpdyPingId;
Expand Down Expand Up @@ -500,7 +498,6 @@ class NET_EXPORT_PRIVATE SpdySynStreamIR
: SpdyFrameWithNameValueBlockIR(stream_id),
associated_to_stream_id_(0),
priority_(0),
slot_(0),
unidirectional_(false) {}
SpdyStreamId associated_to_stream_id() const {
return associated_to_stream_id_;
Expand All @@ -510,8 +507,6 @@ class NET_EXPORT_PRIVATE SpdySynStreamIR
}
SpdyPriority priority() const { return priority_; }
void set_priority(SpdyPriority priority) { priority_ = priority; }
SpdyCredentialSlot slot() const { return slot_; }
void set_slot(SpdyCredentialSlot slot) { slot_ = slot; }
bool unidirectional() const { return unidirectional_; }
void set_unidirectional(bool unidirectional) {
unidirectional_ = unidirectional;
Expand All @@ -522,7 +517,6 @@ class NET_EXPORT_PRIVATE SpdySynStreamIR
private:
SpdyStreamId associated_to_stream_id_;
SpdyPriority priority_;
SpdyCredentialSlot slot_;
bool unidirectional_;

DISALLOW_COPY_AND_ASSIGN(SpdySynStreamIR);
Expand Down
Loading

0 comments on commit dcaebbf

Please sign in to comment.