Simplify WebrtcDataStreamAdapter

Simplify WebrtcDataStreamAdapter

Previously WebrtcDataStreamAdapter acted as a factory of MessagePipe
adapters, now it works as an adapter itself. It gets
webrtc::DataChannelInterface in the constructor and the WebrtcTransport
is responsible for creation of the data channels themselves.

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

[Sergey Ulanov, 2016-07-22 22:48:01]

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

[Jamie, 2016-07-25 18:25:59]

lgtm

https://codereview.chromium.org/2172423002/diff/1/remoting/protocol/webrtc_da...
File remoting/protocol/webrtc_data_stream_adapter.h (right):

https://codereview.chromium.org/2172423002/diff/1/remoting/protocol/webrtc_da...
remoting/protocol/webrtc_data_stream_adapter.h:40: void OnMessage(const
webrtc::DataBuffer& buffer) override;
These are public in the DataChannelObserver interface, so I'm not sure it makes
much sense to declare them private here. The fact that you don't want these to
be public might indicate that inheritance is not the right model here, and you'd
be better off with a private observer as a class member; I suspect that's
overkill in this situation, but something to consider (I'd suggest private
inheritance, but I think that's disallowed).

[Sergey Ulanov, 2016-07-25 18:48:19]

https://codereview.chromium.org/2172423002/diff/1/remoting/protocol/webrtc_da...
File remoting/protocol/webrtc_data_stream_adapter.h (right):

https://codereview.chromium.org/2172423002/diff/1/remoting/protocol/webrtc_da...
remoting/protocol/webrtc_data_stream_adapter.h:40: void OnMessage(const
webrtc::DataBuffer& buffer) override;
On 2016/07/25 18:25:59, Jamie wrote:
> These are public in the DataChannelObserver interface, so I'm not sure it
makes
> much sense to declare them private here. The fact that you don't want these to
> be public might indicate that inheritance is not the right model here, and
you'd
> be better off with a private observer as a class member; I suspect that's
> overkill in this situation, but something to consider (I'd suggest private
> inheritance, but I think that's disallowed).

Putting this interface in private section makes it clear that it's an internal
interface implementation and is not supposed to be called directly. This is
allowed by style guide and the same style is used in many other places in
chromium (and remoting). Adding a private observer class would make this code
more complicated without any benefits.
There was a thread in chromium-dev about it:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CwzjcWd9cYM/dis...

[Sergey Ulanov, 2016-07-25 18:48:30]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-07-25 18:48:57]

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, 2016-07-25 19:27:45]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-25 19:30:24]

Description was changed from

==========
Simplify WebrtcDataStreamAdapter

Previously WebrtcDataStreamAdapter acted as a factory of MessagePipe
adapters, now it works as an adapter itself. It gets
webrtc::DataChannelInterface in the constructor and the WebrtcTransport
is responsible for creation of the data channels themselves.
==========

to

==========
Simplify WebrtcDataStreamAdapter

Previously WebrtcDataStreamAdapter acted as a factory of MessagePipe
adapters, now it works as an adapter itself. It gets
webrtc::DataChannelInterface in the constructor and the WebrtcTransport
is responsible for creation of the data channels themselves.

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

[commit-bot: I haz the power, 2016-07-25 19:30:25]

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