Add unittests for CursorRendererMac

Add unittests for CursorRendererMac to prevent crbug/659183 from regressing
on OSX.

BUG=678783
Review-Url: https://codereview.chromium.org/2682743006
Cr-Commit-Position: refs/heads/master@{#449481}
Committed: https://chromium.googlesource.com/chromium/src/+/1b08639b651c7514b61d6b905f61719b40d4b3c6

[braveyao, 2017-02-09 17:28:06]

The CQ bit was checked by braveyao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-09 17:28:29]

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, 2017-02-09 18:36:16]

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

[commit-bot: I haz the power, 2017-02-09 18:36:17]

Dry run: This issue passed the CQ dry run.

[braveyao, 2017-02-09 18:37:40]

Description was changed from

==========
Add unittest for CursorRendererMac

Add unittests for CursorRendererMac to prevent crbug/659183 from regressing
on OSX.

BUG=678783
==========

to

==========
Add unittests for CursorRendererMac to prevent crbug/659183 from regressing
on OSX.

BUG=678783
==========

[braveyao, 2017-02-09 18:41:12]

braveyao@chromium.org changed reviewers:
+ miu@chromium.org

[braveyao, 2017-02-09 18:41:12]

miu@, found a way to do the unit test for CursorRenderMac. Please take a look.

[miu, 2017-02-09 21:54:07]

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
File content/browser/media/capture/cursor_renderer_mac.h (right):

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac.h:49: private:
Rather than make OnMouseEvent() private, how about friend'ing the test class?

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
File content/browser/media/capture/cursor_renderer_mac_unittest.mm (right):

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:56: // of the
view). To move the cursor to that point by Quartz Dislay service,
s/Dislay/Display/

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:57: // we need to
convert it into Cocoa coordinates(the origin is at bottom-left
style nit: Don't use "we" in comments. Use 3rd-person form.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:57: // we need to
convert it into Cocoa coordinates(the origin is at bottom-left
nit: space before parenthesis.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:60: void
MoveMouseCursorWithinWindow(gfx::Point point) {
Instead of this method, there's already a utility function to do this thing for
you (in ui/gfx/mac/coordinate_conversion.h):

  CGWarpMouseCursorPosition(gfx::ScreenPointToNSPoint(point));
  cursor_renderer_->OnMouseEvent();

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:68: const int
display_height = CGDisplayPixelsHigh(CGMainDisplayID());
ditto here: Use existing utility function.

[braveyao, 2017-02-09 22:47:05]

The CQ bit was checked by braveyao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-09 22:47:37]

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

[braveyao, 2017-02-09 23:15:36]

All addressed! PTAL!

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
File content/browser/media/capture/cursor_renderer_mac.h (right):

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac.h:49: private:
On 2017/02/09 21:54:07, miu wrote:
> Rather than make OnMouseEvent() private, how about friend'ing the test class?

Done.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
File content/browser/media/capture/cursor_renderer_mac_unittest.mm (right):

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:56: // of the
view). To move the cursor to that point by Quartz Dislay service,
On 2017/02/09 21:54:07, miu wrote:
> s/Dislay/Display/

Done.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:57: // we need to
convert it into Cocoa coordinates(the origin is at bottom-left
On 2017/02/09 21:54:07, miu wrote:
> nit: space before parenthesis.

Done.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:57: // we need to
convert it into Cocoa coordinates(the origin is at bottom-left
On 2017/02/09 21:54:07, miu wrote:
> nit: space before parenthesis.

Done.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:60: void
MoveMouseCursorWithinWindow(gfx::Point point) {
On 2017/02/09 21:54:07, miu wrote:
> Instead of this method, there's already a utility function to do this thing
for
> you (in ui/gfx/mac/coordinate_conversion.h):
> 
>   CGWarpMouseCursorPosition(gfx::ScreenPointToNSPoint(point));
>   cursor_renderer_->OnMouseEvent();

Done.
And we still need to convert it to screen coordinates first, because the target
point is in Aura coordinates for the convenience of CursorRenderer. 
Yes this is quite confusing. The comments above still applies. So I kept it.

https://codereview.chromium.org/2682743006/diff/1/content/browser/media/captu...
content/browser/media/capture/cursor_renderer_mac_unittest.mm:68: const int
display_height = CGDisplayPixelsHigh(CGMainDisplayID());
On 2017/02/09 21:54:07, miu wrote:
> ditto here: Use existing utility function.

Done.

[miu, 2017-02-09 23:30:14]

lgtm. Thanks for following up to help keep our code robust! :)

[commit-bot: I haz the power, 2017-02-10 00:03:37]

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

[commit-bot: I haz the power, 2017-02-10 00:03:39]

Dry run: This issue passed the CQ dry run.

[braveyao, 2017-02-10 00:04:06]

The CQ bit was checked by braveyao@chromium.org

[commit-bot: I haz the power, 2017-02-10 00:04:37]

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, 2017-02-10 00:09:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486685046595850,
"parent_rev": "68c724448023c98f1b1d2af5ead5c431c7559dc7", "commit_rev":
"1b08639b651c7514b61d6b905f61719b40d4b3c6"}

[commit-bot: I haz the power, 2017-02-10 00:10:14]

Description was changed from

==========
Add unittests for CursorRendererMac to prevent crbug/659183 from regressing
on OSX.

BUG=678783
==========

to

==========
Add unittests for CursorRendererMac to prevent crbug/659183 from regressing
on OSX.

BUG=678783

Review-Url: https://codereview.chromium.org/2682743006
Cr-Commit-Position: refs/heads/master@{#449481}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b08639b651c7514b61d6b905f61...
==========

[commit-bot: I haz the power, 2017-02-10 00:10:16]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1b08639b651c7514b61d6b905f61...