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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 7218061: Close all writers before JingleSession is destroyed. (Closed)

Created:
9 years, 5 months ago by Sergey Ulanov
Modified:
9 years, 5 months ago
Reviewers:
Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, Paweł Hajdan Jr., dmaclach+watch_chromium.org, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, ajwong+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Close all writers before JingleSession is destroyed. BUG=None TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91225

Patch Set 1 : - #

Total comments: 8

Patch Set 2 : address comments #

Total comments: 2

Patch Set 3 : remove TODO #

Patch Set 4 : fix tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -58 lines) Patch
M remoting/client/input_handler.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/client/plugin/chromoting_instance.cc View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/buffered_socket_writer.h View 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/buffered_socket_writer.cc View 1 4 chunks +12 lines, -12 lines 0 comments Download
M remoting/protocol/client_control_sender.h View 2 chunks +7 lines, -1 line 0 comments Download
M remoting/protocol/client_control_sender.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/protocol/connection_to_client.h View 1 3 chunks +4 lines, -3 lines 0 comments Download
M remoting/protocol/connection_to_client.cc View 1 2 3 5 chunks +20 lines, -21 lines 0 comments Download
M remoting/protocol/connection_to_client_unittest.cc View 2 chunks +17 lines, -0 lines 0 comments Download
M remoting/protocol/connection_to_host.h View 1 4 chunks +10 lines, -4 lines 0 comments Download
M remoting/protocol/connection_to_host.cc View 1 2 3 5 chunks +17 lines, -4 lines 0 comments Download
M remoting/protocol/host_control_sender.h View 2 chunks +7 lines, -1 line 0 comments Download
M remoting/protocol/host_control_sender.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/protocol/input_sender.h View 2 chunks +7 lines, -0 lines 0 comments Download
M remoting/protocol/input_sender.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/protocol/protobuf_video_writer.h View 2 chunks +6 lines, -3 lines 0 comments Download
M remoting/protocol/protobuf_video_writer.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/protocol/rtcp_writer.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/rtcp_writer.cc View 1 chunk +6 lines, -1 line 0 comments Download
M remoting/protocol/rtp_video_reader_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/rtp_video_writer.h View 1 chunk +5 lines, -3 lines 0 comments Download
M remoting/protocol/rtp_video_writer.cc View 1 chunk +7 lines, -1 line 0 comments Download
M remoting/protocol/rtp_video_writer_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/rtp_writer.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/rtp_writer.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/protocol/video_writer.h View 1 2 2 chunks +4 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Sergey Ulanov
9 years, 5 months ago (2011-06-30 00:23:22 UTC) #1
Wez
http://codereview.chromium.org/7218061/diff/1001/remoting/protocol/buffered_socket_writer.cc File remoting/protocol/buffered_socket_writer.cc (right): http://codereview.chromium.org/7218061/diff/1001/remoting/protocol/buffered_socket_writer.cc#newcode63 remoting/protocol/buffered_socket_writer.cc:63: } nit: Does this belong in this CL? http://codereview.chromium.org/7218061/diff/1001/remoting/protocol/buffered_socket_writer.cc#newcode133 ...
9 years, 5 months ago (2011-06-30 18:27:31 UTC) #2
Sergey Ulanov
http://codereview.chromium.org/7218061/diff/1001/remoting/protocol/buffered_socket_writer.cc File remoting/protocol/buffered_socket_writer.cc (right): http://codereview.chromium.org/7218061/diff/1001/remoting/protocol/buffered_socket_writer.cc#newcode63 remoting/protocol/buffered_socket_writer.cc:63: } On 2011/06/30 18:27:31, Wez wrote: > nit: Does ...
9 years, 5 months ago (2011-06-30 20:55:24 UTC) #3
Wez
LGTM http://codereview.chromium.org/7218061/diff/6001/remoting/protocol/video_writer.h File remoting/protocol/video_writer.h (right): http://codereview.chromium.org/7218061/diff/6001/remoting/protocol/video_writer.h#newcode22 remoting/protocol/video_writer.h:22: // TODO(sergeyu): VideoWriter should implement VideoStub interface. nit: ...
9 years, 5 months ago (2011-06-30 21:01:29 UTC) #4
Sergey Ulanov
9 years, 5 months ago (2011-06-30 21:17:22 UTC) #5 
http://codereview.chromium.org/7218061/diff/6001/remoting/protocol/video_writ...
File remoting/protocol/video_writer.h (right):

http://codereview.chromium.org/7218061/diff/6001/remoting/protocol/video_writ...
remoting/protocol/video_writer.h:22: // TODO(sergeyu): VideoWriter should
implement VideoStub interface.
On 2011/06/30 21:01:29, Wez wrote:
> nit: Comment is out of date?

Done.

Powered by Google App Engine
This is Rietveld 408576698