Use webrtc::MouseCursorMonitor for cursor shapes

Use webrtc::MouseCursorMonitor for cursor shapes

Use webrtc::MouseCursorMonitor for cursor shapes instead of
webrtc::VideoFrameCapturer, in preparation for deprecating cursor shape
functionality in the latter.

Fix memory corruption in VideoSchedulerTests_StartAndStop, where a lingering
capture task could trigger a expectation action declared on the stack during
tear down. My changes to the test somehow made the race condition more likely.

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

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

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

[Sergey Ulanov, 2013-12-03 02:01:25]

is this ready for review?

[dcaiafa, 2013-12-03 21:02:03]

Yeah. Sorry, I thought I had published it already.

[Sergey Ulanov, 2013-12-03 21:47:28]

Looks mostly good. I have suggestions about cursor object serialization.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
File remoting/host/basic_desktop_environment.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
remoting/host/basic_desktop_environment.cc:46:
webrtc::DesktopCaptureOptions::CreateDefault()));
DesktopCaptureOptions::CreateDefault() creates a new X11 connection for X11
builds. Ideally we want to reuse the same connection that's used by the screen
capturer. Can you please create DesktopCaptureOptions instance in constructor
and then use it here and in CreateVideoCapturer()?

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
remoting/host/basic_desktop_environment.cc:62: return
scoped_ptr<webrtc::ScreenCapturer>(webrtc::ScreenCapturer::Create());
Please specify DesktopCaptureOptions in Create() method. Also
DesktopCaptureOptions now should be used to tell screen capturer if it should
use X damage. So Me2MeDesktopEnvironment can just call
desktop_capture_options_.set_use_update_notifications(true) in the constructor
instead of overriding CreateVideoCapturer().

https://codereview.chromium.org/92473002/diff/20001/remoting/host/chromoting_...
File remoting/host/chromoting_messages.h (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/chromoting_...
remoting/host/chromoting_messages.h:142: IPC_STRUCT_BEGIN(SerializedMouseCursor)
You can make webrtc::MouseCursor serializable without adding this struct. You
just need to add serialization/deserilization code in
chromoting_param_traits.[h|cc] and then webrtc::MouseCursor can be included in
IPC messages directly and you won't need to covert it to/from
SerializedMouseCursor.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/desktop_ses...
File remoting/host/desktop_session_proxy.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/desktop_ses...
remoting/host/desktop_session_proxy.cc:523: return;
bogus IPC message indicates that something went very wrong. In general it's
better to crash in that case instead of continuing. So this should be LOG(FATAL)
instead of LOG(ERROR). In either case you wouldn't need this logic here if you
move deserialization to chromoting_param_traits.cc

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
File remoting/host/ipc_mouse_cursor_monitor.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
remoting/host/ipc_mouse_cursor_monitor.cc:1: // Copyright (c) 2013 The Chromium
Authors. All rights reserved.
nit: don't need (c)

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
remoting/host/ipc_mouse_cursor_monitor.cc:20: }
nit: this can be moved to the previous line.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/video_sched...
File remoting/host/video_scheduler.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/video_sched...
remoting/host/video_scheduler.cc:117: cursor_proto->set_data(data);
Insted of building data and then copying it to the cursor_proto you can build it
directly inside cursor_proto by calling
cursor_proto->mutable_data()->append(....) in the loop.

[dcaiafa, 2013-12-10 01:32:03]

