Add viz-host HitTestQuery.

components/viz/host/hit_test/hit_test_query.cc

+// 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.
+
+#include "components/viz/host/hit_test/hit_test_query.h"
+
+#include "services/viz/hit_test/public/interfaces/hit_test_region_list.mojom.h"
+
+namespace viz {
+
+HitTestQuery::HitTestQuery() = default;
+
+HitTestQuery::~HitTestQuery() = default;
+
+Target HitTestQuery::FindTargetForLocation(const gfx::Point& location_in_root) {
+  Target target;
+  if (!aggregated_hit_test_region_list_size_)
+    return target;
+
+  FindTargetInRegionForLocation(location_in_root,
+                                aggregated_hit_test_region_list_, &target);
+  return target;

[rjkroege, 2017/07/18 21:38:49]

I am generally opposed to return-by-value here. This is fine for now but there
is an opportunity for optimization here.

+}
+
+bool HitTestQuery::FindTargetInRegionForLocation(
+    const gfx::Point& location_in_parent,
+    AggregatedHitTestRegion* region,
+    Target* target) const {
+  gfx::Point location_transformed(location_in_parent);
+  region->transform.TransformPoint(&location_transformed);

[rjkroege, 2017/07/18 21:38:49]

region data cannot be trusted. Please file a bug for this and add an appropriate
TODO. 

[sadrul, 2017/07/19 15:40:08]

The TODO can probably go in the SetAggregatedHitTestRegionList(list, size)
function, because that's where we would ideally do validation as required.

[sadrul, 2017/07/19 15:40:08]

Can you explain why you are doing the transform here? |location_in_parent|
should already be appropriately transformed to be in the parent's coordinate
space. Why would you need to transform it again?

[riajiang, 2017/07/19 17:45:55]

Done. http://crbug.com/746470

[riajiang, 2017/07/19 17:45:55]

Because we need to transform from parent's coordinate space to this child
|region|'s coordinate space? |region| can have a rotation etc.?

[riajiang, 2017/07/19 19:22:14]

As discussed offline, transformation needed here.

+  if (!region->rect.Contains(location_transformed))
+    return false;
+
+  AggregatedHitTestRegion* child_region = region + 1;
+  AggregatedHitTestRegion* child_region_end = region + region->child_count;
+  if (child_region_end > (aggregated_hit_test_region_list_ +

[sadrul, 2017/07/19 15:40:08]

Should be >= here, right?

[riajiang, 2017/07/19 17:45:55]

This is > now since I changed |child_region_end| based on the comment below?

+                          aggregated_hit_test_region_list_size_)) {
+    return false;
+  }
+
+  gfx::Point location_in_target(location_transformed);
+  location_in_target.Offset(-region->rect.x(), -region->rect.y());
+  while (child_region <= child_region_end) {

[sadrul, 2017/07/19 15:40:08]

Should be <

For the regular iterators, we exclude the 'end'. We should be doing the same
here. (if we need to change how |child_count| is set for that, then we should do
that)

[riajiang, 2017/07/19 17:45:55]

Okay I see. Changed |child_region_end| to be |child_region| + |child_count| so
this is < now. WDYT?

+    if (FindTargetInRegionForLocation(location_in_target, child_region,
+                                      target)) {
+      return true;
+    }
+
+    child_region = child_region + child_region->child_count + 1;

[rjkroege, 2017/07/18 21:38:49]

if any child_count > child_region_count, you need to consider viz as hostile and
report a security problem.

[riajiang, 2017/07/19 17:45:55]

Added in the bug.

+  }
+
+  if (region->flags & mojom::kHitTestMine) {
+    target->frame_sink_id = region->frame_sink_id;

[rjkroege, 2017/07/18 21:38:49]

the viz/host has a window <-> framesink map. there needs to be an injected
validator that the reported frame_sink_id is in this map.

[riajiang, 2017/07/19 17:45:55]

I didn't find it in viz/host - is it existing code or will be in viz/host? I
thought this map was in mus-ws?

Added in the bug.

[rjkroege, 2017/07/19 18:01:25]

so: viz-host is code in the mus-ws process. we need the map as part of both
chrome and ws so makes sense to keep in viz-host code.

[riajiang, 2017/07/19 19:22:14]

I see, will do when adding the map.

+    target->location_in_target = location_in_target;
+    target->flags = region->flags;

[rjkroege, 2017/07/18 21:38:49]

invalid flags should be reported as a security fault.

[riajiang, 2017/07/19 17:45:55]

Added in the bug.

+    return true;
+  }
+  return false;
+}
+
+}  // namespace viz