Issue 7981026: Cleanup ConnectionToHost of non-P2P-API code.

Issue 7981026: Cleanup ConnectionToHost of non-P2P-API code. (Closed)

Created:
9 years, 3 months ago by Sergey Ulanov

Modified:
9 years, 3 months ago

Reviewers:
Wez, Jamie

CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, dmaclach+watch_chromium.org, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, ajwong+watch_chromium.org, sergeyu+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Cleanup ConnectionToHost of non-P2P-API code. BUG=74951 TEST=Compiles Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102311

Patch Set 1 #

Patch Set 2 : - #

Patch Set 3 : - #

Patch Set 4 : - #

Total comments: 6

Created: 9 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+6 lines, -169 lines)			Patch
M	remoting/client/DEPS	View		1 chunk	+0 lines, -4 lines	0 comments	Download
D	remoting/client/ipc_host_resolver.h	View	1 2	1 chunk	+0 lines, -37 lines	0 comments	Download
D	remoting/client/ipc_host_resolver.cc	View	1 2	1 chunk	+0 lines, -70 lines	0 comments	Download
M	remoting/client/plugin/chromoting_instance.h	View	1 2 3	1 chunk	+4 lines, -4 lines	0 comments	Download
M	remoting/client/plugin/chromoting_instance.cc	View	1 2 3	4 chunks	+1 line, -15 lines	0 comments	Download
M	remoting/protocol/connection_to_host.h	View	1	3 chunks	+0 lines, -15 lines	4 comments	Download
M	remoting/protocol/connection_to_host.cc	View		2 chunks	+1 line, -22 lines	2 comments	Download
M	remoting/remoting.gyp	View	1 2	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Jamie

Wez has offered to look at this for me, and I think he's better qualified.

9 years, 3 months ago (2011-09-21 19:25:30 UTC) #2

Wez

LGTM, but... Please update the CL description to summarize the changes it makes; it not ...

9 years, 3 months ago (2011-09-22 13:49:45 UTC) #3

Sergey Ulanov

9 years, 3 months ago (2011-09-22 18:12:00 UTC) #4

On 2011/09/22 13:49:45, Wez wrote:
> LGTM, but...
> 
> Please update the CL description to summarize the changes it makes; it not
only
> resolves the dependency, but removes the old IPC resolver and removes the last
> vestiges of the non-P2P-API implementation from ConnectionToHost.

Done

> 
> Can we get rid of the IPC SocketFactory as well?
No, we still need it. It is used in the browser in P2PTransportImpl.

http://codereview.chromium.org/7981026/diff/1006/remoting/protocol/connection...
File remoting/protocol/connection_to_host.cc (right):

http://codereview.chromium.org/7981026/diff/1006/remoting/protocol/connection...
remoting/protocol/connection_to_host.cc:106: // Initialize |session_manager_|.
On 2011/09/22 13:49:45, Wez wrote:
> nit: This code is now so beautifully simple that this comment adds nothing. ;)

Done.

http://codereview.chromium.org/7981026/diff/1006/remoting/protocol/connection...
File remoting/protocol/connection_to_host.h (right):

http://codereview.chromium.org/7981026/diff/1006/remoting/protocol/connection...
remoting/protocol/connection_to_host.h:73: // |network_manager| and
|socket_factory| may be set to NULL.
On 2011/09/22 13:49:45, Wez wrote:
> Remove this comment.

Done.

http://codereview.chromium.org/7981026/diff/1006/remoting/protocol/connection...
remoting/protocol/connection_to_host.h:75: // TODO(sergeyu): Constructor
shouldn't need thread here.
On 2011/09/22 13:49:45, Wez wrote:
> nit: This comment is out-of-date.

Done.

Expand Messages | Collapse Messages