Add "stylus-tools" Wayland protocol.

Add "stylus-tools" Wayland protocol.
    
This protocol specifies a set of interfaces to control the behavior
of a stylus tool, a special kind of window allowing the user to
explore, annotate or manipulate on-screen content.
    
The first applications of that protocol are:
1. Exclude the stylus tool window from for a voice interaction
   screenshot (implemented in this CL).
2. Only handle stylus input in the stylus tool window and let all
   other types of events pass through to the underlying windows
   (to be implemented).
    
BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
     stylus strokes.

Review-Url: https://codereview.chromium.org/2896943002
Cr-Commit-Position: refs/heads/master@{#474580}
Committed: https://chromium.googlesource.com/chromium/src/+/8e27059bf039fc6e5728c71ad1d4da2ad57b12fc

[Vladislav Kaznacheev, 2017-05-22 23:36:32]

kaznacheev@chromium.org changed reviewers:
+ reveman@chromium.org

[Vladislav Kaznacheev, 2017-05-22 23:36:32]

Please take a look. I am not quite sure if I am doing it right, especially
regarding class properties.

[reveman, 2017-05-23 16:41:06]

looks ok. mostly nits. please include a small unit test for any API you add to
exo::Surface

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface.h
File components/exo/surface.h (right):

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface....
components/exo/surface.h:204: bool GetAnnotationLayer() const { return
annotation_layer_; }
Can we add an ash window property for this instead of requiring browser code to
depend on exo?

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface....
components/exo/surface.h:207: void SetAnnotationLayer(bool on) {
annotation_layer_ = on; }
exo::Surface API should match the wayland interface as closely as possible to
avoid introducing unnecessary terms and concepts. ie. the name here should match
the request we add to the wayland interface for turning this on/off

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
components/exo/wayland/server.cc:3855: surface_->SetAnnotationLayer(true);
Can we add an explicit request for turning this on/off instead of it magically
happening when this interface is instantiated?

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
components/exo/wayland/server.cc:3866: // TODO:
TODO(kaznacheev) and a short explanation and link to bug.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
File
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml
(right):

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:78:
<interface name="zcr_annotation_features_v1" version="1">
nit: please name this interface based on the functionality it provides instead
of this generic name. e.g. zcr_pointer_event_filter_v1.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:83:
If the wl_surface associated with the annotation_features object is destroyed,
nit: please break this line in a way that's consistent with l.80.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:86:
If the annotation_features object is destroyed, the annotation_features state
nit: ditto

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:98:
<enum name="input_mode">
nit: avoid this enum and just use a boolean as there's only two values

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:107:
<request name="set_input_mode">
nit: how about set_stylus_only(bool enabled)?

[Vladislav Kaznacheev, 2017-05-23 23:48:56]

PTAL

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface.h
File components/exo/surface.h (right):

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface....
components/exo/surface.h:204: bool GetAnnotationLayer() const { return
annotation_layer_; }
On 2017/05/23 16:41:06, reveman wrote:
> Can we add an ash window property for this instead of requiring browser code
to
> depend on exo?

Done.

https://codereview.chromium.org/2896943002/diff/20001/components/exo/surface....
components/exo/surface.h:207: void SetAnnotationLayer(bool on) {
annotation_layer_ = on; }
On 2017/05/23 16:41:06, reveman wrote:
> exo::Surface API should match the wayland interface as closely as possible to
> avoid introducing unnecessary terms and concepts. ie. the name here should
match
> the request we add to the wayland interface for turning this on/off

Done.

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
File components/exo/wayland/server.cc (right):

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
components/exo/wayland/server.cc:3855: surface_->SetAnnotationLayer(true);
On 2017/05/23 16:41:06, reveman wrote:
> Can we add an explicit request for turning this on/off instead of it magically
> happening when this interface is instantiated?

Done.

https://codereview.chromium.org/2896943002/diff/20001/components/exo/wayland/...
components/exo/wayland/server.cc:3866: // TODO:
On 2017/05/23 16:41:06, reveman wrote:
> TODO(kaznacheev) and a short explanation and link to bug.

Removed TODO

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
File
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml
(right):

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:78:
<interface name="zcr_annotation_features_v1" version="1">
On 2017/05/23 16:41:06, reveman wrote:
> nit: please name this interface based on the functionality it provides instead
> of this generic name. e.g. zcr_pointer_event_filter_v1.

Done.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:83:
If the wl_surface associated with the annotation_features object is destroyed,
On 2017/05/23 16:41:06, reveman wrote:
> nit: please break this line in a way that's consistent with l.80.

Done.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:86:
If the annotation_features object is destroyed, the annotation_features state
On 2017/05/23 16:41:06, reveman wrote:
> nit: ditto

Done.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:98:
<enum name="input_mode">
On 2017/05/23 16:41:06, reveman wrote:
> nit: avoid this enum and just use a boolean as there's only two values

Done.

https://codereview.chromium.org/2896943002/diff/20001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/annotation/annotation-unstable-v1.xml:107:
<request name="set_input_mode">
On 2017/05/23 16:41:06, reveman wrote:
> nit: how about set_stylus_only(bool enabled)?

Removed the parameter.

[reveman, 2017-05-24 13:38:57]

lgtm + minor nit about language in the protocol description

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
File
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml
(right):

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml:45:
control the stylus-related behavior of the window associated with the
nit: "window associated with..." there's no concept of anything called a window
in the wayland world so this is leaking some implementation details into the
protocol. please reword. maybe " control the stylus-related behavior for input
device event processing related to wl_surface."?

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml:99:
Enables the "stylus-only" mode for the window associated with the
nit: ditto. remove the term "window" somehow and just refer to input events

[Vladislav Kaznacheev, 2017-05-24 18:11:13]

kaznacheev@chromium.org changed reviewers:
+ lhchavez@chromium.org, sky@chromium.org

[Vladislav Kaznacheev, 2017-05-24 18:11:14]

Please review:

sky: ui/aura/client/aura_constants.*
lhchavez:
chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
File
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml
(right):

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml:45:
control the stylus-related behavior of the window associated with the
On 2017/05/24 13:38:57, reveman wrote:
> nit: "window associated with..." there's no concept of anything called a
window
> in the wayland world so this is leaking some implementation details into the
> protocol. please reword. maybe " control the stylus-related behavior for input
> device event processing related to wl_surface."?

Done.

https://codereview.chromium.org/2896943002/diff/40001/third_party/wayland-pro...
third_party/wayland-protocols/unstable/stylus-tools/stylus-tools-unstable-v1.xml:99:
Enables the "stylus-only" mode for the window associated with the
On 2017/05/24 13:38:57, reveman wrote:
> nit: ditto. remove the term "window" somehow and just refer to input events

Done.

[Vladislav Kaznacheev, 2017-05-24 19:21:26]

Description was changed from

==========
Add "annotation" Wayland protocol.

This protocol specifies a set of interfaces to control the behavior
or the annotation layer, a special layer allowing the user to annotate
the on-screen content.

The first applications of that protocol are:
1. Exclude the annotation layer for a voice interaction screenshot
  (implemented in this CL).
2. Only handle stylus input in the annotation layer and let all
   other types of event pass through to the underlying windows.

BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
  stylus strokes.
==========

to

==========
Add "stylus-tools" Wayland protocol.
    
This protocol specifies a set of interfaces to control the behavior
of a stylus tool, a special kind of window allowing the user to
explore, annotate or manipulate on-screen content.
    
The first applications of that protocol are:
1. Exclude the stylus tool window from for a voice interaction
   screenshot (implemented in this CL).
2. Only handle stylus input in the stylus tool window and let all
   other types of events pass through to the underlying windows
   (to be implemented).
    
BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
     stylus strokes.
==========

[sky, 2017-05-24 21:36:38]

https://codereview.chromium.org/2896943002/diff/60001/ui/aura/client/aura_con...
File ui/aura/client/aura_constants.h (right):

https://codereview.chromium.org/2896943002/diff/60001/ui/aura/client/aura_con...
ui/aura/client/aura_constants.h:119: AURA_EXPORT extern const
WindowProperty<bool>* const kStylusOnlyKey;
AFAICT this is only used in chrome. If so, can it be moved to chrome rather than
here?

[Vladislav Kaznacheev, 2017-05-24 22:19:44]

kaznacheev@chromium.org changed reviewers:
- sky@chromium.org

[Vladislav Kaznacheev, 2017-05-24 22:19:45]

https://codereview.chromium.org/2896943002/diff/60001/ui/aura/client/aura_con...
File ui/aura/client/aura_constants.h (right):

https://codereview.chromium.org/2896943002/diff/60001/ui/aura/client/aura_con...
ui/aura/client/aura_constants.h:119: AURA_EXPORT extern const
WindowProperty<bool>* const kStylusOnlyKey;
On 2017/05/24 21:36:38, sky wrote:
> AFAICT this is only used in chrome. If so, can it be moved to chrome rather
than
> here?

Done.

[Luis Héctor Chávez, 2017-05-24 23:05:32]

chrome/browser/chromeos/arc lgtm

[Vladislav Kaznacheev, 2017-05-24 23:23:10]

The CQ bit was checked by kaznacheev@chromium.org

[Vladislav Kaznacheev, 2017-05-24 23:23:10]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/2896943002/#ps80001
(title: "Moved the property out of aura")

[commit-bot: I haz the power, 2017-05-24 23:24:01]

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-05-25 03:31:06]

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

