Remove done notifications from incoming message handlers.

Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to
pace the reader, particularly when video renderer is slow and cannot
keep up with the rate of the incoming messages. It's no longer
necessary because we have explicit ACK messages for video packets.

Committed: https://crrev.com/e61b68d97a31c00367a0e98d9d88c74774026482
Cr-Commit-Position: refs/heads/master@{#372509}

[Sergey Ulanov, 2016-01-29 20:52:16]

Description was changed from

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and MessageDecoder were passing done callbacks
to the messages handlers. These callbacks were necessary to pace the
reader, particularly when video renderer is slow and cannot keep up
with the rate of the incoming messages. It's no longer necessary because
we have explicit ACK messages for video packets.
==========

to

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to 
pace the reader, particularly when video renderer is slow and cannot 
keep up with the rate of the incoming messages. It's no longer 
necessary because we have explicit ACK messages for video packets.
==========

[Sergey Ulanov, 2016-01-29 20:52:57]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-01-29 20:53:46]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2016-01-29 23:48:12]

LGTM.

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/audio...
File remoting/protocol/audio_reader.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/audio...
remoting/protocol/audio_reader.cc:26: base::Bind(&base::DoNothing));
Is it possible to remove the callback from ProcessAudioPacket as well (rather
than introducing this helper method) or is it required elsewhere?

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/clien...
File remoting/protocol/client_video_dispatcher.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/clien...
remoting/protocol/client_video_dispatcher.cc:44: base::Bind(&base::DoNothing));
Similarly here, can you remove the callback parameter?

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/messa...
File remoting/protocol/message_reader.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/messa...
remoting/protocol/message_reader.cc:29: closed_(false),
Optional: Since you're modifying this anyway, consider switching to header
initialization for these members.

[Sergey Ulanov, 2016-01-30 00:14:47]

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/audio...
File remoting/protocol/audio_reader.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/audio...
remoting/protocol/audio_reader.cc:26: base::Bind(&base::DoNothing));
On 2016/01/29 23:48:12, Jamie wrote:
> Is it possible to remove the callback from ProcessAudioPacket as well (rather
> than introducing this helper method) or is it required elsewhere?

It's still used on the sender side. Without it AudioPump will keep pumping audio
when the sender is blocked.

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/clien...
File remoting/protocol/client_video_dispatcher.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/clien...
remoting/protocol/client_video_dispatcher.cc:44: base::Bind(&base::DoNothing));
On 2016/01/29 23:48:12, Jamie wrote:
> Similarly here, can you remove the callback parameter?

VideoStub is used on the sender side, and this callback is still useful there,
particularly to measure send latency.

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/messa...
File remoting/protocol/message_reader.cc (right):

https://codereview.chromium.org/1655433002/diff/20001/remoting/protocol/messa...
remoting/protocol/message_reader.cc:29: closed_(false),
On 2016/01/29 23:48:12, Jamie wrote:
> Optional: Since you're modifying this anyway, consider switching to header
> initialization for these members.

Done.

[Sergey Ulanov, 2016-01-30 00:15:11]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-30 00:15:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1655433002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-01-30 00:16:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1655433002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1655433002/40001

[commit-bot: I haz the power, 2016-01-30 00:19:21]

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

[commit-bot: I haz the power, 2016-01-30 00:19:22]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Sergey Ulanov, 2016-01-30 01:08:56]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-30 01:08:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1655433002/#ps60001
(title: " ")

[Sergey Ulanov, 2016-01-30 01:09:36]

The CQ bit was unchecked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-01-30 01:09:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1655433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1655433002/60001

[Sergey Ulanov, 2016-01-30 01:10:23]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-01-30 01:10:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1655433002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-01-30 01:11:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1655433002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1655433002/80001

[commit-bot: I haz the power, 2016-01-30 01:52:42]

Description was changed from

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to 
pace the reader, particularly when video renderer is slow and cannot 
keep up with the rate of the incoming messages. It's no longer 
necessary because we have explicit ACK messages for video packets.
==========

to

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to 
pace the reader, particularly when video renderer is slow and cannot 
keep up with the rate of the incoming messages. It's no longer 
necessary because we have explicit ACK messages for video packets.
==========

[commit-bot: I haz the power, 2016-01-30 01:52:43]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-01-30 01:53:43]

Description was changed from

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to 
pace the reader, particularly when video renderer is slow and cannot 
keep up with the rate of the incoming messages. It's no longer 
necessary because we have explicit ACK messages for video packets.
==========

to

==========
Remove done notifications from incoming message handlers.

Previously MessageReader and ProtobufMessageParse were passing done
callbacks to the messages handlers. These callbacks were necessary to
pace the reader, particularly when video renderer is slow and cannot
keep up with the rate of the incoming messages. It's no longer
necessary because we have explicit ACK messages for video packets.

Committed: https://crrev.com/e61b68d97a31c00367a0e98d9d88c74774026482
Cr-Commit-Position: refs/heads/master@{#372509}
==========

[commit-bot: I haz the power, 2016-01-30 01:53:44]

Patchset 4 (id:??) landed as
https://crrev.com/e61b68d97a31c00367a0e98d9d88c74774026482
Cr-Commit-Position: refs/heads/master@{#372509}

[nicholss, 2016-02-01 16:53:54]

On 2016/01/30 01:53:44, commit-bot: I haz the power wrote:
> Patchset 4 (id:??) landed as
> https://crrev.com/e61b68d97a31c00367a0e98d9d88c74774026482
> Cr-Commit-Position: refs/heads/master@{#372509}

[nicholss, 2016-02-01 16:54:12]

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