Move screen capturers to remoting/capturer.

Move screen capturers to remoting/capturer.

Separating screen capturer code from the rest of remoting code so that it
can be reused for screen capturing in WebRTC. Also added struct 
MouseCursorShape to avoid protobuf dependency.

BUG=134249
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173789

[Sergey Ulanov, 2012-12-08 02:09:38]

tsepez@ - please approve chromoting_messages.h
wez@ - everything else

[Tom Sepez, 2012-12-08 16:46:00]

chromoting_messages.h LGTM.

[Sergey Ulanov, 2012-12-10 23:31:42]

wez:ping

[Wez, 2012-12-11 05:23:18]

LGTM, but please let alexeypa@ take a look at the IPC changes.

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/c...
File remoting/capturer/capturer_mock_objects.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/c...
remoting/capturer/capturer_mock_objects.h:5: #ifndef
REMOTING_CAPTURER_CAPTURER_MOCK_OBJECTS_H_
nit: Rename this video_capturer_mock_objects.h?

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/c...
remoting/capturer/capturer_mock_objects.h:6: #define
REMOTING_CAPTURER_CAPTURER_MOCK_OBJECTS_H_
Code review seems to have this as a new file, rather than a move?

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/d...
File remoting/capturer/differ.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/d...
remoting/capturer/differ.h:6: #define REMOTING_HOST_DIFFER_H_
nit: Consider re-naming to video_frame_differencer.h

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/d...
File remoting/capturer/differ_block_unittest.cc (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/d...
remoting/capturer/differ_block_unittest.cc:13: static const int kTimesToRun =
900;
We should follow up and move this out to a separate perf-test.

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
File remoting/capturer/mouse_cursor_shape.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
remoting/capturer/mouse_cursor_shape.h:12: // Type is used to define shape of
the mouse cursor.
nit: Clarify that this is used to return cursor shapes from capturers?

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
remoting/capturer/mouse_cursor_shape.h:19: int height;
nit: Use SkISize here.

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
remoting/capturer/mouse_cursor_shape.h:23: int hotspot_y;
nit: Use SkIPoint here.

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/v...
File remoting/capturer/video_frame_capturer_linux.cc (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/v...
remoting/capturer/video_frame_capturer_linux.cc:396:
delegate_->OnCursorShapeChanged(cursor);
We deliberately had the recipient own the cursor, so that it can be passed
around if necessary, e.g. to another thread for encoding to write to the wire.

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/v...
File remoting/capturer/video_frame_capturer_win.cc (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/v...
remoting/capturer/video_frame_capturer_win.cc:123: MouseCursorShape
last_cursor_;
If you keep these as scoped_ptr<> then you can avoid some copying. :)

[Sergey Ulanov, 2012-12-12 18:31:12]

alexeypa: ping

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/capturer...
File remoting/capturer/capturer_mock_objects.h (right):

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/capturer...
remoting/capturer/capturer_mock_objects.h:5: #ifndef
REMOTING_CAPTURER_CAPTURER_MOCK_OBJECTS_H_
On 2012/12/11 05:23:18, Wez wrote:
> nit: Rename this video_capturer_mock_objects.h?

Done.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/capturer...
remoting/capturer/capturer_mock_objects.h:6: #define
REMOTING_CAPTURER_CAPTURER_MOCK_OBJECTS_H_
On 2012/12/11 05:23:18, Wez wrote:
> Code review seems to have this as a new file, rather than a move?

It's correct. I've moved only parts of host_mock_objects.h that are related to
capturer.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/differ.h
File remoting/capturer/differ.h (right):

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/differ.h...
remoting/capturer/differ.h:6: #define REMOTING_HOST_DIFFER_H_
On 2012/12/11 05:23:18, Wez wrote:
> nit: Consider re-naming to video_frame_differencer.h

Fixed the header guard and added TODO to rename it.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/mouse_cu...
File remoting/capturer/mouse_cursor_shape.h (right):

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/mouse_cu...
remoting/capturer/mouse_cursor_shape.h:12: // Type is used to define shape of
the mouse cursor.
On 2012/12/11 05:23:18, Wez wrote:
> nit: Clarify that this is used to return cursor shapes from capturers?

Done.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/mouse_cu...
remoting/capturer/mouse_cursor_shape.h:19: int height;
On 2012/12/11 05:23:18, Wez wrote:
> nit: Use SkISize here.

Done.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/mouse_cu...
remoting/capturer/mouse_cursor_shape.h:23: int hotspot_y;
On 2012/12/11 05:23:18, Wez wrote:
> nit: Use SkIPoint here.

Done.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/video_fr...
File remoting/capturer/video_frame_capturer_linux.cc (right):

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/video_fr...
remoting/capturer/video_frame_capturer_linux.cc:396:
delegate_->OnCursorShapeChanged(cursor);
On 2012/12/11 05:23:18, Wez wrote:
> We deliberately had the recipient own the cursor, so that it can be passed
> around if necessary, e.g. to another thread for encoding to write to the wire.

Changed it to passing pointers, though I don't think it matters much. Cursor
objects are small and it's even less of a problem given that many STL
implementations use ref-counting for std::string.

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/video_fr...
File remoting/capturer/video_frame_capturer_win.cc (right):

https://codereview.chromium.org/11470028/diff/3001/remoting/capturer/video_fr...
remoting/capturer/video_frame_capturer_win.cc:123: MouseCursorShape
last_cursor_;
On 2012/12/11 05:23:18, Wez wrote:
> If you keep these as scoped_ptr<> then you can avoid some copying. :)

We need to copy it either way because we need pass cursor to the Delegate.

[alexeypa (please no reviews), 2012-12-12 19:53:09]

lgtm

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
File remoting/capturer/mouse_cursor_shape.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/3001/remoting/capturer/m...
remoting/capturer/mouse_cursor_shape.h:26: std::string data;
nit: std::vector<uint8> is more appropriate here.

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/capturer/...
File remoting/capturer/video_frame_capturer_mac.mm (right):

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/capturer/...
remoting/capturer/video_frame_capturer_mac.mm:58: void CopyRect(const uint8*
src_plane,
nit: Add a comment explaining what this function does.

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/capturer/...
remoting/capturer/video_frame_capturer_mac.mm:58: void CopyRect(const uint8*
src_plane,
Did you forget to delete CopyRect from base/util.h & .cc?

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/host...
File remoting/host/host_mock_objects.cc (right):

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/host...
remoting/host/host_mock_objects.cc:10: #include
"remoting/capturer/capture_data.h"
nit: Do you still need the include?

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/host...
File remoting/host/host_mock_objects.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/host...
remoting/host/host_mock_objects.h:9: #include "remoting/capturer/capture_data.h"
nit: Do you still need these two includes?

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/ipc_...
File remoting/host/ipc_video_frame_capturer.h (right):

https://chromiumcodereview.appspot.com/11470028/diff/12001/remoting/host/ipc_...
remoting/host/ipc_video_frame_capturer.h:17: struct MouseCursorShape;
nit: Move |MouseCursorShare| after |DesktopSessionProxy|

[Sergey Ulanov, 2012-12-13 02:57:22]

https://codereview.chromium.org/11470028/diff/12001/remoting/capturer/video_f...
File remoting/capturer/video_frame_capturer_mac.mm (right):

https://codereview.chromium.org/11470028/diff/12001/remoting/capturer/video_f...
remoting/capturer/video_frame_capturer_mac.mm:58: void CopyRect(const uint8*
src_plane,
On 2012/12/12 19:53:10, alexeypa wrote:
> nit: Add a comment explaining what this function does.

Done.

https://codereview.chromium.org/11470028/diff/12001/remoting/capturer/video_f...
remoting/capturer/video_frame_capturer_mac.mm:58: void CopyRect(const uint8*
src_plane,
On 2012/12/12 19:53:10, alexeypa wrote:
> Did you forget to delete CopyRect from base/util.h & .cc?

Done.

https://codereview.chromium.org/11470028/diff/12001/remoting/host/host_mock_o...
File remoting/host/host_mock_objects.cc (right):

https://codereview.chromium.org/11470028/diff/12001/remoting/host/host_mock_o...
remoting/host/host_mock_objects.cc:10: #include
"remoting/capturer/capture_data.h"
On 2012/12/12 19:53:10, alexeypa wrote:
> nit: Do you still need the include?

Done.

https://codereview.chromium.org/11470028/diff/12001/remoting/host/host_mock_o...
File remoting/host/host_mock_objects.h (right):

https://codereview.chromium.org/11470028/diff/12001/remoting/host/host_mock_o...
remoting/host/host_mock_objects.h:9: #include "remoting/capturer/capture_data.h"
On 2012/12/12 19:53:10, alexeypa wrote:
> nit: Do you still need these two includes?

Done.

https://codereview.chromium.org/11470028/diff/12001/remoting/host/ipc_video_f...
File remoting/host/ipc_video_frame_capturer.h (right):

https://codereview.chromium.org/11470028/diff/12001/remoting/host/ipc_video_f...
remoting/host/ipc_video_frame_capturer.h:17: struct MouseCursorShape;
On 2012/12/12 19:53:10, alexeypa wrote:
> nit: Move |MouseCursorShare| after |DesktopSessionProxy|

Done.

[commit-bot: I haz the power, 2012-12-13 02:57:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/21002

[commit-bot: I haz the power, 2012-12-13 02:58:45]

Presubmit check for 11470028-21002 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright (\(c\) )?(2012|2011|2010|2009|2008|2007|2006|2006-2008) The
Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?:
\*/)?\n
Found a bad license header in these files:
  remoting/capturer/mouse_cursor_shape.h

Presubmit checks took 5.8s to calculate.

[commit-bot: I haz the power, 2012-12-14 00:42:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/26001

[commit-bot: I haz the power, 2012-12-14 00:43:37]

Presubmit check for 11470028-26001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright (\(c\) )?(2012|2011|2010|2009|2008|2007|2006|2006-2008) The
Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?:
\*/)?\n
Found a bad license header in these files:
  remoting/capturer/mouse_cursor_shape.h

Presubmit checks took 5.8s to calculate.

[commit-bot: I haz the power, 2012-12-14 01:46:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/24002

[commit-bot: I haz the power, 2012-12-14 02:27:43]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-12-15 01:14:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/35003

[commit-bot: I haz the power, 2012-12-15 01:50:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-12-17 21:28:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/46002

[commit-bot: I haz the power, 2012-12-17 22:37:43]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-12-18 02:18:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/30008

[commit-bot: I haz the power, 2012-12-18 06:40:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/11470028/39008

[commit-bot: I haz the power, 2012-12-18 07:15:18]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.