Replace cc::ViewportSelectionBound with gfx::SelectionBound

Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a
cc::ViewportSelection(Bound), which closely resembles
gfx::SelectionBound. This CL works towards unifying these
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5ca57dfa32cb7a1d497890a166e82df86077a11d
Cr-Commit-Position: refs/heads/master@{#397855}

[Fady Samuel, 2016-06-02 18:04:30]

Description was changed from

==========
WIP: Replace cc::ViewportSelectionBounds with ui::SelectionBounds

BUG=
==========

to

==========
WIP: Replace cc::ViewportSelectionBounds with ui::SelectionBounds

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-03 03:46:32]

Description was changed from

==========
WIP: Replace cc::ViewportSelectionBounds with ui::SelectionBounds

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
WIP: Replace cc::ViewportSelectionBound with ui::SelectionBound

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-03 03:46:52]

fsamuel@chromium.org changed reviewers:
+ sadrul@chromium.org

[Fady Samuel, 2016-06-03 03:46:53]

Hi Sadrul,

Could you please give me some initial thoughts? Thanks!

Fady

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:8302: //
EXPECT_EQ(selection.start.type(), selection_after.start.type());
What should I do about these types? They're no longer the same :(

[sadrul, 2016-06-03 07:59:21]

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:8302: //
EXPECT_EQ(selection.start.type(), selection_after.start.type());
On 2016/06/03 03:46:53, Fady Samuel wrote:
> What should I do about these types? They're no longer the same :(

Looks like LayerSelectionBound could also be replaced by gfx::SelectionBound?

[Fady Samuel, 2016-06-03 16:24:34]

Description was changed from

==========
WIP: Replace cc::ViewportSelectionBound with ui::SelectionBound

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-03 16:25:16]

+ajuma@ for cc review.
+tsepez@ for ipc review

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/2030033003/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:8302: //
EXPECT_EQ(selection.start.type(), selection_after.start.type());
On 2016/06/03 07:59:21, sadrul wrote:
> On 2016/06/03 03:46:53, Fady Samuel wrote:
> > What should I do about these types? They're no longer the same :(
> 
> Looks like LayerSelectionBound could also be replaced by gfx::SelectionBound?

You're right but that looks like a lot of work. I wanted something more
incremental. Looks like I can just replace the type for now. I'll replace
everything in a subsequent CL.

[Fady Samuel, 2016-06-03 16:25:43]

fsamuel@chromium.org changed reviewers:
+ ajuma@chromium.org, tsepez@chromium.org
- sadrul@chromium.org

[Fady Samuel, 2016-06-03 16:25:44]

+ajuma@ for cc review
+tsepez@ for ipc review.

[Tom Sepez, 2016-06-03 17:17:07]

https://codereview.chromium.org/2030033003/diff/40001/ui/gfx/ipc/gfx_param_tr...
File ui/gfx/ipc/gfx_param_traits.cc (right):

https://codereview.chromium.org/2030033003/diff/40001/ui/gfx/ipc/gfx_param_tr...
ui/gfx/ipc/gfx_param_traits.cc:109:
r->set_type(static_cast<gfx::SelectionBound::Type>(type));
We should add traits for the type enum in IDL using IPC_ENUM_TRAITS_MAX_VALUE in
some .h file, forcing IPC to verify it and getting rid of the cast.

The alternative is manual verification here.

[ajuma, 2016-06-03 17:32:27]

cc lgtm

[Fady Samuel, 2016-06-03 17:38:19]

Description was changed from

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-03 17:39:06]

PTAL Tom!

https://codereview.chromium.org/2030033003/diff/40001/ui/gfx/ipc/gfx_param_tr...
File ui/gfx/ipc/gfx_param_traits.cc (right):

https://codereview.chromium.org/2030033003/diff/40001/ui/gfx/ipc/gfx_param_tr...
ui/gfx/ipc/gfx_param_traits.cc:109:
r->set_type(static_cast<gfx::SelectionBound::Type>(type));
On 2016/06/03 17:17:06, Tom Sepez wrote:
> We should add traits for the type enum in IDL using IPC_ENUM_TRAITS_MAX_VALUE
in
> some .h file, forcing IPC to verify it and getting rid of the cast.
> 
> The alternative is manual verification here.