ptal

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
File remoting/host/basic_desktop_environment.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
remoting/host/basic_desktop_environment.cc:46:
webrtc::DesktopCaptureOptions::CreateDefault()));
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> DesktopCaptureOptions::CreateDefault() creates a new X11 connection for X11
> builds. Ideally we want to reuse the same connection that's used by the screen
> capturer. Can you please create DesktopCaptureOptions instance in constructor
> and then use it here and in CreateVideoCapturer()?

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/basic_deskt...
remoting/host/basic_desktop_environment.cc:62: return
scoped_ptr<webrtc::ScreenCapturer>(webrtc::ScreenCapturer::Create());
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> Please specify DesktopCaptureOptions in Create() method. Also
> DesktopCaptureOptions now should be used to tell screen capturer if it should
> use X damage. So Me2MeDesktopEnvironment can just call
> desktop_capture_options_.set_use_update_notifications(true) in the constructor
> instead of overriding CreateVideoCapturer().

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/chromoting_...
File remoting/host/chromoting_messages.h (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/chromoting_...
remoting/host/chromoting_messages.h:142: IPC_STRUCT_BEGIN(SerializedMouseCursor)
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> You can make webrtc::MouseCursor serializable without adding this struct. You
> just need to add serialization/deserilization code in
> chromoting_param_traits.[h|cc] and then webrtc::MouseCursor can be included in
> IPC messages directly and you won't need to covert it to/from
> SerializedMouseCursor.

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/desktop_ses...
File remoting/host/desktop_session_proxy.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/desktop_ses...
remoting/host/desktop_session_proxy.cc:523: return;
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> bogus IPC message indicates that something went very wrong. In general it's
> better to crash in that case instead of continuing. So this should be
LOG(FATAL)
> instead of LOG(ERROR). In either case you wouldn't need this logic here if you
> move deserialization to chromoting_param_traits.cc

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
File remoting/host/ipc_mouse_cursor_monitor.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
remoting/host/ipc_mouse_cursor_monitor.cc:1: // Copyright (c) 2013 The Chromium
Authors. All rights reserved.
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> nit: don't need (c)

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/ipc_mouse_c...
remoting/host/ipc_mouse_cursor_monitor.cc:20: }
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> nit: this can be moved to the previous line.

Done.

https://codereview.chromium.org/92473002/diff/20001/remoting/host/video_sched...
File remoting/host/video_scheduler.cc (right):

https://codereview.chromium.org/92473002/diff/20001/remoting/host/video_sched...
remoting/host/video_scheduler.cc:117: cursor_proto->set_data(data);
On 2013/12/03 21:47:28, Sergey Ulanov wrote:
> Insted of building data and then copying it to the cursor_proto you can build
it
> directly inside cursor_proto by calling
> cursor_proto->mutable_data()->append(....) in the loop.

Done.

[Sergey Ulanov, 2013-12-10 03:09:34]

