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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10870021: Make MessageReader class not ref-counted. (Closed)

Created:
8 years, 4 months ago by Sergey Ulanov
Modified:
8 years, 4 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, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Make MessageReader class not ref-counted. Previously MessageReader class was ref-counted, which means in some cases it could outlive the socket. Specifically the problem is when received messages are processed asynchronously and the callback for one of the messages is called after the socket is destroyed. BUG=139257 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=153079

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -69 lines) Patch
M remoting/protocol/message_reader.h View 1 2 3 6 chunks +15 lines, -13 lines 0 comments Download
M remoting/protocol/message_reader.cc View 1 2 3 6 chunks +15 lines, -17 lines 0 comments Download
M remoting/protocol/message_reader_unittest.cc View 1 4 chunks +3 lines, -39 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Sergey Ulanov
8 years, 4 months ago (2012-08-22 20:51:40 UTC) #1
Wez
http://codereview.chromium.org/10870021/diff/3001/remoting/protocol/message_reader.h File remoting/protocol/message_reader.h (right): http://codereview.chromium.org/10870021/diff/3001/remoting/protocol/message_reader.h#newcode36 remoting/protocol/message_reader.h:36: class MessageReader : public base::SupportsWeakPtr<MessageReader>, Use a WeakPtrFactory member ...
8 years, 4 months ago (2012-08-22 22:36:15 UTC) #2
Sergey Ulanov
http://codereview.chromium.org/10870021/diff/3001/remoting/protocol/message_reader.h File remoting/protocol/message_reader.h (right): http://codereview.chromium.org/10870021/diff/3001/remoting/protocol/message_reader.h#newcode36 remoting/protocol/message_reader.h:36: class MessageReader : public base::SupportsWeakPtr<MessageReader>, On 2012/08/22 22:36:15, Wez ...
8 years, 4 months ago (2012-08-22 22:41:41 UTC) #3
Wez
LGTM w/ nits. http://codereview.chromium.org/10870021/diff/7001/remoting/protocol/message_reader.cc File remoting/protocol/message_reader.cc (right): http://codereview.chromium.org/10870021/diff/7001/remoting/protocol/message_reader.cc#newcode27 remoting/protocol/message_reader.cc:27: weak_ptr_factory_(this) { ALLOW_THIS_IN_INITIALIZER() here? http://codereview.chromium.org/10870021/diff/7001/remoting/protocol/message_reader.h File ...
8 years, 4 months ago (2012-08-23 01:18:57 UTC) #4
Sergey Ulanov
http://codereview.chromium.org/10870021/diff/7001/remoting/protocol/message_reader.cc File remoting/protocol/message_reader.cc (right): http://codereview.chromium.org/10870021/diff/7001/remoting/protocol/message_reader.cc#newcode27 remoting/protocol/message_reader.cc:27: weak_ptr_factory_(this) { On 2012/08/23 01:18:58, Wez wrote: > ALLOW_THIS_IN_INITIALIZER() ...
8 years, 4 months ago (2012-08-23 17:06:17 UTC) #5
commit-bot: I haz the power
8 years, 4 months ago (2012-08-23 17:06:48 UTC) #6

Powered by Google App Engine
This is Rietveld 408576698