Add mojo <--> net IO adapters.

Add mojo <--> net IO adapters.

This refactors the net->mojo adapter currently in the URL loader and moves it to the net_adapters file so it can be shared with the tcp_socket. An adapter going in the opposite direction (currently unused) is also added.

Makes some changes to the existing adapter: naming for clarify (since there is now one going in the opposite direction, things were getting pretty confusing), and some of the setup logic was moved into a static function so it can be shared.

Clarify requirements for calling EndWriteData (the old code called this in error cases as well).

Committed: https://crrev.com/1167859505244b69c95e3f7ceb9c7a048a6c0774
Cr-Commit-Position: refs/heads/master@{#299211}

[brettw, 2014-10-10 19:41:38]

brettw@chromium.org changed reviewers:
+ viettrungluu@chromium.org, yzshen@chromium.org

[brettw, 2014-10-10 19:41:53]

trung: data_pipe.h
yzshen: everything else

[yzshen1, 2014-10-10 21:19:00]

https://codereview.chromium.org/649783002/diff/60001/mojo/public/c/system/dat...
File mojo/public/c/system/data_pipe.h (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/public/c/system/dat...
mojo/public/c/system/data_pipe.h:190: // When |MojoBeginWriteData)_| returtns
MOJO_RESULT_OK, and the caller has
nit: MojoBeginWriteData)_ -> MojoBeginWriteData()

https://codereview.chromium.org/649783002/diff/60001/mojo/public/c/system/dat...
mojo/public/c/system/data_pipe.h:193: // |MojoEndWriteData()| need not be called
for other return values.
need->needs

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
File mojo/services/network/net_adapters.cc (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
mojo/services/network/net_adapters.cc:41: *num_bytes = kMaxReadSize;
It seems a little confusing why we use kMaxReadSize in BeginWrite(). (But I
understand that is from previous code.)

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
mojo/services/network/net_adapters.cc:70: : buffer_(buffer) {
|handle| should be used to initialize the corresponding member?

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
File mojo/services/network/net_adapters.h (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
mojo/services/network/net_adapters.h:67:
NetToMojoIOBuffer(NetToMojoPendingBuffer* pending_buffer);
explicit?

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
mojo/services/network/net_adapters.h:89: scoped_refptr<MojoToNetPendingBuffer>*
pending,
nit: wrong indent.

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ne...
mojo/services/network/net_adapters.h:118:
MojoToNetIOBuffer(MojoToNetPendingBuffer* pending_buffer);
explicit?

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ur...
File mojo/services/network/url_loader_impl.cc (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ur...
mojo/services/network/url_loader_impl.cc:283: url_request_->Read(new
NetToMojoIOBuffer(pending_write_.get()),
This may leak the buffer, because Read() may not add/release ref (e.g., when
fails and returns early).

[brettw, 2014-10-10 22:36:59]

Thanks for the great review. PTAL

https://codereview.chromium.org/649783002/diff/60001/mojo/public/c/system/dat...
File mojo/public/c/system/data_pipe.h (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/public/c/system/dat...
mojo/public/c/system/data_pipe.h:193: // |MojoEndWriteData()| need not be called
for other return values.
It seems correct to me.

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ur...
File mojo/services/network/url_loader_impl.cc (right):

https://codereview.chromium.org/649783002/diff/60001/mojo/services/network/ur...
mojo/services/network/url_loader_impl.cc:283: url_request_->Read(new
NetToMojoIOBuffer(pending_write_.get()),
On 2014/10/10 21:19:00, yzshen1 wrote:
> This may leak the buffer, because Read() may not add/release ref (e.g., when
> fails and returns early).

Ooh, good catch!

[yzshen1, 2014-10-10 22:38:41]

LGTM

[brettw, 2014-10-10 22:40:44]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-10-10 22:41:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/649783002/230001

[commit-bot: I haz the power, 2014-10-10 23:30:20]

Committed patchset #3 (id:230001)

[commit-bot: I haz the power, 2014-10-10 23:31:09]

Patchset 3 (id:??) landed as
https://crrev.com/1167859505244b69c95e3f7ceb9c7a048a6c0774
Cr-Commit-Position: refs/heads/master@{#299211}