X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
Committed: https://crrev.com/4dcd8ad19534e6084fff6bc2229acd90ecf4c316
Cr-Commit-Position: refs/heads/master@{#417978}

[Tom (Use chromium acct), 2016-09-08 22:16:58]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-08 22:17:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom (Use chromium acct), 2016-09-08 22:17:46]

thomasanderson@google.com changed reviewers:
+ sky@chromium.org

[Tom (Use chromium acct), 2016-09-08 22:17:47]

+ sky for review
+ erg FYI

[sky, 2016-09-08 23:41:24]

https://codereview.chromium.org/2327623002/diff/1/ui/views/widget/desktop_aur...
File ui/views/widget/desktop_aura/desktop_screen_x11.h (right):

https://codereview.chromium.org/2327623002/diff/1/ui/views/widget/desktop_aur...
ui/views/widget/desktop_aura/desktop_screen_x11.h:106: class VIEWS_EXPORT
TestDesktopScreenX11 : public DesktopScreenX11 {
Document what this class does and why it's needed. Also, it should be in its own
header that is only built in tests.

[sky, 2016-09-08 23:41:33]

Description was changed from

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
==========

to

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
==========

[sky, 2016-09-08 23:41:33]

sky@chromium.org changed reviewers:
+ erg@chromium.org

[commit-bot: I haz the power, 2016-09-09 00:22:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 00:22:52]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Tom (Use chromium acct), 2016-09-09 01:13:47]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 01:14:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom (Use chromium acct), 2016-09-09 01:14:11]

https://codereview.chromium.org/2327623002/diff/1/ui/views/widget/desktop_aur...
File ui/views/widget/desktop_aura/desktop_screen_x11.h (right):

