Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(55)

Issue 10815074: Instead of enqueueing SPDY frames, enqueue SPDY streams that are ready to produce data. (Closed)

Created:
8 years, 5 months ago by Ryan Hamilton
Modified:
8 years, 5 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Instead of enqueueing SPDY frames, enqueue SPDY streams that are ready to produce data. This allows us to lazily allocate a stream id. The second CL was reverted because of use-after-free problems. Producers were deleted before they were pop()'d from the WriteQueue, which turns out to be a no-no. This version fixes this defect. The initial CL was reverted because of memory leaks. Both SpdyIOBufferProducers leaked the SpdyFrame they owned. The second version of the CL fixes this defect. Attempting to re-land 144649 Revert 147692 - Revert 144655 - Revert 144649 BUG=111708 TEST=net_unittests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148660

Patch Set 1 #

Patch Set 2 : Remove logging and cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+461 lines, -209 lines) Patch
M net/http/http_network_transaction_spdy2_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M net/http/http_network_transaction_spdy3_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M net/spdy/spdy_io_buffer.h View 2 chunks +9 lines, -1 line 0 comments Download
M net/spdy/spdy_io_buffer.cc View 1 chunk +15 lines, -0 lines 0 comments Download
M net/spdy/spdy_network_transaction_spdy2_unittest.cc View 2 chunks +28 lines, -13 lines 0 comments Download
M net/spdy/spdy_network_transaction_spdy3_unittest.cc View 2 chunks +28 lines, -13 lines 0 comments Download
M net/spdy/spdy_session.h View 10 chunks +73 lines, -15 lines 0 comments Download
M net/spdy/spdy_session.cc View 1 24 chunks +153 lines, -98 lines 0 comments Download
M net/spdy/spdy_session_spdy2_unittest.cc View 7 chunks +11 lines, -11 lines 0 comments Download
M net/spdy/spdy_session_spdy3_unittest.cc View 1 9 chunks +20 lines, -17 lines 0 comments Download
M net/spdy/spdy_stream.h View 6 chunks +14 lines, -2 lines 0 comments Download
M net/spdy/spdy_stream.cc View 6 chunks +85 lines, -24 lines 0 comments Download
M net/spdy/spdy_stream_spdy2_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M net/spdy/spdy_stream_spdy3_unittest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M net/spdy/spdy_websocket_stream.h View 1 chunk +2 lines, -0 lines 0 comments Download
M net/spdy/spdy_websocket_stream_spdy2_unittest.cc View 4 chunks +6 lines, -2 lines 0 comments Download
M net/spdy/spdy_websocket_stream_spdy3_unittest.cc View 5 chunks +7 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Ryan Hamilton
willchan: PTAL. Patch set 1 is the original CL. You can do a diff of ...
8 years, 5 months ago (2012-07-26 19:54:15 UTC) #1
willchan no longer on Chromium
lgtm
8 years, 5 months ago (2012-07-26 20:37:16 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/10815074/20001
8 years, 5 months ago (2012-07-26 20:39:23 UTC) #3
commit-bot: I haz the power
8 years, 5 months ago (2012-07-26 23:53:22 UTC) #4
Change committed as 148660

Powered by Google App Engine
This is Rietveld 408576698