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

Issue 6602039: P2P sockets IPC dispatcher for the renderer side. (Closed)

Created:
9 years, 9 months ago by Sergey Ulanov
Modified:
9 years, 7 months ago
CC:
chromium-reviews, jam, darin-cc_chromium.org, brettw-cc_chromium.org
Visibility:
Public.

Description

P2P sockets IPC dispatcher for the renderer side. BUG=None TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76731

Patch Set 1 : - #

Total comments: 30

Patch Set 2 : addressed comments #

Total comments: 10

Patch Set 3 : - #

Patch Set 4 : fix compilation #

Patch Set 5 : - #

Unified diffs Side-by-side diffs Delta from patch set Stats (+364 lines, -0 lines) Patch
M chrome/chrome_renderer.gypi View 1 1 chunk +4 lines, -0 lines 0 comments Download
A chrome/renderer/p2p/socket_client.h View 1 2 3 1 chunk +84 lines, -0 lines 0 comments Download
A chrome/renderer/p2p/socket_client.cc View 1 2 3 4 1 chunk +100 lines, -0 lines 0 comments Download
A chrome/renderer/p2p/socket_dispatcher.h View 1 2 3 1 chunk +70 lines, -0 lines 0 comments Download
A chrome/renderer/p2p/socket_dispatcher.cc View 1 2 3 4 1 chunk +95 lines, -0 lines 0 comments Download
M chrome/renderer/render_view.h View 1 2 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/renderer/render_view.cc View 1 2 3 chunks +7 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Sergey Ulanov
9 years, 9 months ago (2011-03-01 18:29:41 UTC) #1
awong
some comments. Mostly based on style and comments. http://codereview.chromium.org/6602039/diff/20/chrome/renderer/p2p/socket_client.cc File chrome/renderer/p2p/socket_client.cc (right): http://codereview.chromium.org/6602039/diff/20/chrome/renderer/p2p/socket_client.cc#newcode69 chrome/renderer/p2p/socket_client.cc:69: void ...
9 years, 9 months ago (2011-03-01 20:21:55 UTC) #2
Alpha Left Google
It looks about right. However I have the following concerns: 1. The use of refcounting ...
9 years, 9 months ago (2011-03-01 21:32:20 UTC) #3
Alpha Left Google
Also one last thing: 4. Please show me the definition of the IPC messages and ...
9 years, 9 months ago (2011-03-01 21:32:48 UTC) #4
Sergey Ulanov
On 2011/03/01 21:32:48, Alpha wrote: > Also one last thing: > > 4. Please show ...
9 years, 9 months ago (2011-03-01 22:42:55 UTC) #5
Sergey Ulanov
http://codereview.chromium.org/6602039/diff/20/chrome/chrome_renderer.gypi File chrome/chrome_renderer.gypi (right): http://codereview.chromium.org/6602039/diff/20/chrome/chrome_renderer.gypi#newcode135 chrome/chrome_renderer.gypi:135: 'renderer/p2p/sockets_dispatcher.cc', On 2011/03/01 21:32:20, Alpha wrote: > hm.. it ...
9 years, 9 months ago (2011-03-02 16:12:28 UTC) #6
Sergey Ulanov
+jam John, can you please make sure I integrate with RenderView correctly.
9 years, 9 months ago (2011-03-02 20:58:05 UTC) #7
Alpha Left Google
Looks good to me if we get the name of classes right. http://codereview.chromium.org/6602039/diff/7001/chrome/renderer/p2p/socket_client.cc File chrome/renderer/p2p/socket_client.cc ...
9 years, 9 months ago (2011-03-02 21:56:10 UTC) #8
Sergey Ulanov
I'd prefer to rename them in a separate CL later. Addressed all other comments. http://codereview.chromium.org/6602039/diff/7001/chrome/renderer/p2p/socket_client.cc ...
9 years, 9 months ago (2011-03-02 23:56:43 UTC) #9
Alpha Left Google
9 years, 9 months ago (2011-03-02 23:58:42 UTC) #10
looking good. LGTM.

Powered by Google App Engine
This is Rietveld 408576698