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

Issue 265933007: SPDY: Defer SpdyBufferProducer deletions in SpdyWriteQueue (Closed)

Created:
6 years, 7 months ago by Johnny
Modified:
6 years, 7 months ago
CC:
cbentzel+watch_chromium.org
Visibility:
Public.

Description

These can post callbacks which re-enter into SpdyWriteQueue. BUG=369539 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268730

Patch Set 1 #

Patch Set 2 : Defer deletion using std::vector. Don't post. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -3 lines) Patch
M net/spdy/spdy_write_queue.cc View 1 5 chunks +16 lines, -3 lines 2 comments Download

Messages

Total messages: 10 (0 generated)
Johnny
6 years, 7 months ago (2014-05-02 18:41:03 UTC) #1
willchan no longer on Chromium
I have a counter-proposal. How about we delay destroying the SpdyBufferProducer within the loop iteration? ...
6 years, 7 months ago (2014-05-06 00:12:13 UTC) #2
Johnny
Hmm. I have no strong objections to using a vector, it's a perfectly correct solution, ...
6 years, 7 months ago (2014-05-06 15:17:59 UTC) #3
Johnny
Switched to vector, PTAL
6 years, 7 months ago (2014-05-06 16:30:28 UTC) #4
willchan no longer on Chromium
lgtm https://codereview.chromium.org/265933007/diff/20001/net/spdy/spdy_write_queue.cc File net/spdy/spdy_write_queue.cc (right): https://codereview.chromium.org/265933007/diff/20001/net/spdy/spdy_write_queue.cc#newcode100 net/spdy/spdy_write_queue.cc:100: std::vector<SpdyBufferProducer*> erased_buffer_producers; You can also use ScopedVector (https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped_vector.h&q=ScopedVector&sq=package:chromium&l=15) ...
6 years, 7 months ago (2014-05-06 16:38:13 UTC) #5
Johnny
https://codereview.chromium.org/265933007/diff/20001/net/spdy/spdy_write_queue.cc File net/spdy/spdy_write_queue.cc (right): https://codereview.chromium.org/265933007/diff/20001/net/spdy/spdy_write_queue.cc#newcode100 net/spdy/spdy_write_queue.cc:100: std::vector<SpdyBufferProducer*> erased_buffer_producers; On 2014/05/06 16:38:14, willchan wrote: > You ...
6 years, 7 months ago (2014-05-06 16:54:27 UTC) #6
Johnny
The CQ bit was checked by jgraettinger@chromium.org
6 years, 7 months ago (2014-05-06 16:54:32 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jgraettinger@chromium.org/265933007/20001
6 years, 7 months ago (2014-05-06 16:54:47 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-07 07:32:00 UTC) #9
commit-bot: I haz the power
6 years, 7 months ago (2014-05-07 13:42:36 UTC) #10
Message was sent while issue was closed.
Change committed as 268730

Powered by Google App Engine
This is Rietveld 408576698