Mojo: Add signal for new data pipe consumer data

Mojo: Add signal for new data pipe consumer data

Adds a new MOJO_HANDLE_SIGNAL_NEW_DATA_READABLE signal. This signal is
raised on a data pipe consumer handle whenever new data is written into
the pipe, and is held high until the next MojoReadData or
MojoBeginReadData attempt on the handle.

This may act as a more useful signal for data pipe consumers who want to
wait for a minimum amount of data to become available before reading, as
it can be watched independent of general readability so that new work is
only attempted when new data is ready.

BUG=700171
Review-Url: https://codereview.chromium.org/2741353002
Cr-Commit-Position: refs/heads/master@{#456450}
Committed: https://chromium.googlesource.com/chromium/src/+/e9032c783d6a470dcd04d3af16cb02d4ee621f7b

[Ken Rockot(use gerrit already), 2017-03-12 21:42:00]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-12 21:42:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-12 22:00:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-12 22:00:00]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Ken Rockot(use gerrit already), 2017-03-12 22:10:46]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-12 22:10:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2017-03-12 22:25:16]

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

[Ken Rockot(use gerrit already), 2017-03-12 22:25:16]

Per your suggestion. Very simple change, most of the work here is just updating
test expectations.

[commit-bot: I haz the power, 2017-03-13 00:09:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 00:09:57]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2017-03-13 15:23:41]

Description was changed from

==========
Mojo: Add new handle signal for new data pipe consumer data

Adds a new MOJO_HANDLE_SIGNAL_NEW_DATA_READABLE signal. This signal is
raised on a data pipe consumer handle whenever new data is written into
the pipe, and is held high until the next MojoReadData or
MojoBeginReadData attempt on the handle.

This may act as a more useful signal for data pipe consumers who want to
wait for a minimum amount of data to become available before reading, as
it can be watched independent of general readability so that new work is
only attempted when new data is ready.

BUG=700171
==========

to

==========
Mojo: Add signal for new data pipe consumer data

Adds a new MOJO_HANDLE_SIGNAL_NEW_DATA_READABLE signal. This signal is
raised on a data pipe consumer handle whenever new data is written into
the pipe, and is held high until the next MojoReadData or
MojoBeginReadData attempt on the handle.

This may act as a more useful signal for data pipe consumers who want to
wait for a minimum amount of data to become available before reading, as
it can be watched independent of general readability so that new work is
only attempted when new data is ready.

BUG=700171
==========

[Ken Rockot(use gerrit already), 2017-03-13 15:31:03]

After thinking a bit more about how this can be used, I think it really is the
right approach for data pipes.

To be clear, while it does improve the one SimpleWatcher use case we have with
ALL_OR_NONE reads, it is still generally hard to use SimpleWatcher for data
pipes even with this signal.

We can however make use of this signal to create a special DataPipeReadWatcher
helper which provides all the nice facilities users want when reading pipes -
namely minimum read size and pause/resume - and is able to handle the case where
it starts watching with (non-"new") data already on the pipe.

My plan is to go back in a follow-up patch and clean up existing SimpleWatcher
users by adding that new helper. WDYT?

https://codereview.chromium.org/2741353002/diff/20001/mojo/edk/system/data_pi...
File mojo/edk/system/data_pipe_consumer_dispatcher.cc (right):

https://codereview.chromium.org/2741353002/diff/20001/mojo/edk/system/data_pi...
mojo/edk/system/data_pipe_consumer_dispatcher.cc:127: new_data_available_ =
false;
Note: No Awakable updates here since we're only causing a signal to go low,
which is uninteresting to Awakables. The follow-up watcher patch updates
watchers accordingly.

https://codereview.chromium.org/2741353002/diff/20001/mojo/edk/system/data_pi...
mojo/edk/system/data_pipe_consumer_dispatcher.cc:127: new_data_available_ =
false;
Note: No Awakable updates here since we're only causing a signal to go low,
which is uninteresting to Awakables. The follow-up watcher patch updates
watchers accordingly.

[Ken Rockot(use gerrit already), 2017-03-13 16:34:35]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 16:34:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 17:49:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 17:49:27]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-03-13 19:03:04]

I think we also need to the same change for the data pipe producer for write?

LGTM

[Ken Rockot(use gerrit already), 2017-03-13 19:06:23]

Yep, I was going to add a similar facility
(MOJO_HANDLE_SIGNAL_NEW_CAPACITY_WRITABLE) for producers in a future patch,
along with a special DataPipeWriteWatcher.

On Mon, Mar 13, 2017 at 12:03 PM, <yzshen@chromium.org> wrote:

> I think we also need to the same change for the data pipe producer for
> write?
>
> LGTM
>
>
>
> https://codereview.chromium.org/2741353002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2017-03-13 19:06:35]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2017-03-13 19:07:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 20:07:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489431995127080,
"parent_rev": "189891d210bb98c91e155d4794dffa03d95f1e15", "commit_rev":
"e9032c783d6a470dcd04d3af16cb02d4ee621f7b"}

[commit-bot: I haz the power, 2017-03-13 20:08:41]

Description was changed from

==========
Mojo: Add signal for new data pipe consumer data

Adds a new MOJO_HANDLE_SIGNAL_NEW_DATA_READABLE signal. This signal is
raised on a data pipe consumer handle whenever new data is written into
the pipe, and is held high until the next MojoReadData or
MojoBeginReadData attempt on the handle.

This may act as a more useful signal for data pipe consumers who want to
wait for a minimum amount of data to become available before reading, as
it can be watched independent of general readability so that new work is
only attempted when new data is ready.

BUG=700171
==========

to

==========
Mojo: Add signal for new data pipe consumer data

Adds a new MOJO_HANDLE_SIGNAL_NEW_DATA_READABLE signal. This signal is
raised on a data pipe consumer handle whenever new data is written into
the pipe, and is held high until the next MojoReadData or
MojoBeginReadData attempt on the handle.

This may act as a more useful signal for data pipe consumers who want to
wait for a minimum amount of data to become available before reading, as
it can be watched independent of general readability so that new work is
only attempted when new data is ready.

BUG=700171

Review-Url: https://codereview.chromium.org/2741353002
Cr-Commit-Position: refs/heads/master@{#456450}
Committed:
https://chromium.googlesource.com/chromium/src/+/e9032c783d6a470dcd04d3af16cb...
==========

[commit-bot: I haz the power, 2017-03-13 20:08:42]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/e9032c783d6a470dcd04d3af16cb...