Updating Message Reader and Writer classes to use an interface.

Updating Message Reader and Writer classes to use an interface.

This change updates the message reader and writer classes to use an interface
which will allow tests to more easily use fake objects to force specific test
conditions.  The CL which uses this change will be sent out shortly.

I chose this path instead of relying purely on the in/out files to drive the
tests as I think this approach is easier to grok and debug.  Using a fake
object also allows for simulating additional error conditions.

BUG=584463
Committed: https://crrev.com/ba9fd2175b03955c52a60f3bd1ea95313948afe4
Cr-Commit-Position: refs/heads/master@{#385087}

[joedow, 2016-04-04 18:39:03]

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

[joedow, 2016-04-04 18:39:04]

PTAL!

Thanks,
Joe

[Sergey Ulanov, 2016-04-04 22:19:28]

lgtm

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_reader.h (right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader.h:13: // Used for
receiving remote security key messages using a file handle.
Now this interface can be used for any receiver, i.e. it's not only for
files/pipes. Update this comment and the comment for Start()

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_reader_impl.h
(right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader_impl.h:23: // Used
for receiving remote security key messages using a file handle.
This is the same comment as what we have for RemoteSecurityKeyMessageReader.
Maybe change it to something like this:
// RemoteSecurityKeyMessageReader implementation that receives messages from
// a pipe.

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader_impl.h:31: //
RemoteSecurityKeyMessageReader interface.
This should be in public section, because it's not an implementation detail.

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_writer_unittest.cc
(right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_writer_unittest.cc:5:
#include "remoting/host/security_key/remote_security_key_message_writer.h"
I think this should be in remote_security_key_message_writer_impl.h and the file
 needs to be renamed to remote_security_key_message_writer_impl_unittest.cc .
These tests are for the implementation, not for the interface.

[joedow, 2016-04-05 00:50:19]

Thanks!

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_reader.h (right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader.h:13: // Used for
receiving remote security key messages using a file handle.
On 2016/04/04 22:19:28, Sergey Ulanov wrote:
> Now this interface can be used for any receiver, i.e. it's not only for
> files/pipes. Update this comment and the comment for Start()

Done.

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_reader_impl.h
(right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader_impl.h:23: // Used
for receiving remote security key messages using a file handle.
On 2016/04/04 22:19:28, Sergey Ulanov wrote:
> This is the same comment as what we have for RemoteSecurityKeyMessageReader.
> Maybe change it to something like this:
> // RemoteSecurityKeyMessageReader implementation that receives messages from
> // a pipe.

Done.

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_reader_impl.h:31: //
RemoteSecurityKeyMessageReader interface.
On 2016/04/04 22:19:28, Sergey Ulanov wrote:
> This should be in public section, because it's not an implementation detail.

Done.

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
File remoting/host/security_key/remote_security_key_message_writer_unittest.cc
(right):

https://codereview.chromium.org/1852283003/diff/20001/remoting/host/security_...
remoting/host/security_key/remote_security_key_message_writer_unittest.cc:5:
#include "remoting/host/security_key/remote_security_key_message_writer.h"
On 2016/04/04 22:19:28, Sergey Ulanov wrote:
> I think this should be in remote_security_key_message_writer_impl.h and the
file
>  needs to be renamed to remote_security_key_message_writer_impl_unittest.cc .
> These tests are for the implementation, not for the interface.

Done.

[joedow, 2016-04-05 02:12:29]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-04-05 02:12:29]

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

[commit-bot: I haz the power, 2016-04-05 02:12:54]

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

[commit-bot: I haz the power, 2016-04-05 02:18:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-05 02:21:19]

Description was changed from

==========
Updating Message Reader and Writer classes to use an interface.

This change updates the message reader and writer classes to use an interface
which will allow tests to more easily use fake objects to force specific test
conditions.  The CL which uses this change will be sent out shortly.

I chose this path instead of relying purely on the in/out files to drive the
tests as I think this approach is easier to grok and debug.  Using a fake
object also allows for simulating additional error conditions.

BUG=584463
==========

to

==========
Updating Message Reader and Writer classes to use an interface.

This change updates the message reader and writer classes to use an interface
which will allow tests to more easily use fake objects to force specific test
conditions.  The CL which uses this change will be sent out shortly.

I chose this path instead of relying purely on the in/out files to drive the
tests as I think this approach is easier to grok and debug.  Using a fake
object also allows for simulating additional error conditions.

BUG=584463

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

[commit-bot: I haz the power, 2016-04-05 02:21:20]

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