Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(120)

Issue 10382184: [Chromoting] Initial plumbing for cursor shape. (Closed)

Created:
8 years, 7 months ago by garykac
Modified:
8 years, 6 months ago
Reviewers:
Sergey Ulanov, Wez
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

[Chromoting] Initial plumbing for cursor shape. This cl contains: * protocol for sending cursor shape on control channel from host to client * cross-platform (Pepper) client code for rendering host cursor * Linux host support for reading current cursor shape Separate CLs will follow with Mac and Windows host support. BUG=116229 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140205

Patch Set 1 #

Patch Set 2 : Add comment about ignoring cursor shape packets on client #

Patch Set 3 : fff #

Patch Set 4 : Add comment for CaptureCursor #

Total comments: 26

Patch Set 5 : Use control channel #

Patch Set 6 : Add missing cursor shape stub file #

Patch Set 7 : Remove extra LOGs #

Total comments: 53

Patch Set 8 : Review comments #

Patch Set 9 : Fixup tests #

Patch Set 10 : Call SetCursor only for native format images #

Total comments: 4

Patch Set 11 : Sync and merge. Move stub into Instance. #

Patch Set 12 : Use ClientStub instead of CursorShapeStub #

Total comments: 8

Patch Set 13 : VLOG and static_cast #

Patch Set 14 : Fix mac/win compiles/ #

Patch Set 15 : Fix merge conflict with InstancePrivate #

Patch Set 16 : Fix Mac capturer unittest #

