Issue 10828129: SPDY: WriteHeaders should not invoke OnDataSent callback

Issue 10828129: SPDY: WriteHeaders should not invoke OnDataSent callback (Closed)

Created:
8 years, 4 months ago by Takashi Toyoshima

Modified:
8 years, 4 months ago

Reviewers:
Ryan Hamilton

CC:
chromium-reviews, cbentzel+watch_chromium.org, eroman, darin-cc_chromium.org, mmenke

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

SPDY: WriteHeaders should invoke OnDataSent callback Introduce a completion waiting list, and check which kind of frame is sent. OnDataSent() is invoked iff the frame wis data frame. BUG=42320 TEST=net_unittests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151647

Patch Set 1 : baseline 10810069 #

Patch Set 2 : done #

Total comments: 1

Patch Set 3 : rebase #

Patch Set 4 : add OnHeadersSent #

Patch Set 5 : add check for OnHeadersSent in tests #

Total comments: 4

Patch Set 6 : add proper comments #

Patch Set 7 : rebase #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+77 lines, -30 lines)			Patch
M	net/spdy/spdy_http_stream.h	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	net/spdy/spdy_http_stream.cc	View	1 2 3 4 5 6	1 chunk	+5 lines, -0 lines	0 comments	Download
M	net/spdy/spdy_proxy_client_socket.h	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	net/spdy/spdy_proxy_client_socket.cc	View	1 2 3 4 5 6	1 chunk	+5 lines, -0 lines	0 comments	Download
M	net/spdy/spdy_session_spdy2_unittest.cc	View	1 2 3 4 5 6	2 chunks	+10 lines, -8 lines	0 comments	Download
M	net/spdy/spdy_session_spdy3_unittest.cc	View	1 2 3 4 5 6	2 chunks	+10 lines, -9 lines	0 comments	Download
M	net/spdy/spdy_stream.h	View	1 2 3 4 5 6	3 chunks	+13 lines, -3 lines	0 comments	Download
M	net/spdy/spdy_stream.cc	View	1 2 3 4 5 6	3 chunks	+14 lines, -4 lines	0 comments	Download
M	net/spdy/spdy_stream_spdy2_unittest.cc	View	1 2 3 4	1 chunk	+2 lines, -3 lines	0 comments	Download
M	net/spdy/spdy_stream_spdy3_unittest.cc	View	1 2 3 4	1 chunk	+2 lines, -3 lines	0 comments	Download
M	net/spdy/spdy_stream_test_util.h	View	1 2 3 4 5 6	3 chunks	+3 lines, -0 lines	0 comments	Download
M	net/spdy/spdy_stream_test_util.cc	View	1 2 3 4 5 6	2 chunks	+5 lines, -0 lines	0 comments	Download
M	net/spdy/spdy_websocket_stream.h	View	1 2 3 4 5 6	1 chunk	+1 line, -0 lines	0 comments	Download
M	net/spdy/spdy_websocket_stream.cc	View	1 2 3 4 5 6	1 chunk	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Ryan Hamilton

http://codereview.chromium.org/10828129/diff/4001/net/spdy/spdy_stream.cc File net/spdy/spdy_stream.cc (right): http://codereview.chromium.org/10828129/diff/4001/net/spdy/spdy_stream.cc#newcode829 net/spdy/spdy_stream.cc:829: DCHECK(type == TYPE_HEADER); I believe we should notify the ...

8 years, 4 months ago (2012-08-02 16:33:53 UTC) #1

Takashi Toyoshima

Hi Ryan, I revised my change to add OnHeadersSent delegate.

8 years, 4 months ago (2012-08-06 08:49:33 UTC) #2

Ryan Hamilton

http://codereview.chromium.org/10828129/diff/3031/net/spdy/spdy_stream.cc File net/spdy/spdy_stream.cc (right): http://codereview.chromium.org/10828129/diff/3031/net/spdy/spdy_stream.cc#newcode151 net/spdy/spdy_stream.cc:151: DCHECK(frame.type == TYPE_HEADERS); It doesn't look like the HEADERS ...

8 years, 4 months ago (2012-08-13 17:10:01 UTC) #4

Takashi Toyoshima

https://chromiumcodereview.appspot.com/10828129/diff/3031/net/spdy/spdy_stream.cc File net/spdy/spdy_stream.cc (right): https://chromiumcodereview.appspot.com/10828129/diff/3031/net/spdy/spdy_stream.cc#newcode151 net/spdy/spdy_stream.cc:151: DCHECK(frame.type == TYPE_HEADERS); I added frame.type at line 145 ...

8 years, 4 months ago (2012-08-14 09:14:02 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/toyoshim@chromium.org/10828129/16002

8 years, 4 months ago (2012-08-15 01:15:29 UTC) #7

Change committed as 151647

Expand Messages | Collapse Messages