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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 664243002: Mojo UDP: add a client-side wrapper which handles the client<->server buffering (Closed)

Created:
6 years, 2 months ago by yzshen1
Modified:
4 years, 4 months ago
Reviewers:
brettw
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org, wtc
Base URL:
https://chromium.googlesource.com/chromium/src.git/+/master
Project:
chromium
Visibility:
Public.

Description

Mojo UDP: add a client-side wrapper which handles the client<->server buffering for users. BUG=402671 TEST=newly added unittest

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+472 lines, -60 lines) Patch
M mojo/mojo_services.gypi View 1 chunk +0 lines, -20 lines 0 comments Download
M mojo/services/network/BUILD.gn View 2 chunks +2 lines, -1 line 0 comments Download
M mojo/services/network/udp_socket_impl.cc View 3 chunks +7 lines, -2 lines 0 comments Download
M mojo/services/network/udp_socket_unittest.cc View 1 2 10 chunks +107 lines, -35 lines 0 comments Download
M mojo/services/public/cpp/network/BUILD.gn View 2 chunks +3 lines, -1 line 0 comments Download
A mojo/services/public/cpp/network/udp_socket_wrapper.h View 1 2 1 chunk +145 lines, -0 lines 0 comments Download
A mojo/services/public/cpp/network/udp_socket_wrapper.cc View 1 chunk +207 lines, -0 lines 0 comments Download
M mojo/services/public/interfaces/network/udp_socket.mojom View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (1 generated)
yzshen1
Hi, Brett. Would you please take a look? Thanks!
6 years, 2 months ago (2014-10-20 21:18:36 UTC) #2
brettw
lgtm https://codereview.chromium.org/664243002/diff/20001/mojo/services/network/udp_socket_unittest.cc File mojo/services/network/udp_socket_unittest.cc (right): https://codereview.chromium.org/664243002/diff/20001/mojo/services/network/udp_socket_unittest.cc#newcode433 mojo/services/network/udp_socket_unittest.cc:433: TestCallback* send_callbacks = new TestCallback[i]; Can you use ...
6 years, 2 months ago (2014-10-21 18:21:34 UTC) #3
yzshen1
6 years, 2 months ago (2014-10-21 21:59:30 UTC) #4 
Thanks Brett!

https://codereview.chromium.org/664243002/diff/20001/mojo/services/network/ud...
File mojo/services/network/udp_socket_unittest.cc (right):

https://codereview.chromium.org/664243002/diff/20001/mojo/services/network/ud...
mojo/services/network/udp_socket_unittest.cc:433: TestCallback* send_callbacks =
new TestCallback[i];
On 2014/10/21 18:21:33, brettw wrote:
> Can you use scoped_ptr<TestCallback[]> and same for the receive callbacks?

Done.

https://codereview.chromium.org/664243002/diff/20001/mojo/services/public/cpp...
File mojo/services/public/cpp/network/udp_socket_wrapper.h (right):

https://codereview.chromium.org/664243002/diff/20001/mojo/services/public/cpp...
mojo/services/public/cpp/network/udp_socket_wrapper.h:14: class UDPSocketWrapper
: public UDPSocketClient {
On 2014/10/21 18:21:33, brettw wrote:
> It seems like this should have an extensive comment here about what it does
and
> some examples since this is supposed to be public.

Done.

Powered by Google App Engine
This is Rietveld 408576698