VizHitTest: Do not skip OOPIF even if it's occluded

We previously employed an optimization that skips OOPIFs occluded by
parent frame's layers. This optimization is problematic as the layer's
bounds do not necessarily represent its hit test region, and it works
incorrectly when the occluding layers have pointer-events property.

This CL removes that optimization.

Bug: 846798, 896786
Change-Id: Id33c7d658cfd677ab3eb496371a213de2423d349
Reviewed-on: https://chromium-review.googlesource.com/c/1296821Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604329}

cc/trees/layer_tree_host_impl.cc

@@ -2654,9 +2654,6 @@ base::Optional<viz::HitTestRegionList> LayerTreeHostImpl::BuildHitTestData() {
       gfx::Rect layer_screen_space_rect = MathUtil::MapEnclosingClippedRect(
           surface_layer->ScreenSpaceTransform(),
           gfx::Rect(surface_layer->bounds()));
-      if (overlapping_region.Contains(layer_screen_space_rect))
-        continue;
-
       auto flag = GetFlagsForSurfaceLayer(surface_layer);
       if (overlapping_region.Intersects(layer_screen_space_rect))
         flag |= viz::HitTestRegionFlags::kHitTestAsk;

content/browser/site_per_process_hit_test_browsertest.cc

@@ -722,8 +722,8 @@ class SitePerProcessHitTestBrowserTest
     if (std::get<0>(GetParam()) == 1) {
       feature_list_.InitAndEnableFeature(features::kEnableVizHitTestDrawQuad);
     } else if (std::get<0>(GetParam()) == 2) {
-      feature_list_.InitAndEnableFeature(
-          features::kEnableVizHitTestSurfaceLayer);
+      feature_list_.InitWithFeatures({features::kEnableVizHitTestSurfaceLayer},
+                                     {features::kEnableVizHitTestDrawQuad});
     } else {
       feature_list_.InitWithFeatures({}, {features::kEnableVizHitTestDrawQuad,
                                           features::kVizDisplayCompositor});
@@ -2157,13 +2157,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHighDPIHitTestBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
                        HitTestStaleDataDeletedView) {
-  // TODO(sunxd): Hit test regions are not submitted for overlapping surfaces,
-  // causing /2 to fail outside of Viz. https::/crbug.com/846798
-  if (base::FeatureList::IsEnabled(features::kEnableVizHitTestSurfaceLayer) &&
-      !base::FeatureList::IsEnabled(features::kVizDisplayCompositor)) {
-    return;
-  }
-
   // Have two iframes to avoid going to short circuit path during the second
   // targeting.
   GURL main_url(
@@ -2231,7 +2224,14 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
   auto result = web_contents->GetInputEventRouter()->FindTargetSynchronously(
       rwhv_parent, down_event);
   EXPECT_EQ(result.view, rwhv_parent);
-  EXPECT_TRUE(result.should_query_view);
+  // When VizHitTestSurfaceLayer is enabled and there is only one child frame,
+  // we can find the target frame and are sure there are no other possible
+  // targets, in this case, we dispatch the event immediately without
+  // asynchronously querying the root-view.
+  if (features::IsVizHitTestingSurfaceLayerEnabled())
+    EXPECT_FALSE(result.should_query_view);
+  else
+    EXPECT_TRUE(result.should_query_view);
   EXPECT_EQ(result.target_location.value(), parent_location);
 }
 
@@ -5655,6 +5655,32 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
   EXPECT_EQ(kFastHitTestFlags, hit_test_data[3].flags);
 }
 
+IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
+                       OccludedOOPIF) {
+  if (!features::IsVizHitTestingSurfaceLayerEnabled())
+    return;
+
+  auto hit_test_data =
+      SetupAndGetHitTestData("/frame_tree/page_with_occluded_iframes.html");
+  float device_scale_factor = current_device_scale_factor();
+  gfx::Transform expected_transform1;
+  gfx::Transform expected_transform2;
+  expected_transform2.Translate(-110 * device_scale_factor, 0);
+
+  // We should not skip OOPIFs that are occluded by parent frame elements, since
+  // in cc an element's bound may not be its hit test area.
+  DCHECK(hit_test_data.size() == 4);
+  EXPECT_TRUE(ApproximatelyEqual(
+      TransformRectToQuadF(gfx::Rect(100, 100), expected_transform1),
+      TransformRectToQuadF(hit_test_data[3])));
+  EXPECT_EQ(kSlowHitTestFlags, hit_test_data[3].flags);
+
+  EXPECT_TRUE(ApproximatelyEqual(
+      TransformRectToQuadF(gfx::Rect(100, 100), expected_transform2),
+      TransformRectToQuadF(hit_test_data[2])));
+  EXPECT_EQ(kSlowHitTestFlags, hit_test_data[2].flags);
+}
+
 static const int kHitTestOption[] = {0, 1, 2};
 static const float kOneScale[] = {1.f};
 

content/test/data/frame_tree/page_with_occluded_iframes.html

+
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <style>
+    iframe {
+      position: absolute;
+      top: 0px;
+      left: 0px;
+      width: 100px;
+      height: 100px;
+      border: none;
+      z-index: 0;
+    }
+    .on-top-of-iframe {
+      position: absolute;
+      width: 100px;
+      height: 100px;
+      border: none;
+      z-index: 100;
+    }
+    </style>
+  </head>
+  <body>
+    <iframe src="/cross-site/baz.com/title1.html"></iframe>
+    <div class="on-top-of-iframe" style="left: 0px; top: 0px; pointer-events: none; background: blue;">Should not hit test.</div>
+
+    <iframe src="/cross-site/bar.com/title1.html" style="left: 110px;"></iframe>
+    <div class="on-top-of-iframe" style="left: 110px; top: 0px;">
+      <div style="height: 3px; width: 1000px; background: blue;"></div>
+      <div style="height: 1000px; width: 3px; background: blue;"></div>
+    </div>
+  </body>
+</html>