remove the SentFin method from the stream

When a FIN is dequeued from the stream by the streamFramer, it is
guaranteed to be sent out. There's no need to explicitely signal that to
the stream.

internal/mocks/stream.go

@@ -190,16 +190,6 @@ func (_mr *MockStreamIMockRecorder) Reset(arg0 interface{}) *gomock.Call {
 	return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "Reset", reflect.TypeOf((*MockStreamI)(nil).Reset), arg0)
 }
 
-// SentFin mocks base method
-func (_m *MockStreamI) SentFin() {
-	_m.ctrl.Call(_m, "SentFin")
-}
-
-// SentFin indicates an expected call of SentFin
-func (_mr *MockStreamIMockRecorder) SentFin() *gomock.Call {
-	return _mr.mock.ctrl.RecordCallWithMethodType(_mr.mock, "SentFin", reflect.TypeOf((*MockStreamI)(nil).SentFin))
-}
-
 // SetDeadline mocks base method
 func (_m *MockStreamI) SetDeadline(_param0 time.Time) error {
 	ret := _m.ctrl.Call(_m, "SetDeadline", _param0)

session_test.go

@@ -1434,9 +1434,9 @@ var _ = Describe("Session", func() {
 			for i := 2; i <= 1000; i++ {
 				s, err := sess.GetOrOpenStream(protocol.StreamID(i*2 + 1))
 				Expect(err).NotTo(HaveOccurred())
-				err = s.Close()
-				Expect(err).NotTo(HaveOccurred())
-				s.(*stream).SentFin()
+				Expect(s.Close()).To(Succeed())
+				_, sentFin := s.(*stream).GetDataForWriting(1000) // trigger "sending" of the FIN bit
+				Expect(sentFin).To(BeTrue())
 				s.(*stream).CloseRemote(0)
 				_, err = s.Read([]byte("a"))
 				Expect(err).To(MatchError(io.EOF))

stream.go

@@ -24,7 +24,6 @@ type streamI interface {
 	GetWriteOffset() protocol.ByteCount
 	Finished() bool
 	Cancel(error)
-	SentFin()
 	// methods needed for flow control
 	GetWindowUpdate() protocol.ByteCount
 	UpdateSendWindow(protocol.ByteCount)
@@ -273,6 +272,14 @@ func (s *stream) HasDataForWriting() bool {
 }
 
 func (s *stream) GetDataForWriting(maxBytes protocol.ByteCount) ([]byte, bool /* should send FIN */) {
+	data, shouldSendFin := s.getDataForWritingImpl(maxBytes)
+	if shouldSendFin {
+		s.finSent.Set(true)
+	}
+	return data, shouldSendFin
+}
+
+func (s *stream) getDataForWritingImpl(maxBytes protocol.ByteCount) ([]byte, bool /* should send FIN */) {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
 
@@ -317,10 +324,6 @@ func (s *stream) shouldSendReset() bool {
 	return (s.resetLocally.Get() || s.resetRemotely.Get()) && !s.finishedWriteAndSentFin()
 }
 
-func (s *stream) SentFin() {
-	s.finSent.Set(true)
-}
-
 // AddStreamFrame adds a new stream frame
 func (s *stream) AddStreamFrame(frame *wire.StreamFrame) error {
 	maxOffset := frame.Offset + frame.DataLen()

stream_framer.go

@@ -121,9 +121,6 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res []
 		if len(frame.Data) == 0 && !frame.FinBit {
 			return true, nil
 		}
-		if frame.FinBit {
-			s.SentFin()
-		}
 
 		// Finally, check if we are now FC blocked and should queue a BLOCKED frame
 		if !frame.FinBit && s.IsFlowControlBlocked() {

stream_framer_test.go

@@ -273,7 +273,6 @@ var _ = Describe("Stream Framer", func() {
 				stream1.EXPECT().HasDataForWriting().Return(true)
 				stream1.EXPECT().GetDataForWriting(gomock.Any()).Return(nil, true)
 				stream1.EXPECT().GetWriteOffset().Return(offset)
-				stream1.EXPECT().SentFin()
 				setNoData(stream2)
 
 				fs := framer.PopStreamFrames(1000)
@@ -289,7 +288,6 @@ var _ = Describe("Stream Framer", func() {
 				stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foobar"), true)
 				stream1.EXPECT().HasDataForWriting().Return(true)
 				stream1.EXPECT().GetWriteOffset().Return(offset)
-				stream1.EXPECT().SentFin()
 				setNoData(stream2)
 
 				fs := framer.PopStreamFrames(1000)
@@ -327,7 +325,6 @@ var _ = Describe("Stream Framer", func() {
 			stream1.EXPECT().GetDataForWriting(gomock.Any()).Return([]byte("foo"), true)
 			stream1.EXPECT().HasDataForWriting().Return(true)
 			stream1.EXPECT().GetWriteOffset()
-			stream1.EXPECT().SentFin()
 			setNoData(stream2)
 			frames := framer.PopStreamFrames(1000)
 			Expect(frames).To(HaveLen(1))

stream_test.go

@@ -641,7 +641,8 @@ var _ = Describe("Stream", func() {
 			It("doesn't call onReset if it already sent a FIN", func() {
 				mockFC.EXPECT().UpdateHighestReceived(protocol.ByteCount(0), true)
 				str.Close()
-				str.SentFin()
+				_, sentFin := str.GetDataForWriting(1000)
+				Expect(sentFin).To(BeTrue())
 				str.RegisterRemoteError(testErr, 0)
 				Expect(resetCalled).To(BeFalse())
 			})
@@ -726,7 +727,8 @@ var _ = Describe("Stream", func() {
 
 			It("doesn't call onReset if it already sent a FIN", func() {
 				str.Close()
-				str.SentFin()
+				_, sentFin := str.GetDataForWriting(1000)
+				Expect(sentFin).To(BeTrue())
 				str.Reset(testErr)
 				Expect(resetCalled).To(BeFalse())
 			})
@@ -957,7 +959,6 @@ var _ = Describe("Stream", func() {
 				data, sendFin := str.GetDataForWriting(1000)
 				Expect(data).To(BeEmpty())
 				Expect(sendFin).To(BeTrue())
-				str.SentFin()
 				Expect(str.HasDataForWriting()).To(BeFalse())
 				data, sendFin = str.GetDataForWriting(1000)
 				Expect(data).To(BeEmpty())
@@ -1022,14 +1023,14 @@ var _ = Describe("Stream", func() {
 
 		It("is not finished if it is only closed for writing", func() {
 			str.Close()
-			str.SentFin()
+			_, sentFin := str.GetDataForWriting(1000)
+			Expect(sentFin).To(BeTrue())
 			Expect(str.Finished()).To(BeFalse())
 		})
 
 		It("cancels the context after it is closed", func() {
 			Expect(str.Context().Done()).ToNot(BeClosed())
 			str.Close()
-			str.SentFin()
 			Expect(str.Context().Done()).To(BeClosed())
 		})
 
@@ -1065,7 +1066,8 @@ var _ = Describe("Stream", func() {
 		It("is finished after finishing writing and receiving a RST", func() {
 			mockFC.EXPECT().UpdateHighestReceived(protocol.ByteCount(13), true)
 			str.Close()
-			str.SentFin()
+			_, sentFin := str.GetDataForWriting(1000)
+			Expect(sentFin).To(BeTrue())
 			str.RegisterRemoteError(testErr, 13)
 			Expect(str.Finished()).To(BeTrue())
 		})