Done. Added IPC_ENUM_TRAITS_MAX_VALUE to ui/gfx/ipc since it's a ui/gfx type.

[Tom Sepez, 2016-06-03 17:47:04]

lgtm

[Fady Samuel, 2016-06-03 17:48:39]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 17:48:40]

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

[commit-bot: I haz the power, 2016-06-03 17:49:21]

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

[commit-bot: I haz the power, 2016-06-03 17:57:31]

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

[commit-bot: I haz the power, 2016-06-03 17:57:32]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Fady Samuel, 2016-06-03 17:58:31]

Description was changed from

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Fady Samuel, 2016-06-03 17:58:36]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2016-06-03 17:59:15]

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

[commit-bot: I haz the power, 2016-06-03 18:05:26]

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

[commit-bot: I haz the power, 2016-06-03 18:05:28]

Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Fady Samuel, 2016-06-03 18:13:35]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 18:13:35]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2030033003/#ps80001
(title: "Add gyp geometry dependency")

[commit-bot: I haz the power, 2016-06-03 18:13:58]

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

[commit-bot: I haz the power, 2016-06-03 18:32:46]

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

[commit-bot: I haz the power, 2016-06-03 18:32:47]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Fady Samuel, 2016-06-03 19:10:11]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 19:10:11]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2030033003/#ps100001
(title: "Fix android")

[commit-bot: I haz the power, 2016-06-03 19:10:44]

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

[commit-bot: I haz the power, 2016-06-03 19:30:06]

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

[commit-bot: I haz the power, 2016-06-03 19:30:08]

Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Fady Samuel, 2016-06-03 20:56:15]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 20:56:16]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2030033003/#ps120001
(title: "Another attempt at fixing android")

[commit-bot: I haz the power, 2016-06-03 20:56:44]

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

[commit-bot: I haz the power, 2016-06-03 21:15:27]

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

[commit-bot: I haz the power, 2016-06-03 21:15:28]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Fady Samuel, 2016-06-03 22:16:43]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 22:16:43]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2030033003/#ps140001
(title: "More Android fixing")

[commit-bot: I haz the power, 2016-06-03 22:17:07]

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

[commit-bot: I haz the power, 2016-06-03 22:34:51]

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

[commit-bot: I haz the power, 2016-06-03 22:34:53]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Fady Samuel, 2016-06-03 22:57:39]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-06-03 22:57:39]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2030033003/#ps160001
(title: "Android build compiles locally")

[commit-bot: I haz the power, 2016-06-03 22:58:10]

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

[commit-bot: I haz the power, 2016-06-04 00:35:14]

Description was changed from

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-04 00:35:16]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-06-04 00:37:47]

Description was changed from

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a 
cc::ViewportSelection(Bound), which closely resembles 
gfx::SelectionBound. This CL works towards unifying these 
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace cc::ViewportSelectionBound with ui::SelectionBound

We'd like to be able to fully serialize CompositorFrames over
mojo. CompositorFrameMetadata has a
cc::ViewportSelection(Bound), which closely resembles
gfx::SelectionBound. This CL works towards unifying these
structs to reduce code duplication.

BUG=611802
TBR=piman@ for content changes.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5ca57dfa32cb7a1d497890a166e82df86077a11d
Cr-Commit-Position: refs/heads/master@{#397855}
==========

[commit-bot: I haz the power, 2016-06-04 00:37:49]

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

[Nico, 2017-05-12 14:54:28]

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

[Nico, 2017-05-12 14:54:29]

(fix here: https://chromium-review.googlesource.com/c/504767/)

https://codereview.chromium.org/2030033003/diff/160001/ui/gfx/ipc/BUILD.gn
File ui/gfx/ipc/BUILD.gn (left):

https://codereview.chromium.org/2030033003/diff/160001/ui/gfx/ipc/BUILD.gn#ol...
ui/gfx/ipc/BUILD.gn:2: # Use of this source code is governed by a BSD-style
license that can be
tsk tsk