Add unittests for null/empty window shapes

Add unittests to check that null/empty window shapes work.

Followup to https://codereview.chromium.org/602013002/, which fixes a bug with empty window shapes not being handled properly on CrOS.

BUG=417014
Committed: https://crrev.com/22a07738bd740028562a26f649ad94a17a6b20bf
Cr-Commit-Position: refs/heads/master@{#297337}

[garykac, 2014-09-26 20:23:17]

garykac@chromium.org changed reviewers:
+ erg@google.com, tapted@chromium.org

[garykac, 2014-09-26 20:23:18]

erg for x11_topmost_window_finder_interactive_uitest.cc
tapted for shaped_app_window_targeter_unittest.cc

Note that the shaped_app_window_targeter tests will fail without
crrev.com/602013002.

[tapted, 2014-09-29 01:16:21]

awesome stuff! <3 tests :)

https://codereview.chromium.org/607623002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc
(right):

https://codereview.chromium.org/607623002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc:104:
ui::MouseEvent move(ui::ET_MOUSE_MOVED,
This is super-nitty, but I kinda feel this would be more compelling in the first
block; along with the "without any custom shapes" test (i.e. just after it).

`move2` doesn't really add much to this test case. However, if sending the
"same" event before and after adding a custom shape changes the window that it
hits, it's easier to see how adding an empty shape changes the behavior.

Here, I had to look up to the SetUp() function to convince myself whether the
coordinates in the event should result in a hit on the window or not.

[Elliot Glaysher, 2014-09-29 17:51:48]

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

[Elliot Glaysher, 2014-09-29 17:51:48]

lgtm

[garykac, 2014-09-29 23:51:00]

https://codereview.chromium.org/607623002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc
(right):

https://codereview.chromium.org/607623002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/shaped_app_window_targeter_unittest.cc:104:
ui::MouseEvent move(ui::ET_MOUSE_MOVED,
On 2014/09/29 01:16:20, tapted wrote:
> This is super-nitty, but I kinda feel this would be more compelling in the
first
> block; along with the "without any custom shapes" test (i.e. just after it).

Moved.

> `move2` doesn't really add much to this test case. However, if sending the
> "same" event before and after adding a custom shape changes the window that it
> hits, it's easier to see how adding an empty shape changes the behavior.

Removed.

> Here, I had to look up to the SetUp() function to convince myself whether the
> coordinates in the event should result in a hit on the window or not.

It took me a while to figure it out as well (since I didn't write the original
test). I considered adding ASCII art to make it clear what the shape was, but
thought it might be too "heavy".  I added one now, so let me know if you think
it's useful to keep in the cl.

[tapted, 2014-09-30 00:47:26]

lgtm

On 2014/09/29 23:50:59, garykac wrote:
> It took me a while to figure it out as well (since I didn't write the original
> test). I considered adding ASCII art to make it clear what the shape was, but
> thought it might be too "heavy".  I added one now, so let me know if you think
> it's useful to keep in the cl. 

nice! I like it. But (a nit) lacking some way to "shade in" a square, I would
just remove the corners, like

// Window Shape (global coordinates)
//    30       70     90       130
//  30          +-----+
//       .      |     |             <- mouse move (40,40)
//  70 +--------|-----|---------+
//     |        |  .  |         |   <- mouse move (80,80)
//  90 +--------|-----|---------+
//              |     |
//  30          +-----+

(IMO, it's easier to infer that the missing parts are the square window, since
my first assumption was that the lines inside were just guidelines rather than
the actual shape).

[garykac, 2014-09-30 00:56:02]

On 2014/09/30 00:47:26, tapted wrote:
> lgtm
> 
> On 2014/09/29 23:50:59, garykac wrote:
> > It took me a while to figure it out as well (since I didn't write the
original
> > test). I considered adding ASCII art to make it clear what the shape was,
but
> > thought it might be too "heavy".  I added one now, so let me know if you
think
> > it's useful to keep in the cl. 
> 
> nice! I like it. But (a nit) lacking some way to "shade in" a square, I would
> just remove the corners, like
> 
> // Window Shape (global coordinates)
> //    30       70     90       130
> //  30          +-----+
> //       .      |     |             <- mouse move (40,40)
> //  70 +--------|-----|---------+
> //     |        |  .  |         |   <- mouse move (80,80)
> //  90 +--------|-----|---------+
> //              |     |
> //  30          +-----+
> 
> (IMO, it's easier to infer that the missing parts are the square window, since
> my first assumption was that the lines inside were just guidelines rather than
> the actual shape).

Done.

[garykac, 2014-09-30 00:56:41]

The CQ bit was checked by garykac@chromium.org

[commit-bot: I haz the power, 2014-09-30 00:57:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/607623002/60001

[commit-bot: I haz the power, 2014-09-30 01:55:34]

Committed patchset #4 (id:60001) as 9665ed566e2eed0081ac67a5c4acd4e765cd0409

[commit-bot: I haz the power, 2014-09-30 01:56:11]

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