PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the renderer.

PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the renderer.

WILL NOT BE SUBMITTED!

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

[carlosk, 2016-02-11 15:20:05]

Description was changed from

==========
PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the
renderer.

WILL NOT BE SUBMITTED!

BUG=
==========

to

==========
PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the
renderer.

WILL NOT BE SUBMITTED!

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[Ken Rockot(use gerrit already), 2016-02-11 16:40:38]

rockot@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-02-11 16:40:39]

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
File content/browser/loader/mojo_stream_writer.cc (right):

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:35: // The user having to default
initialize it is bad.
The API at this level (mojo/public/c/system) is specifically intended to be
C-compatible, hence no constructor.

The public/cpp mojo::DataPipe however could have a nicer constructor.

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:49: // needed here and in the
Begin* calls below.
The flag has no effect on Begin* (i.e. two-phase) calls. Two-phase reads and
writes give you whatever they can, i.e. the minimum between what you asked for
and what capacity is left before reads/writes would have to wrap around the ring
boundary.

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:52:
MOJO_WRITE_DATA_FLAG_ALL_OR_NONE), MOJO_RESULT_OK);
I haven't dug into the details of how MojoStreamWriter is used, but it looks
like you're only calling EndWriteData when the consumer signals that it's read
data. This should mean that no data can ever be read or written. BeginWriteData
essentially just locks a portion of the pipe buffer. EndWriteData makes whatever
you wrote available to the consumer. Did I just miss something?

[carlosk, 2016-02-16 12:34:39]

Description was changed from

==========
PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the
renderer.

WILL NOT BE SUBMITTED!

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
PROTOTYPE: PlzNavigate: use a Mojo data pipe to stream navigation data to the
renderer.

WILL NOT BE SUBMITTED!

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[carlosk, 2016-02-16 15:34:07]

Thanks Ken.

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
File content/browser/loader/mojo_stream_writer.cc (right):

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:35: // The user having to default
initialize it is bad.
On 2016/02/11 16:40:39, Ken Rockot wrote:
> The API at this level (mojo/public/c/system) is specifically intended to be
> C-compatible, hence no constructor.
> 
> The public/cpp mojo::DataPipe however could have a nicer constructor.

Or the one in mojo/public/cpp/system/data_pipe.h could have a nice C++11 helper
function to build the default options struct.

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:49: // needed here and in the
Begin* calls below.
On 2016/02/11 16:40:39, Ken Rockot wrote:
> The flag has no effect on Begin* (i.e. two-phase) calls. Two-phase reads and
> writes give you whatever they can, i.e. the minimum between what you asked for
> and what capacity is left before reads/writes would have to wrap around the
ring
> boundary.

This is not what the method documentation for MojoBeginWriteData says:
https://code.google.com/p/chromium/codesearch#chromium/src/mojo/public/c/syst...
. Is this accurate?

Is that comment not up to date? As I'm basing this code off of StreamWriter it
enforces that the returned buffer size must be greater or equal to the requested
size. I'm just trying to mimic that behavior.

https://codereview.chromium.org/1693563002/diff/1/content/browser/loader/mojo...
content/browser/loader/mojo_stream_writer.cc:52:
MOJO_WRITE_DATA_FLAG_ALL_OR_NONE), MOJO_RESULT_OK);
On 2016/02/11 16:40:39, Ken Rockot wrote:
> I haven't dug into the details of how MojoStreamWriter is used, but it looks
> like you're only calling EndWriteData when the consumer signals that it's read
> data. This should mean that no data can ever be read or written.
BeginWriteData
> essentially just locks a portion of the pipe buffer. EndWriteData makes
whatever
> you wrote available to the consumer. Did I just miss something?

These overridden observer method names are really confusing. OnWillRead is
called when data from the network will be read, so the buffer to write it to
must be ready. OnReadCompleted after the data was read from the network and it's
supposedly now written to the buffer.

So I think I am indeed using these correctly: BeginWriteDataRaw to allocate the
buffer and EndWriteDataRaw to signal that data has been written to the buffer
and can now be sent to through the pipe.

[Charlie Harrison, 2016-03-21 14:04:37]

csharrison@chromium.org changed reviewers:
+ csharrison@chromium.org

[Charlie Harrison, 2016-03-29 14:31:39]

csharrison@chromium.org changed reviewers:
- csharrison@chromium.org

[carlosk, 2016-09-06 22:45:44]

Patchset #4 (id:60001) has been deleted