lgtm

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:95: CHECK(p.get());
Why do you need this CHECK()?

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:123: return false;
nit: add {} because the condition doesn't fit one one

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.h (right):

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.h:42: struct
ParamTraits<scoped_ptr<webrtc::MouseCursor>> {
Does this always compile (e.g. on Windows)? I think you need to add a space
between > and > at the end.

[dcaiafa, 2013-12-10 23:07:36]

Adding cdn@ for chromoting_messages.* rubber stamp. Cris, please take a look at
chromoting_param_traits.* as well. Thanks!

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:95: CHECK(p.get());
Because the scoped_ptr could be NULL. E.g.: Send(new
ChromotingDesktopNetworkMsg_MouseCursor(scoped_ptr<webrtc::MouseCursor>()));

...and it's not allowed. The only reason we use scoped_ptr is because
MouseCursor is immutable.

On 2013/12/10 03:09:35, Sergey Ulanov wrote:
> Why do you need this CHECK()?

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:123: return false;
On 2013/12/10 03:09:35, Sergey Ulanov wrote:
> nit: add {} because the condition doesn't fit one one 

Done.

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.h (right):

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.h:42: struct
ParamTraits<scoped_ptr<webrtc::MouseCursor>> {
On 2013/12/10 03:09:35, Sergey Ulanov wrote:
> Does this always compile (e.g. on Windows)? I think you need to add a space
> between > and > at the end.

Builds fine in Windows. But there might still be an old target compiler that has
problems with that. So, done.

[Sergey Ulanov, 2013-12-11 00:55:58]

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/60001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:95: CHECK(p.get());
On 2013/12/10 23:07:36, dcaiafa wrote:
> Because the scoped_ptr could be NULL. E.g.: Send(new
> ChromotingDesktopNetworkMsg_MouseCursor(scoped_ptr<webrtc::MouseCursor>()));
> 
> ...and it's not allowed. The only reason we use scoped_ptr is because
> MouseCursor is immutable.
> 
> On 2013/12/10 03:09:35, Sergey Ulanov wrote:
> > Why do you need this CHECK()?
> 

right. My point was that scoped_ptr<>::operator->() already asserts when it's
called for NULL, so you don't explicit check here.

[Cris Neckar, 2013-12-11 20:59:39]

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:92: void
ParamTraits<scoped_ptr<webrtc::MouseCursor>>::Write(
This just reads as strange. I would get rid of the scoped_ptr wrapper. Why can't
you just create a ParamTraits for MouseCursor that creates a new instance using
the data off the wire?

[Sergey Ulanov, 2013-12-11 21:16:36]

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:92: void
ParamTraits<scoped_ptr<webrtc::MouseCursor>>::Write(
On 2013/12/11 20:59:40, Cris Neckar wrote:
> This just reads as strange. I would get rid of the scoped_ptr wrapper. Why
can't
> you just create a ParamTraits for MouseCursor that creates a new instance
using
> the data off the wire?

webrtc::MouseCursor is immutable and it doesn't have default constructor, which
is required by ParamTraits<>. The problem is that Read() gets a pointer to an
"empty" instance of an object that and is supposed to fill the data, which
doesn't work for immutable objects.

[Cris Neckar, 2013-12-11 21:32:49]

On 2013/12/11 21:16:36, Sergey Ulanov wrote:
>
https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
> File remoting/host/chromoting_param_traits.cc (right):
> 
>
https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
> remoting/host/chromoting_param_traits.cc:92: void
> ParamTraits<scoped_ptr<webrtc::MouseCursor>>::Write(
> On 2013/12/11 20:59:40, Cris Neckar wrote:
> > This just reads as strange. I would get rid of the scoped_ptr wrapper. Why
> can't
> > you just create a ParamTraits for MouseCursor that creates a new instance
> using
> > the data off the wire?
> 
> webrtc::MouseCursor is immutable and it doesn't have default constructor,
which
> is required by ParamTraits<>. The problem is that Read() gets a pointer to an
> "empty" instance of an object that and is supposed to fill the data, which
> doesn't work for immutable objects.

Eww. Alright I wish we could wrap this in something else as I definitely don't
want this to become a trend.

Let me take a look.

[Cris Neckar, 2013-12-11 21:47:12]

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:92: void
ParamTraits<scoped_ptr<webrtc::MouseCursor>>::Write(
On 2013/12/11 21:16:37, Sergey Ulanov wrote:
> On 2013/12/11 20:59:40, Cris Neckar wrote:
> > This just reads as strange. I would get rid of the scoped_ptr wrapper. Why
> can't
> > you just create a ParamTraits for MouseCursor that creates a new instance
> using
> > the data off the wire?
> 
> webrtc::MouseCursor is immutable and it doesn't have default constructor,
which
> is required by ParamTraits<>. The problem is that Read() gets a pointer to an
> "empty" instance of an object that and is supposed to fill the data, which
> doesn't work for immutable objects.

What is the scope of this object exactly? I assume this will be deleted when the
message handler returns (or even when the message is dispatched)? This means you
are going to have to release ownership at some point prior to this for it to
even work?

Could you just create a wrapper class around MouseCursor which will be created
in the pickle macros and then it can call the constructor for MouseCursor within
it's param traits?

I talked to tsepez@ about this and we are pretty scared of introducing object
lifetime issues if we start scoping things to messages. I'd really like to not
set this precedent and potentially have others copy this code in the future.

[Sergey Ulanov, 2014-01-23 07:23:35]

My change for webrtc::MouseCursor has been rolled to chromium, so you can remove
scoped_ptr<> from this CL now.

[dcaiafa, 2014-01-23 19:15:38]

cdn@, please take another look. We removed the scoped_ptr hack in
chromoting_messages.* and chromoting_param_traits.*.

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/80001/remoting/host/chromoting_...
remoting/host/chromoting_param_traits.cc:92: void
ParamTraits<scoped_ptr<webrtc::MouseCursor>>::Write(
MouseCursor is now mutable so I removed the hack.

On 2013/12/11 21:47:12, Cris Neckar wrote:
> On 2013/12/11 21:16:37, Sergey Ulanov wrote:
> > On 2013/12/11 20:59:40, Cris Neckar wrote:
> > > This just reads as strange. I would get rid of the scoped_ptr wrapper. Why
> > can't
> > > you just create a ParamTraits for MouseCursor that creates a new instance
> > using
> > > the data off the wire?
> > 
> > webrtc::MouseCursor is immutable and it doesn't have default constructor,
> which
> > is required by ParamTraits<>. The problem is that Read() gets a pointer to
an
> > "empty" instance of an object that and is supposed to fill the data, which
> > doesn't work for immutable objects.
> 
> What is the scope of this object exactly? I assume this will be deleted when
the
> message handler returns (or even when the message is dispatched)? This means
you
> are going to have to release ownership at some point prior to this for it to
> even work?
> 
> Could you just create a wrapper class around MouseCursor which will be created
> in the pickle macros and then it can call the constructor for MouseCursor
within
> it's param traits?
> 
> I talked to tsepez@ about this and we are pretty scared of introducing object
> lifetime issues if we start scoping things to messages. I'd really like to not
> set this precedent and potentially have others copy this code in the future.

[Cris Neckar, 2014-01-23 20:16:31]

LGTM for IPC changes with 1 nit

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
remoting/host/chromoting_param_traits.cc:136: return false;
In the constructor for MouseCursor there is an assert that the hotspot is within
the image. Can you check this here please. Not sure what can go wrong if it is
outside the image bounds but seeing as there is and assert I assume it isn't
good.

[Wez, 2014-01-23 20:56:17]

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
remoting/host/chromoting_param_traits.cc:136: return false;
On 2014/01/23 20:16:32, Cris Neckar wrote:
> In the constructor for MouseCursor there is an assert that the hotspot is
within
> the image. Can you check this here please. Not sure what can go wrong if it is
> outside the image bounds but seeing as there is and assert I assume it isn't
> good.

In general it's valid to have a hotspot outside a cursor image; some clients may
not support that, but they should really adapt such cursors as necessary.

[Cris Neckar, 2014-01-23 20:59:02]

On 2014/01/23 20:56:17, Wez wrote:
>
https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
> File remoting/host/chromoting_param_traits.cc (right):
> 
>
https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
> remoting/host/chromoting_param_traits.cc:136: return false;
> On 2014/01/23 20:16:32, Cris Neckar wrote:
> > In the constructor for MouseCursor there is an assert that the hotspot is
> within
> > the image. Can you check this here please. Not sure what can go wrong if it
is
> > outside the image bounds but seeing as there is and assert I assume it isn't
> > good.
> 
> In general it's valid to have a hotspot outside a cursor image; some clients
may
> not support that, but they should really adapt such cursors as necessary.

No worries. Figured I'd bring it up given that MouseCursor asserts on it.

Thank you guys for addressing the scoped pointer thing.. I really appreciate it
:)

[commit-bot: I haz the power, 2014-01-23 21:08:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/140001

[commit-bot: I haz the power, 2014-01-23 21:30:36]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Sergey Ulanov, 2014-01-23 21:32:13]

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
File remoting/host/chromoting_param_traits.cc (right):

https://codereview.chromium.org/92473002/diff/140001/remoting/host/chromoting...
remoting/host/chromoting_param_traits.cc:136: return false;
On 2014/01/23 20:56:18, Wez wrote:
> On 2014/01/23 20:16:32, Cris Neckar wrote:
> > In the constructor for MouseCursor there is an assert that the hotspot is
> within
> > the image. Can you check this here please. Not sure what can go wrong if it
is
> > outside the image bounds but seeing as there is and assert I assume it isn't
> > good.
> 
> In general it's valid to have a hotspot outside a cursor image; some clients
may
> not support that, but they should really adapt such cursors as necessary.

Neither the capturers nor the PPB_MouseCursor interface support hotspot outside
of the image, so webrtc::MessageCursor should never have hotspot outside the
bounds. In the same time I don't think accepting such cursor here will have any
negative effect.

[commit-bot: I haz the power, 2014-01-28 01:51:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/460001

[commit-bot: I haz the power, 2014-01-28 03:04:03]

Retried try job too often on linux_clang for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[dcaiafa, 2014-01-28 20:16:23]

Sergey: please take a look at the latest patch. It fixes a build break in Linux
due to X11 headers being brought in into a top level header file
(basic_desktop_environment.h). It's a bit of a hack but I couldn't come up with
something better.

[Sergey Ulanov, 2014-01-28 20:35:06]

LGTM.

Now I officially hate X11.

https://codereview.chromium.org/92473002/diff/480001/remoting/host/basic_desk...
File remoting/host/basic_desktop_environment.h (right):

https://codereview.chromium.org/92473002/diff/480001/remoting/host/basic_desk...
remoting/host/basic_desktop_environment.h:60: webrtc::DesktopCaptureOptions*
desktop_capture_options() {
nit: I think it's better to return const reference here to make it clear that
BasicDesktopEnvironment owns DesktopCaptureOptions and it should not be mutated.

[dcaiafa, 2014-01-28 22:36:28]

To illustrate the sort of build breaks that X11 introduced: one of its headers
has the following:

#define CurrentTime (int)0

And libjingle has at least two methods called |CurrentTime|.

https://codereview.chromium.org/92473002/diff/480001/remoting/host/basic_desk...
File remoting/host/basic_desktop_environment.h (right):

https://codereview.chromium.org/92473002/diff/480001/remoting/host/basic_desk...
remoting/host/basic_desktop_environment.h:60: webrtc::DesktopCaptureOptions*
desktop_capture_options() {
Me2MeDesktopEnvironment must be able to change it (it calls
set_use_update_notifications). That was actually the reason for adding the
accessor.

On 2014/01/28 20:35:07, Sergey Ulanov wrote:
> nit: I think it's better to return const reference here to make it clear that
> BasicDesktopEnvironment owns DesktopCaptureOptions and it should not be
mutated.

[commit-bot: I haz the power, 2014-01-28 22:38:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/480001

[commit-bot: I haz the power, 2014-01-28 23:21:09]

Retried try job too often on mac_rel for step(s) remoting_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2014-01-29 01:33:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/500001

[commit-bot: I haz the power, 2014-01-29 15:05:19]

Change committed as 247689

[gavinp, 2014-01-29 17:03:46]

On 2014/01/29 15:05:19, I haz the power (commit-bot) wrote:
> Change committed as 247689

I reverted this in r247691, as it broke the build.

As for how a CQ commit can break the build? It seems that trybots don't build
the chromoting code.

[commit-bot: I haz the power, 2014-01-29 20:06:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/520001

[commit-bot: I haz the power, 2014-01-29 20:17:11]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-29 20:20:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/520001

[commit-bot: I haz the power, 2014-01-30 00:08:44]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-30 15:36:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/520001

[commit-bot: I haz the power, 2014-01-30 15:36:21]

Failed to apply patch for remoting/remoting.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file remoting/remoting.gyp
  Hunk #1 FAILED at 297.
  1 out of 1 hunk FAILED -- saving rejects to file remoting/remoting.gyp.rej

Patch:       remoting/remoting.gyp
Index: remoting/remoting.gyp
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index
30ddfa9e0f3829f82c46154bd40e590e1653e63c..955416b01c7c7319394042672830c649b5c587d4
100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -297,6 +297,8 @@
             'host/ipc_host_event_logger.h',
             'host/ipc_input_injector.cc',
             'host/ipc_input_injector.h',
+            'host/ipc_mouse_cursor_monitor.cc',
+            'host/ipc_mouse_cursor_monitor.h',
             'host/ipc_screen_controls.cc',
             'host/ipc_screen_controls.h',
             'host/ipc_util.h',

[commit-bot: I haz the power, 2014-01-30 17:47:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/540001

[commit-bot: I haz the power, 2014-01-30 21:56:23]

Change committed as 248045

[commit-bot: I haz the power, 2014-01-30 21:56:29]

CQ bit was unchecked on CL. Ignoring.

[dcaiafa, 2014-08-07 00:50:44]

Rebased and fixed unittest memory corruption in preparation for relanding this.

jln: please review IPC changes in chromoting_messages.h. Nothing changed since
cdn@ looked at it but he's no longer in the message* OWNERS list :'(

[dcaiafa1, 2014-08-07 15:40:31]

+jschuh -cdn

jschuh: please review IPC in chromoting_messages.h. Notice that nothing changed
since cdn@ reviewed this, but he left the project.

[jschuh, 2014-08-07 15:50:23]

If nothing changed other than rebasing and fixing the test, then the original
lgtm for ipc is fine.

[dcaiafa, 2014-08-07 23:25:37]

The CQ bit was checked by dcaiafa@chromium.org

[commit-bot: I haz the power, 2014-08-08 00:00:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/92473002/580001

[commit-bot: I haz the power, 2014-08-08 06:33:56]

Change committed as 288226