[Chromoting] Add plumbing to carry clipboard events from the host to the client.

[Chromoting] Add plumbing to carry clipboard events from the host to the client.

BUG=117473
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=130023

[simonmorris, 2012-03-29 21:04:39]

ptal

[Wez, 2012-03-30 22:20:15]

lgtm

http://codereview.chromium.org/9921034/diff/1/remoting/client/chromoting_clie...
File remoting/client/chromoting_client.cc (right):

http://codereview.chromium.org/9921034/diff/1/remoting/client/chromoting_clie...
remoting/client/chromoting_client.cc:109: return;
nit: Consider adding the TODO(): Implement me! as you did for the other
direction?

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
File remoting/protocol/client_control_dispatcher.cc (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
remoting/protocol/client_control_dispatcher.cc:49: DCHECK(clipboard_stub_);
not for this CL: It feels like these checks belong in OnInitialized()?

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
File remoting/protocol/client_control_dispatcher.h (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
remoting/protocol/client_control_dispatcher.h:41: // message. Doesn't take
ownership of |clipboard_stub|, which must outlive
nit: Could just say "|clipboard_stub| must outlive this dispatcher", and
similarly above, which would imply ownership (or lack thereof)

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/host_control_...
File remoting/protocol/host_control_dispatcher.h (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/host_control_...
remoting/protocol/host_control_dispatcher.h:46: // message. Doesn't take
ownership of |host_stub|, which must outlive
nit: Could reword this as suggested for ClientControlDispatcher.

[simonmorris, 2012-03-30 22:36:06]

http://codereview.chromium.org/9921034/diff/1/remoting/client/chromoting_clie...
File remoting/client/chromoting_client.cc (right):

http://codereview.chromium.org/9921034/diff/1/remoting/client/chromoting_clie...
remoting/client/chromoting_client.cc:109: return;
On 2012/03/30 22:20:15, Wez wrote:
> nit: Consider adding the TODO(): Implement me! as you did for the other
> direction?

Done.

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
File remoting/protocol/client_control_dispatcher.cc (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
remoting/protocol/client_control_dispatcher.cc:49: DCHECK(clipboard_stub_);
On 2012/03/30 22:20:15, Wez wrote:
> not for this CL: It feels like these checks belong in OnInitialized()?

Could be, though the stubs are used here.

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
File remoting/protocol/client_control_dispatcher.h (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
remoting/protocol/client_control_dispatcher.h:41: // message. Doesn't take
ownership of |clipboard_stub|, which must outlive
On 2012/03/30 22:20:15, Wez wrote:
> nit: Could just say "|clipboard_stub| must outlive this dispatcher", and
> similarly above, which would imply ownership (or lack thereof)

Done.

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/host_control_...
File remoting/protocol/host_control_dispatcher.h (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/host_control_...
remoting/protocol/host_control_dispatcher.h:46: // message. Doesn't take
ownership of |host_stub|, which must outlive
On 2012/03/30 22:20:15, Wez wrote:
> nit: Could reword this as suggested for ClientControlDispatcher.

Done.

[commit-bot: I haz the power, 2012-03-30 23:24:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonmorris@chromium.org/9921034/2003

[Wez, 2012-03-31 00:38:46]

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
File remoting/protocol/client_control_dispatcher.cc (right):

http://codereview.chromium.org/9921034/diff/1/remoting/protocol/client_contro...
remoting/protocol/client_control_dispatcher.cc:49: DCHECK(clipboard_stub_);
On 2012/03/30 22:36:07, simonmorris wrote:
> On 2012/03/30 22:20:15, Wez wrote:
> > not for this CL: It feels like these checks belong in OnInitialized()?
> 
> Could be, though the stubs are used here.

True.  Checking in OnMessageReceived() risks us missing cases where the stubs
weren't set, since we won't realise until a message is received, though. :)

[commit-bot: I haz the power, 2012-03-31 01:28:39]

Change committed as 130023