Implement HitTestAggregator

services/viz/hit_test/public/interfaces/hit_test_data.mojom

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module viz.hit_test.mojom;
+
+import "cc/ipc/surface_id.mojom";
+import "ui/gfx/geometry/mojo/geometry.mojom";
+import "ui/gfx/mojo/transform.mojom";
+
+// Region maps to this surface ( me ).
+const uint32 kHitTestMine = 0x01;
+
+// Region ignored for hit testing ( transparent backgrounds & hover:none ).
+const uint32 kHitTestIgnore = 0x02;
+
+// Region maps to child surface ( OOPIF ).
+const uint32 kHitTestChildSurface = 0x04;
+
+// Irregular boundary - send HitTestRequest to resolve.
+const uint32 kHitTestAsk = 0x08;
+
+// No touch event handler exists - no need to send touch events.
+const uint32 kHitTestNoTouchEventHandler = 0x10;

[sadrul, 2017/06/27 04:23:35]

As mentioned in https://codereview.chromium.org/2908783002/#msg62, instead of
saying kHitTestNoTouchEventHandler, this should be kHitTestTouchEventHandler. We
should also have a kHitTestMouseEventHandler.

For aura windows, the window borders can have different hit-test regions for
mouse vs. for touch. This means aura will send some HitTest* which should be
used only for mouse events, and some HitTest* which should be used only for
touch events. It would be simplest to have explicit kHitTestForTouch,
kHitTestForMouse (and potentially kHitTestForStylus etc. in the future if so
desired) flags.

[gklassen, 2017/06/27 21:46:33]

Hmmm... I reviewed this implementation in detail, but I had thought the
intention was to indicate regions that did not have a touch event handler and
that this was the exception.  I agree that we will have to review details for
exactly what flags are required to handle this well.  Ok to revisit after we
have this working? 

+
+struct HitTestRegion {
+

[sadrul, 2017/06/27 04:23:35]

no blank line.

[gklassen, 2017/06/27 21:46:33]

Done.

+  // Flags to indicate the type of HitTestRegion.
+  uint32 flags;
+
+  // SurfaceId is required when flags = kHitTestChildSurface.
+  cc.mojom.SurfaceId surface_id;
+
+  // The rect of the region in the coordinate space of the embedder.
+  gfx.mojom.Rect rect;
+
+  // The transform applied to the child region.

[sadrul, 2017/06/27 04:23:35]

What is 'child region'?
(https://codereview.chromium.org/2908783002/diff/480001/components/viz/hit_tes...)

[gklassen, 2017/06/27 21:46:33]

Done.

+  gfx.mojom.Transform transform;
+};
+
+struct HitTestData {
+

[sadrul, 2017/06/27 04:23:35]

ditto

[gklassen, 2017/06/27 21:46:33]

Done.

+  // SurfaceId corresponding to this HitTestData.
+  cc.mojom.SurfaceId surface_id;
+
+  // Flags indicate how to handle events that match no sub-regions.
+  // kHitTestMine routes un-matched events to this surface ( opaque ).
+  // kHitTestIgnore keeps previous match in the parent ( transparent ).
+  uint32 flags;
+
+  // The bounds of the surface.
+  gfx.mojom.Rect bounds;
+
+  // The transform that applies to all regions in this surface.
+  gfx.mojom.Transform transform;
+
+  // The list of sub-regions in front to back order.
+  // TODO(gklassen): invert order if it's easier to write them back to front.
+  array<HitTestRegion> regions;

[sadrul, 2017/06/27 04:23:35]

As mentioned in Ria's CL, I think I would go with HitTestData and
HitTestDataList (or HitTestRegion and HitTestRegionList). Having both
HitTestData and HitTestRegion is confusing, because neither is clearly a list of
the other thing.

[gklassen, 2017/06/27 21:46:33]

Agreed that neither is clearly a list of the other thing - but perhaps that's
ok?  Currently HitTestData includes all the hit-test information required for
any surface and that includes a list of regions - that *seems* to match the
structure has defined quite well - is there a better way to capture that?  

[sadrul, 2017/06/28 05:27:40]

I think the distinction between the two that matters most, when reading the
code, is which one includes all data (or I guess the 'root data' or the
'toplevel data'), vs. which one includes data about one specific
target+event-source. I am OK with any pair of names that makes that readily
obvious. I don't think (HitTestRegion, HitTestData) pair meets that bar. In
several offline discussions, I myself have been confused between the two, which
to me means more folks would be similarly confused.

[gklassen, 2017/06/28 16:29:59]

Done.

+};