Implement ui/gfx/geometry StructTraits unit tests

Implement ui/gfx/geometry StructTraits unit tests

This CL also fixes a bug where SizeF had int32 types instead of floats.

BUG=611802
Committed: https://crrev.com/9360d7387864d71dd80fb85787fbf33d95e39541
Cr-Commit-Position: refs/heads/master@{#397546}

[Fady Samuel, 2016-06-02 01:34:28]

fsamuel@chromium.org changed reviewers:
+ sky@chromium.org, tsepez@chromium.org

[Fady Samuel, 2016-06-02 01:34:29]

+sky@ for ui/ and mojo DEP OWNER 
+tsepez@ for ipc changes

[sky, 2016-06-02 03:22:21]

LGTM

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

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/BUILD.gn#newcode815
ui/gfx/BUILD.gn:815: "geometry/mojo:unit_test",
Can you make deps that aren't in the same absolute? From the style guide: "Other
deps should always use fully-qualified path names unless relative ones are
required for some reason.".

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:64: };
nit: DISALLOW..

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_traits_test_service.mojom (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_traits_test_service.mojom:9: interface
GeometryTraitsTestService {
nit: add description. At the least that all functions echo their arguments.

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/mojo/traits_test...
File ui/gfx/mojo/traits_test_service.mojom (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/mojo/traits_test...
ui/gfx/mojo/traits_test_service.mojom:9: interface TraitsTestService {
nit: comment here too

[Fady Samuel, 2016-06-02 12:09:24]

PTAL Tom

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

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/BUILD.gn#newcode815
ui/gfx/BUILD.gn:815: "geometry/mojo:unit_test",
On 2016/06/02 03:22:21, sky wrote:
> Can you make deps that aren't in the same absolute? From the style guide:
"Other
> deps should always use fully-qualified path names unless relative ones are
> required for some reason.".

Done.

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:64: };
On 2016/06/02 03:22:21, sky wrote:
> nit: DISALLOW..

Done.

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_traits_test_service.mojom (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_traits_test_service.mojom:9: interface
GeometryTraitsTestService {
On 2016/06/02 03:22:21, sky wrote:
> nit: add description. At the least that all functions echo their arguments.

Done.

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/mojo/traits_test...
File ui/gfx/mojo/traits_test_service.mojom (right):

https://codereview.chromium.org/2037453002/diff/20001/ui/gfx/mojo/traits_test...
ui/gfx/mojo/traits_test_service.mojom:9: interface TraitsTestService {
On 2016/06/02 03:22:21, sky wrote:
> nit: comment here too

Done.

[Tom Sepez, 2016-06-02 18:29:57]

LGTM with a few comments.

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc (right):

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:71: const int32_t x =
1234;
what happens with negative values?  These are ints.  Can we make one of them
negative?

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:83: const float y =
6789.0f;
should this have a non-zero fractional part?

[Fady Samuel, 2016-06-02 19:07:43]

Thanks Tom! I've addressed your comments and will CQ.

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
File ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc (right):

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:71: const int32_t x =
1234;
On 2016/06/02 18:29:57, Tom Sepez wrote:
> what happens with negative values?  These are ints.  Can we make one of them
> negative?

Done.

https://codereview.chromium.org/2037453002/diff/40001/ui/gfx/geometry/mojo/ge...
ui/gfx/geometry/mojo/geometry_struct_traits_unittest.cc:83: const float y =
6789.0f;
On 2016/06/02 18:29:57, Tom Sepez wrote:
> should this have a non-zero fractional part?

Done.

[Fady Samuel, 2016-06-02 19:07:47]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-02 19:07:48]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2037453002/#ps60001
(title: "Rebased + Addressed Tom's comment")

[commit-bot: I haz the power, 2016-06-02 19:08:21]

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

[commit-bot: I haz the power, 2016-06-02 20:36:27]

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

[commit-bot: I haz the power, 2016-06-02 20:36:28]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Fady Samuel, 2016-06-02 20:41:48]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-02 20:41:48]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2037453002/#ps80001
(title: "Fixed windows")

[commit-bot: I haz the power, 2016-06-02 20:42:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2037453002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2037453002/80001

[commit-bot: I haz the power, 2016-06-02 23:55:39]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-02 23:56:52]

Description was changed from

==========
Implement ui/gfx/geometry StructTraits unit tests

This CL also fixes a bug where SizeF had int32 types instead of floats.

BUG=611802
==========

to

==========
Implement ui/gfx/geometry StructTraits unit tests

This CL also fixes a bug where SizeF had int32 types instead of floats.

BUG=611802

Committed: https://crrev.com/9360d7387864d71dd80fb85787fbf33d95e39541
Cr-Commit-Position: refs/heads/master@{#397546}
==========

[commit-bot: I haz the power, 2016-06-02 23:56:54]

Patchset 5 (id:??) landed as
https://crrev.com/9360d7387864d71dd80fb85787fbf33d95e39541
Cr-Commit-Position: refs/heads/master@{#397546}