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

Issue 10981009: Fix ChannelMultiplexer to properly handle base channel creation failure. (Closed)

Created:
8 years, 3 months ago by Sergey Ulanov
Modified:
8 years, 2 months ago
Reviewers:
Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Fix ChannelMultiplexer to properly handle base channel creation failure. ChannelMultiplexer may be destroyed from channel connection callback when it fails to connect the base channel. Previously it didn't handle this case properly. BUG=152039 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158740

Patch Set 1 #

Total comments: 12

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+171 lines, -60 lines) Patch
M remoting/protocol/channel_multiplexer.h View 1 3 chunks +11 lines, -3 lines 0 comments Download
M remoting/protocol/channel_multiplexer.cc View 1 2 5 chunks +40 lines, -30 lines 0 comments Download
M remoting/protocol/channel_multiplexer_unittest.cc View 10 chunks +53 lines, -13 lines 0 comments Download
M remoting/protocol/connection_to_client_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M remoting/protocol/fake_session.h View 1 3 chunks +15 lines, -3 lines 0 comments Download
M remoting/protocol/fake_session.cc View 1 2 chunks +52 lines, -10 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Sergey Ulanov
8 years, 3 months ago (2012-09-24 23:10:47 UTC) #1
Wez
http://codereview.chromium.org/10981009/diff/1/remoting/protocol/channel_multiplexer.cc File remoting/protocol/channel_multiplexer.cc (right): http://codereview.chromium.org/10981009/diff/1/remoting/protocol/channel_multiplexer.cc#newcode441 remoting/protocol/channel_multiplexer.cc:441: if (pending_channels_.size() == 0) nit: !pending_channels_.empty() http://codereview.chromium.org/10981009/diff/1/remoting/protocol/channel_multiplexer.cc#newcode448 remoting/protocol/channel_multiplexer.cc:448: if ...
8 years, 2 months ago (2012-09-25 17:43:58 UTC) #2
Sergey Ulanov
http://codereview.chromium.org/10981009/diff/1/remoting/protocol/channel_multiplexer.cc File remoting/protocol/channel_multiplexer.cc (right): http://codereview.chromium.org/10981009/diff/1/remoting/protocol/channel_multiplexer.cc#newcode441 remoting/protocol/channel_multiplexer.cc:441: if (pending_channels_.size() == 0) On 2012/09/25 17:43:58, Wez wrote: ...
8 years, 2 months ago (2012-09-25 18:59:07 UTC) #3
Wez
lgtm http://codereview.chromium.org/10981009/diff/4002/remoting/protocol/channel_multiplexer.cc File remoting/protocol/channel_multiplexer.cc (right): http://codereview.chromium.org/10981009/diff/4002/remoting/protocol/channel_multiplexer.cc#newcode444 remoting/protocol/channel_multiplexer.cc:444: // Every time this function is called it ...
8 years, 2 months ago (2012-09-25 23:34:15 UTC) #4
Sergey Ulanov
http://codereview.chromium.org/10981009/diff/4002/remoting/protocol/channel_multiplexer.cc File remoting/protocol/channel_multiplexer.cc (right): http://codereview.chromium.org/10981009/diff/4002/remoting/protocol/channel_multiplexer.cc#newcode444 remoting/protocol/channel_multiplexer.cc:444: // Every time this function is called it connects ...
8 years, 2 months ago (2012-09-26 00:46:45 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10981009/9001
8 years, 2 months ago (2012-09-26 00:47:04 UTC) #6
commit-bot: I haz the power
8 years, 2 months ago (2012-09-26 03:46:13 UTC) #7
Change committed as 158740

Powered by Google App Engine
This is Rietveld 408576698