Patch Set 17 : Remove unused vars. Fix Mac Capturer Unittest. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+332 lines, -33 lines) Patch
M remoting/client/chromoting_client.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -1 line 0 comments Download
M remoting/client/chromoting_client.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +5 lines, -0 lines 0 comments Download
M remoting/client/chromoting_view.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +4 lines, -0 lines 0 comments Download
M remoting/client/plugin/chromoting_instance.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +6 lines, -0 lines 0 comments Download
M remoting/client/plugin/chromoting_instance.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +50 lines, -0 lines 0 comments Download
M remoting/client/plugin/pepper_view.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M remoting/client/plugin/pepper_view.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
M remoting/host/capturer.h View 1 2 3 4 5 6 7 3 chunks +13 lines, -2 lines 0 comments Download
M remoting/host/capturer_fake.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -1 line 0 comments Download
M remoting/host/capturer_fake.cc View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -1 line 0 comments Download
M remoting/host/capturer_linux.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 11 chunks +87 lines, -11 lines 0 comments Download
M remoting/host/capturer_mac.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +10 lines, -3 lines 0 comments Download
M remoting/host/capturer_mac_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +19 lines, -1 line 0 comments Download
M remoting/host/capturer_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +10 lines, -2 lines 0 comments Download
M remoting/host/host_mock_objects.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
M remoting/host/screen_recorder.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +6 lines, -0 lines 0 comments Download
M remoting/host/screen_recorder.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +28 lines, -1 line 0 comments Download
M remoting/host/screen_recorder_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M remoting/proto/control.proto View 1 2 3 4 5 6 7 1 chunk +13 lines, -0 lines 0 comments Download
M remoting/proto/internal.proto View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/client_control_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +3 lines, -2 lines 0 comments Download
M remoting/protocol/client_control_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M remoting/protocol/client_stub.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -1 line 0 comments Download
M remoting/protocol/connection_to_client.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/connection_to_host.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
A remoting/protocol/cursor_shape_stub.h View 1 2 3 4 5 6 7 1 chunk +31 lines, -0 lines 0 comments Download
M remoting/protocol/host_control_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +9 lines, -4 lines 0 comments Download
M remoting/protocol/host_control_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +7 lines, -0 lines 0 comments Download
M remoting/protocol/protocol_mock_objects.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
garykac
8 years, 7 months ago (2012-05-15 22:55:05 UTC) #1
Sergey Ulanov
http://codereview.chromium.org/10382184/diff/4002/remoting/host/capturer.h File remoting/host/capturer.h (right): http://codereview.chromium.org/10382184/diff/4002/remoting/host/capturer.h#newcode11 remoting/host/capturer.h:11: #include "remoting/base/cursor_shape_data.h" Can you forward-declare CursorShapeData? http://codereview.chromium.org/10382184/diff/4002/remoting/host/capturer.h#newcode11 remoting/host/capturer.h:11: #include ...
8 years, 7 months ago (2012-05-15 23:51:34 UTC) #2
Wez
http://codereview.chromium.org/10382184/diff/4002/remoting/client/chromoting_client.cc File remoting/client/chromoting_client.cc (right): http://codereview.chromium.org/10382184/diff/4002/remoting/client/chromoting_client.cc#newcode126 remoting/client/chromoting_client.cc:126: // TODO(garykac) Add proper support for cursor shape packets. ...
8 years, 7 months ago (2012-05-16 00:20:04 UTC) #3
Sergey Ulanov
http://codereview.chromium.org/10382184/diff/4002/remoting/proto/video.proto File remoting/proto/video.proto (right): http://codereview.chromium.org/10382184/diff/4002/remoting/proto/video.proto#newcode38 remoting/proto/video.proto:38: message CursorShapeInfo { On 2012/05/16 00:20:04, Wez wrote: > ...
8 years, 7 months ago (2012-05-16 20:39:11 UTC) #4
garykac
Changed to use control channel. Includes basic client implements so this works end-to-end for linux-host ...
8 years, 7 months ago (2012-05-22 00:42:02 UTC) #5
Wez
On 2012/05/22 00:42:02, garykac wrote: > Changed to use control channel. > > Includes basic ...
8 years, 7 months ago (2012-05-22 23:29:47 UTC) #6
garykac
On 2012/05/22 23:29:47, Wez wrote: > On 2012/05/22 00:42:02, garykac wrote: > > Changed to ...
8 years, 7 months ago (2012-05-22 23:32:46 UTC) #7
Wez
http://codereview.chromium.org/10382184/diff/6003/remoting/base/cursor_shape_data.h File remoting/base/cursor_shape_data.h (right): http://codereview.chromium.org/10382184/diff/6003/remoting/base/cursor_shape_data.h#newcode16 remoting/base/cursor_shape_data.h:16: class CursorShapeData : public base::RefCountedThreadSafe<CursorShapeData> { This class just ...
8 years, 7 months ago (2012-05-23 00:01:56 UTC) #8
Wez
On 2012/05/22 23:32:46, garykac wrote: > On 2012/05/22 23:29:47, Wez wrote: > > On 2012/05/22 ...
8 years, 7 months ago (2012-05-23 00:02:36 UTC) #9
garykac
On 2012/05/23 00:02:36, Wez wrote: > On 2012/05/22 23:32:46, garykac wrote: > > On 2012/05/22 ...
8 years, 7 months ago (2012-05-23 01:17:02 UTC) #10
garykac
http://codereview.chromium.org/10382184/diff/6003/remoting/base/cursor_shape_data.h File remoting/base/cursor_shape_data.h (right): http://codereview.chromium.org/10382184/diff/6003/remoting/base/cursor_shape_data.h#newcode16 remoting/base/cursor_shape_data.h:16: class CursorShapeData : public base::RefCountedThreadSafe<CursorShapeData> { On 2012/05/23 00:01:57, ...
8 years, 7 months ago (2012-05-26 01:58:01 UTC) #11
Wez
http://codereview.chromium.org/10382184/diff/6003/remoting/client/chromoting_view.h File remoting/client/chromoting_view.h (right): http://codereview.chromium.org/10382184/diff/6003/remoting/client/chromoting_view.h#newcode21 remoting/client/chromoting_view.h:21: class ChromotingView : public protocol::CursorShapeStub { On 2012/05/26 01:58:01, ...
8 years, 6 months ago (2012-05-29 18:02:54 UTC) #12
garykac
http://codereview.chromium.org/10382184/diff/27001/remoting/host/screen_recorder.cc File remoting/host/screen_recorder.cc (right): http://codereview.chromium.org/10382184/diff/27001/remoting/host/screen_recorder.cc#newcode229 remoting/host/screen_recorder.cc:229: FROM_HERE, base::Bind(&ScreenRecorder::DoEncodeCursorShape, On 2012/05/29 18:02:54, Wez wrote: > You ...
8 years, 6 months ago (2012-05-30 23:22:45 UTC) #13
Wez
LGTM - looks great! http://codereview.chromium.org/10382184/diff/37005/remoting/client/plugin/chromoting_instance.cc File remoting/client/plugin/chromoting_instance.cc (right): http://codereview.chromium.org/10382184/diff/37005/remoting/client/plugin/chromoting_instance.cc#newcode612 remoting/client/plugin/chromoting_instance.cc:612: LOG(WARNING) << "Unable to set ...
8 years, 6 months ago (2012-05-30 23:42:41 UTC) #14
garykac
http://codereview.chromium.org/10382184/diff/37005/remoting/client/plugin/chromoting_instance.cc File remoting/client/plugin/chromoting_instance.cc (right): http://codereview.chromium.org/10382184/diff/37005/remoting/client/plugin/chromoting_instance.cc#newcode612 remoting/client/plugin/chromoting_instance.cc:612: LOG(WARNING) << "Unable to set cursor shape - non-native ...
8 years, 6 months ago (2012-05-31 00:29:41 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/garykac@chromium.org/10382184/38002
8 years, 6 months ago (2012-05-31 00:30:04 UTC) #16
commit-bot: I haz the power
Try job failure for 10382184-38002 (previous was lost) (retry) on mac_rel for step "compile" (clobber ...
8 years, 6 months ago (2012-05-31 01:13:39 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/garykac@chromium.org/10382184/40001
8 years, 6 months ago (2012-06-01 19:34:38 UTC) #18
commit-bot: I haz the power
Try job failure for 10382184-40001 (retry) (previous was lost) on mac_rel for step "compile" (clobber ...
8 years, 6 months ago (2012-06-01 20:08:25 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/garykac@chromium.org/10382184/50001
8 years, 6 months ago (2012-06-02 17:16:03 UTC) #20
commit-bot: I haz the power
8 years, 6 months ago (2012-06-02 22:16:47 UTC) #21
Change committed as 140205

Powered by Google App Engine
This is Rietveld 408576698