https://codereview.chromium.org/2327623002/diff/1/ui/views/widget/desktop_aur...
ui/views/widget/desktop_aura/desktop_screen_x11.h:106: class VIEWS_EXPORT
TestDesktopScreenX11 : public DesktopScreenX11 {
On 2016/09/08 23:41:24, sky wrote:
> Document what this class does and why it's needed. Also, it should be in its
own
> header that is only built in tests.

Done.

[commit-bot: I haz the power, 2016-09-09 03:03:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 03:03:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2016-09-09 15:25:35]

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:753: "test/test_desktop_screen_x11.cc",
test_support_internal isn't visible outside of ui/views. You need to move this,
most likely to test_support.

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
File ui/views/test/test_desktop_screen_x11.h (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
ui/views/test/test_desktop_screen_x11.h:30: gfx::Point GetCursorScreenPoint()
override;
Generally we prefix overrides with where the override comes from, e.g.
// DesktopScreenX11:
gfx::Point GetCursorScreenPoint() override;

Also, newline between 30/31.

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
ui/views/test/test_desktop_screen_x11.h:31: void SetCursorScreenPoint(const
gfx::Point& point);
optional: We generally inline trivial functions like this (and then name using
unix_hacker_style), in this case set_cursor_screen_point.

[Tom (Use chromium acct), 2016-09-09 21:22:59]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[Tom (Use chromium acct), 2016-09-09 21:23:17]

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:753: "test/test_desktop_screen_x11.cc",
On 2016/09/09 15:25:35, sky wrote:
> test_support_internal isn't visible outside of ui/views. You need to move
this,
> most likely to test_support.

The tests were alerady able to link
but ok, done

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
File ui/views/test/test_desktop_screen_x11.h (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
ui/views/test/test_desktop_screen_x11.h:30: gfx::Point GetCursorScreenPoint()
override;
On 2016/09/09 15:25:35, sky wrote:
> Generally we prefix overrides with where the override comes from, e.g.
> // DesktopScreenX11:
> gfx::Point GetCursorScreenPoint() override;
> 
> Also, newline between 30/31.

Done.

https://codereview.chromium.org/2327623002/diff/20001/ui/views/test/test_desk...
ui/views/test/test_desktop_screen_x11.h:31: void SetCursorScreenPoint(const
gfx::Point& point);
On 2016/09/09 15:25:35, sky wrote:
> optional: We generally inline trivial functions like this (and then name using
> unix_hacker_style), in this case set_cursor_screen_point.

Done.

[commit-bot: I haz the power, 2016-09-09 21:24:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 21:28:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 21:28:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tom (Use chromium acct), 2016-09-09 21:45:40]

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:753: "test/test_desktop_screen_x11.cc",
On 2016/09/09 21:23:16, Tom Anderson wrote:
> On 2016/09/09 15:25:35, sky wrote:
> > test_support_internal isn't visible outside of ui/views. You need to move
> this,
> > most likely to test_support.
> 
> The tests were alerady able to link
> but ok, done

ERROR at //ui/views/test/ui_controls_factory_desktop_aurax11.cc:28:11: Include
not allowed.
#include "ui/views/test/test_desktop_screen_x11.h"
          ^--------------------------------------
It is not in any dependency of
  //ui/views:test_support_internal
The include file is in the target(s):
  //ui/views:test_support
which should somehow be reachable.

[sky, 2016-09-09 22:21:02]

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn
File ui/views/BUILD.gn (right):

https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn#newco...
ui/views/BUILD.gn:753: "test/test_desktop_screen_x11.cc",
On 2016/09/09 21:45:40, Tom Anderson wrote:
> On 2016/09/09 21:23:16, Tom Anderson wrote:
> > On 2016/09/09 15:25:35, sky wrote:
> > > test_support_internal isn't visible outside of ui/views. You need to move
> > this,
> > > most likely to test_support.
> > 
> > The tests were alerady able to link
> > but ok, done
> 
> ERROR at //ui/views/test/ui_controls_factory_desktop_aurax11.cc:28:11: Include
> not allowed.
> #include "ui/views/test/test_desktop_screen_x11.h"
>           ^--------------------------------------
> It is not in any dependency of
>   //ui/views:test_support_internal
> The include file is in the target(s):
>   //ui/views:test_support
> which should somehow be reachable.

Sorry, I think the way you had it is right. I missed that 'test_support' has a
public_dep on this. So, keep the file in test_support_internal.

[Tom (Use chromium acct), 2016-09-09 22:30:29]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 22:30:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 23:53:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 23:54:00]

Dry run: This issue passed the CQ dry run.

[Tom (Use chromium acct), 2016-09-09 23:54:55]

On 2016/09/09 22:21:02, sky wrote:
> https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn
> File ui/views/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2327623002/diff/20001/ui/views/BUILD.gn#newco...
> ui/views/BUILD.gn:753: "test/test_desktop_screen_x11.cc",
> On 2016/09/09 21:45:40, Tom Anderson wrote:
> > On 2016/09/09 21:23:16, Tom Anderson wrote:
> > > On 2016/09/09 15:25:35, sky wrote:
> > > > test_support_internal isn't visible outside of ui/views. You need to
move
> > > this,
> > > > most likely to test_support.
> > > 
> > > The tests were alerady able to link
> > > but ok, done
> > 
> > ERROR at //ui/views/test/ui_controls_factory_desktop_aurax11.cc:28:11:
Include
> > not allowed.
> > #include "ui/views/test/test_desktop_screen_x11.h"
> >           ^--------------------------------------
> > It is not in any dependency of
> >   //ui/views:test_support_internal
> > The include file is in the target(s):
> >   //ui/views:test_support
> > which should somehow be reachable.
> 
> Sorry, I think the way you had it is right. I missed that 'test_support' has a
> public_dep on this. So, keep the file in test_support_internal.

Moved back.  Looks like tests are all green

[sky, 2016-09-12 15:51:17]

LGTM

[Tom (Use chromium acct), 2016-09-12 17:09:10]

The CQ bit was checked by thomasanderson@google.com

[commit-bot: I haz the power, 2016-09-12 17:09:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 18:31:14]

Description was changed from

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
==========

to

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
==========

[commit-bot: I haz the power, 2016-09-12 18:31:17]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-12 18:32:53]

Description was changed from

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741
==========

to

==========
X11: Add TestDesktopScreenX11 to simulate mouse movement in ui tests

BUG=640741

Committed: https://crrev.com/4dcd8ad19534e6084fff6bc2229acd90ecf4c316
Cr-Commit-Position: refs/heads/master@{#417978}
==========

[commit-bot: I haz the power, 2016-09-12 18:32:55]

Patchset 4 (id:??) landed as
https://crrev.com/4dcd8ad19534e6084fff6bc2229acd90ecf4c316
Cr-Commit-Position: refs/heads/master@{#417978}