[commit-bot: I haz the power, 2017-05-25 03:31:08]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[Vladislav Kaznacheev, 2017-05-25 05:15:41]

The CQ bit was checked by kaznacheev@chromium.org

[commit-bot: I haz the power, 2017-05-25 05:16:08]

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-05-25 06:13:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1495689341525380,
"parent_rev": "e370c17ca1827988dd84a545bb728f9b6699d42d", "commit_rev":
"8e27059bf039fc6e5728c71ad1d4da2ad57b12fc"}

[commit-bot: I haz the power, 2017-05-25 06:13:43]

Description was changed from

==========
Add "stylus-tools" Wayland protocol.
    
This protocol specifies a set of interfaces to control the behavior
of a stylus tool, a special kind of window allowing the user to
explore, annotate or manipulate on-screen content.
    
The first applications of that protocol are:
1. Exclude the stylus tool window from for a voice interaction
   screenshot (implemented in this CL).
2. Only handle stylus input in the stylus tool window and let all
   other types of events pass through to the underlying windows
   (to be implemented).
    
BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
     stylus strokes.
==========

to

==========
Add "stylus-tools" Wayland protocol.
    
This protocol specifies a set of interfaces to control the behavior
of a stylus tool, a special kind of window allowing the user to
explore, annotate or manipulate on-screen content.
    
The first applications of that protocol are:
1. Exclude the stylus tool window from for a voice interaction
   screenshot (implemented in this CL).
2. Only handle stylus input in the stylus tool window and let all
   other types of events pass through to the underlying windows
   (to be implemented).
    
BUG=b:37640737
TEST=Invoke metalayer, check that the screenshot does not contain
     stylus strokes.

Review-Url: https://codereview.chromium.org/2896943002
Cr-Commit-Position: refs/heads/master@{#474580}
Committed:
https://chromium.googlesource.com/chromium/src/+/8e27059bf039fc6e5728c71ad1d4...
==========

[commit-bot: I haz the power, 2017-05-25 06:13:44]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/8e27059bf039fc6e5728c71ad1d4...