Issue 3341005: Add mouse/keyboard event support to Chromoting client (Pepper and X11).

Issue 3341005: Add mouse/keyboard event support to Chromoting client (Pepper and X11). (Closed)

Created:
10 years, 3 months ago by garykac

Modified:
9 years, 6 months ago

Reviewers:
awong, Alpha Left Google

CC:
chromium-reviews, Sergey Ulanov, dmac, awong, garykac, Alpha Left Google

Base URL:
http://src.chromium.org/git/chromium.git

Visibility:
Public.

Description

Add mouse/keyboard event support to Chromoting client (Pepper and X11). BUG=50247 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58761

Patch Set 3 : review comments #

Created: 10 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+49 lines, -6 lines)			Patch
M	remoting/client/input_handler.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	remoting/client/input_handler.cc	View		1 chunk	+10 lines, -0 lines	0 comments	Download
M	remoting/client/plugin/chromoting_instance.cc	View	1 2	1 chunk	+11 lines, -5 lines	0 comments	Download
M	remoting/client/plugin/pepper_input_handler.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	remoting/client/plugin/pepper_input_handler.cc	View	1 2	1 chunk	+9 lines, -0 lines	0 comments	Download
M	remoting/client/x11_input_handler.h	View	1 2	1 chunk	+4 lines, -0 lines	0 comments	Download
M	remoting/client/x11_input_handler.cc	View	1 2	3 chunks	+11 lines, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

awong

LGTM with nits fixed. http://codereview.chromium.org/3341005/diff/2001/3003 File remoting/client/plugin/chromoting_instance.cc (right): http://codereview.chromium.org/3341005/diff/2001/3003#newcode124 remoting/client/plugin/chromoting_instance.cc:124: return true; Can we insert ...

10 years, 3 months ago (2010-09-01 22:35:41 UTC) #2

garykac

10 years, 3 months ago (2010-09-01 23:06:24 UTC) #3

http://codereview.chromium.org/3341005/diff/2001/3003
File remoting/client/plugin/chromoting_instance.cc (right):

http://codereview.chromium.org/3341005/diff/2001/3003#newcode124
remoting/client/plugin/chromoting_instance.cc:124: return true;
On 2010/09/01 22:35:41, awong wrote:
> Can we insert newlines after returns and breaks in switches?
> 
> BTW, what happaens if you return false?

You don't receive keyboard events. ^_^

http://codereview.chromium.org/3341005/diff/2001/3004
File remoting/client/plugin/pepper_input_handler.cc (right):

http://codereview.chromium.org/3341005/diff/2001/3004#newcode21
remoting/client/plugin/pepper_input_handler.cc:21: void
PepperInputHandler::HandleKeyEvent(bool keydown, const PP_Event_Key& event) {
On 2010/09/01 22:35:41, awong wrote:
> 80-chars

Done.

http://codereview.chromium.org/3341005/diff/2001/3006
File remoting/client/x11_input_handler.cc (right):

http://codereview.chromium.org/3341005/diff/2001/3006#newcode80
remoting/client/x11_input_handler.cc:80: int buffsize = sizeof(buffer)-1;
On 2010/09/01 22:35:41, awong wrote:
> Add spaces around binary operators please.

Done.

http://codereview.chromium.org/3341005/diff/2001/3007
File remoting/client/x11_input_handler.h (right):

http://codereview.chromium.org/3341005/diff/2001/3007#newcode31
remoting/client/x11_input_handler.h:31: // cause conflicts with other headers.
See note at top of x11_input_handler.cc.
On 2010/09/01 22:35:41, awong wrote:
> 80-chars.

Done.

Expand Messages | Collapse Messages