Commit 668ce807 authored by Yi Gu's avatar Yi Gu Committed by Commit Bot

[VizHitTesting] Ignore kHitTestAsk for root_view in V2

When building hit test data for v2, root_view could be overlapped by
ui::Layers on CrOS which it shouldn't. This leads to all hit testing
on CrOS with v2 hit test data becoming async.

This patch ignores the kHitTestAsk flag for root if it was caused by
being overlapped. The best way to fix this is not setting the flag
upon building the hit test data in LTHI. Will try to do that in a
follow up patch.

Bug: 999576, 996865
Change-Id: I84ad8fa6a732f52a9bc2f40149954562da5ef69a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787118
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693929}
parent d4be4c68
......@@ -177,7 +177,8 @@ Target HitTestQuery::FindTargetForLocationStartingFromImpl(
return Target();
FindTargetInRegionForLocation(event_source, location, start_index,
is_location_relative_to_parent, &target);
is_location_relative_to_parent, frame_sink_id,
&target);
UMA_HISTOGRAM_CUSTOM_MICROSECONDS_TIMES("Event.VizHitTest.TargetTimeUs",
target_timer.Elapsed(),
base::TimeDelta::FromMicroseconds(1),
......@@ -190,6 +191,7 @@ bool HitTestQuery::FindTargetInRegionForLocation(
const gfx::PointF& location,
size_t region_index,
bool is_location_relative_to_parent,
const FrameSinkId& root_view_frame_sink_id,
Target* target) const {
gfx::PointF location_transformed(location);
......@@ -226,13 +228,27 @@ bool HitTestQuery::FindTargetInRegionForLocation(
const uint32_t flags = hit_test_data_[region_index].flags;
DCHECK(hit_test_data_[region_index].frame_sink_id.is_valid());
// Root view should not be overlapped by others. However, the root view
// information is not visible to LTHI::BuildHitTestData(). Therefore the
// kOverlappedRegion flag could still be set for the root view upon building
// hit test data, e.g. overlapped by ShelfApp on ChromeOS.
// The kHitTestAsk flag should be ignored in such a case because there is no
// need to do async hit testing on the root merely because it was overlapped.
// TODO(yigu): Do not set the kHitTestAsk and kOverlappedRegion flags for
// root when building hit test data.
bool root_view_overlapped =
hit_test_data_[region_index].frame_sink_id == root_view_frame_sink_id &&
hit_test_data_[region_index].async_hit_test_reasons ==
AsyncHitTestReasons::kOverlappedRegion;
// Verify that async_hit_test_reasons is set if and only if there's
// a kHitTestAsk flag.
DCHECK_EQ(!!(flags & HitTestRegionFlags::kHitTestAsk),
!!hit_test_data_[region_index].async_hit_test_reasons);
if ((flags & HitTestRegionFlags::kHitTestAsk) &&
!(flags & HitTestRegionFlags::kHitTestIgnore)) {
!(flags & HitTestRegionFlags::kHitTestIgnore) && !root_view_overlapped) {
target->frame_sink_id = hit_test_data_[region_index].frame_sink_id;
target->location_in_target = location_in_target;
target->flags = flags;
......@@ -241,10 +257,13 @@ bool HitTestQuery::FindTargetInRegionForLocation(
return true;
}
// If the current region is not the root view, neither is its children.
// Therefore when recursively calling FindTargetInRegionForLocation, pass an
// invalid frame sink id as "root".
while (child_region < child_region_end) {
if (FindTargetInRegionForLocation(
event_source, location_in_target, child_region,
/*is_location_relative_to_parent=*/true, target)) {
/*is_location_relative_to_parent=*/true, FrameSinkId(), target)) {
return true;
}
......
......@@ -130,6 +130,7 @@ class VIZ_HOST_EXPORT HitTestQuery {
const gfx::PointF& location,
size_t region_index,
bool is_location_relative_to_parent,
const FrameSinkId& root_view_frame_sink_id,
Target* target) const;
// Transform |location_in_target| to be in |region_index|'s coordinate space.
......
......@@ -1004,7 +1004,7 @@ TEST_F(HitTestQueryTest, RootHitTestAskFlag) {
active_data_.push_back(AggregatedHitTestRegion(
e_id, HitTestRegionFlags::kHitTestAsk | HitTestRegionFlags::kHitTestMouse,
e_bounds, transform_e_to_e, 0,
AsyncHitTestReasons::kOverlappedRegion)); // e
AsyncHitTestReasons::kUseDrawQuadData)); // e
SendHitTestData();
// All points are in e's coordinate system when we reach this case.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment