Remove extra tests for SurfaceHitTest path.

Viz hit testing launched on all platforms as of M74. Delete browser and
unit tests that cover the SurfaceHitTest path. Most of the parameterized
tests were no longer testing the intended code anyways.
features::IsVizDisplayCompositorEnabled() always returns true on some
platforms which forces on viz hit testing regardless of
features::kEnableVizHitTest status.

Bug: 810128
Change-Id: Ie10957a7cfbc58b763bef085759a6fc950d0b5e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638283Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665914}

chrome/browser/pdf/pdf_extension_test.cc

@@ -408,25 +408,7 @@ class PDFExtensionLoadTest : public PDFExtensionTest,
   PDFExtensionLoadTest() {}
 };
 
-class PDFExtensionHitTestTest : public PDFExtensionTest,
-                                public testing::WithParamInterface<bool> {
- public:
-  PDFExtensionHitTestTest() {}
-
- protected:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    PDFExtensionTest::SetUpCommandLine(command_line);
-    if (GetParam()) {
-      std::map<std::string, std::string> parameters{{"provider", "draw_quad"}};
-      feature_list_.InitAndEnableFeatureWithParameters(
-          features::kEnableVizHitTest, parameters);
-    } else {
-      feature_list_.InitAndDisableFeature(features::kEnableVizHitTest);
-    }
-  }
-
-  base::test::ScopedFeatureList feature_list_;
-};
+using PDFExtensionHitTestTest = PDFExtensionTest;
 
 class PDFAnnotationsTest : public PDFExtensionTest {
  public:
@@ -630,10 +612,6 @@ INSTANTIATE_TEST_SUITE_P(PDFTestFiles,
                          PDFExtensionLoadTest,
                          testing::Range(0, kNumberLoadTestParts));
 
-INSTANTIATE_TEST_SUITE_P(/* no prefix */,
-                         PDFExtensionHitTestTest,
-                         testing::Bool());
-
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Basic) {
   RunTestsInFile("basic_test.js", "test.pdf");
 
@@ -1972,7 +1950,7 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
 #endif  // !defined(OS_MACOSX)
 
 // Flaky in nearly all configurations; see https://crbug.com/856169.
-IN_PROC_BROWSER_TEST_P(PDFExtensionHitTestTest, DISABLED_MouseLeave) {
+IN_PROC_BROWSER_TEST_F(PDFExtensionHitTestTest, DISABLED_MouseLeave) {
   GURL url = embedded_test_server()->GetURL("/pdf/pdf_embed.html");
 
   // Load page with embedded PDF and make sure it succeeds.
@@ -2023,7 +2001,7 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionHitTestTest, DISABLED_MouseLeave) {
   EXPECT_EQ(1, enter_count);
 }
 
-IN_PROC_BROWSER_TEST_P(PDFExtensionHitTestTest, ContextMenuCoordinates) {
+IN_PROC_BROWSER_TEST_F(PDFExtensionHitTestTest, ContextMenuCoordinates) {
   GURL url = embedded_test_server()->GetURL("/pdf/pdf_embed.html");
 
   // Load page with embedded PDF and make sure it succeeds.

content/browser/renderer_host/input/touch_input_browsertest.cc

@@ -10,7 +10,6 @@
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-#include "components/viz/common/features.h"
 #include "content/browser/gpu/compositor_util.h"
 #include "content/browser/renderer_host/render_widget_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -126,10 +125,8 @@ class TouchInputBrowserTest : public ContentBrowserTest {
     frame_observer.WaitForAnyFrameSubmission();
 #endif
 
-    if (features::IsVizHitTestingEnabled()) {
-      HitTestRegionObserver observer(host->GetFrameSinkId());
-      observer.WaitForHitTestData();
-    }
+    HitTestRegionObserver observer(host->GetFrameSinkId());
+    observer.WaitForHitTestData();
   }
 
   void SetUpCommandLine(base::CommandLine* cmd) override {

content/browser/renderer_host/render_widget_host_input_event_router_unittest.cc

@@ -7,7 +7,6 @@
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
 #include "build/build_config.h"
-#include "components/viz/common/features.h"
 #include "components/viz/host/hit_test/hit_test_query.h"
 #include "components/viz/host/host_frame_sink_manager.h"
 #include "components/viz/test/host_frame_sink_manager_test_api.h"
@@ -170,16 +169,14 @@ class MockRootRenderWidgetHostView : public TestRenderWidgetHostView {
                         bool query_renderer) {
     current_hittest_result_ = result_view;
     force_query_renderer_on_hit_test_ = query_renderer;
-    if (features::IsVizHitTestingEnabled()) {
-      DCHECK(GetHostFrameSinkManager());
+    DCHECK(GetHostFrameSinkManager());
 
-      viz::HostFrameSinkManager::DisplayHitTestQueryMap hit_test_map;
-      hit_test_map[GetFrameSinkId()] =
-          std::make_unique<StubHitTestQuery>(result_view, query_renderer);
+    viz::HostFrameSinkManager::DisplayHitTestQueryMap hit_test_map;
+    hit_test_map[GetFrameSinkId()] =
+        std::make_unique<StubHitTestQuery>(result_view, query_renderer);
 
-      viz::HostFrameSinkManagerTestApi(GetHostFrameSinkManager())
-          .SetDisplayHitTestQuery(std::move(hit_test_map));
-    }
+    viz::HostFrameSinkManagerTestApi(GetHostFrameSinkManager())
+        .SetDisplayHitTestQuery(std::move(hit_test_map));
   }
 
   void Reset() { last_gesture_seen_ = blink::WebInputEvent::kUndefined; }

content/browser/site_per_process_hit_test_browsertest.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/site_per_process_browsertest.h"
-
 #include <tuple>
 
 #include "base/bind.h"
@@ -26,6 +24,7 @@
 #include "content/browser/renderer_host/input/touch_emulator.h"
 #include "content/browser/renderer_host/render_widget_host_input_event_router.h"
 #include "content/browser/renderer_host/render_widget_host_view_child_frame.h"
+#include "content/browser/site_per_process_browsertest.h"
 #include "content/common/frame_messages.h"
 #include "content/common/input/input_handler.mojom-test-utils.h"
 #include "content/common/view_messages.h"
@@ -689,10 +688,15 @@ class SystemEventRewriter : public ui::EventRewriter {
 };
 #endif
 
+enum class HitTestType {
+  kDrawQuad,
+  kSurfaceLayer,
+};
+
 }  // namespace
 
 class SitePerProcessHitTestBrowserTest
-    : public testing::WithParamInterface<std::tuple<int, float>>,
+    : public testing::WithParamInterface<std::tuple<HitTestType, float>>,
       public SitePerProcessBrowserTest {
  public:
   SitePerProcessHitTestBrowserTest() {}
@@ -716,20 +720,13 @@ class SitePerProcessHitTestBrowserTest
     ui::PlatformEventSource::SetIgnoreNativePlatformEvents(true);
     const char kParam[] = "provider";
     std::map<std::string, std::string> parameters;
-    if (std::get<0>(GetParam()) == 1) {
+    if (std::get<0>(GetParam()) == HitTestType::kDrawQuad) {
       parameters[kParam] = "draw_quad";
-      feature_list_.InitAndEnableFeatureWithParameters(
-          features::kEnableVizHitTest, parameters);
-    } else if (std::get<0>(GetParam()) == 2) {
+    } else if (std::get<0>(GetParam()) == HitTestType::kSurfaceLayer) {
       parameters[kParam] = "surface_layer";
-      feature_list_.InitAndEnableFeatureWithParameters(
-          features::kEnableVizHitTest, parameters);
-    } else {
-      feature_list_.InitWithFeatures(
-          {},
-          {features::kVizDisplayCompositor, features::kEnableVizHitTestDrawQuad,
-           features::kEnableVizHitTestSurfaceLayer});
     }
+    feature_list_.InitAndEnableFeatureWithParameters(
+        features::kEnableVizHitTest, parameters);
   }
 
   base::test::ScopedFeatureList feature_list_;
@@ -1435,10 +1432,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
                        CSSTransformedIframeTouchEventCoordinates) {
-  // This test only makes sense if viz hit testing is enabled.
-  if (std::get<0>(GetParam()) == 0)
-    return;
-
   GURL url(embedded_test_server()->GetURL(
       "/frame_tree/page_with_positioned_scaled_frame.html"));
   ASSERT_TRUE(NavigateToURL(shell(), url));
@@ -2795,12 +2788,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
               kHitTestTolerance);
   EXPECT_FALSE(child_frame_monitor.EventWasReceived());
 
-  // Surface hit test can only learn about pointer-events changes when
-  // submitting compositing frame, so we disable the second half of the test for
-  // surface hit test.
-  if (!features::IsVizHitTestingEnabled())
-    return;
-
   // Remove pointer-events: none property from iframe, also remove child2 to
   // properly notify the observer the update.
   // Wait for the confirmation of the deletion so that surface hit test is aware
@@ -5035,9 +5022,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
 // hit test data without crashing.
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
                        TouchpadPinchWhenMissingHitTestDataDoesNotCrash) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
-
   GURL main_url(embedded_test_server()->GetURL(
       "a.com", "/frame_tree/page_with_positioned_frame.html"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
@@ -6286,8 +6270,6 @@ const uint32_t
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        TransformedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data =
       SetupAndGetHitTestData("/frame_tree/page_with_transformed_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6312,8 +6294,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        ClippedOOPIFFastPath) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data =
       SetupAndGetHitTestData("/frame_tree/page_with_clipped_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6353,8 +6333,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        RotatedClippedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data = SetupAndGetHitTestData(
       "/frame_tree/page_with_rotated_clipped_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6394,8 +6372,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        ClippedRotatedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data = SetupAndGetHitTestData(
       "/frame_tree/page_with_clipped_rotated_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6457,8 +6433,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        ClipPathOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data =
       SetupAndGetHitTestData("/frame_tree/page_with_clip_path_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6492,8 +6466,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        OverlappedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data =
       SetupAndGetHitTestData("/frame_tree/page_with_overlapped_iframes.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6522,8 +6494,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        MaskedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data =
       SetupAndGetHitTestData("/frame_tree/page_with_masked_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6542,8 +6512,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        AncestorMaskedOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data = SetupAndGetHitTestData(
       "/frame_tree/page_with_ancestor_masked_iframe.html");
   float device_scale_factor = current_device_scale_factor();
@@ -6568,8 +6536,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
                        PointerEventsNoneOOPIF) {
-  if (!features::IsVizHitTestingEnabled())
-    return;
   auto hit_test_data = SetupAndGetHitTestData(
       "/frame_tree/page_with_positioned_frame_pointer-events_none.html", 0);
   float device_scale_factor = current_device_scale_factor();
@@ -6690,7 +6656,8 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestDataGenerationBrowserTest,
 // All tests are flaky on MSAN. https://crbug.com/959924
 #if !defined(MEMORY_SANITIZER)
 
-static const int kHitTestOption[] = {0, 1, 2};
+static const HitTestType kHitTestOption[] = {HitTestType::kDrawQuad,
+                                             HitTestType::kSurfaceLayer};
 static const float kOneScale[] = {1.f};
 
 INSTANTIATE_TEST_SUITE_P(/* no prefix */,

content/public/test/hit_test_region_observer.cc

@@ -7,12 +7,8 @@
 #include <algorithm>
 
 #include "base/test/test_timeouts.h"
-#include "components/viz/common/features.h"
 #include "components/viz/host/hit_test/hit_test_query.h"
 #include "components/viz/host/host_frame_sink_manager.h"
-#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
-#include "components/viz/service/surfaces/surface.h"
-#include "components/viz/service/surfaces/surface_manager.h"
 #include "content/browser/compositor/surface_utils.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
@@ -28,103 +24,6 @@
 namespace content {
 
 namespace {
-class SurfaceHitTestReadyNotifier {
- public:
-  explicit SurfaceHitTestReadyNotifier(RenderWidgetHostViewBase* target_view);
-  ~SurfaceHitTestReadyNotifier() {}
-
-  void WaitForSurfaceReady(RenderWidgetHostViewBase* root_container);
-
- private:
-  bool ContainsSurfaceId(const viz::SurfaceId& container_surface_id);
-
-  viz::SurfaceManager* surface_manager_;
-  RenderWidgetHostViewBase* target_view_;
-
-  DISALLOW_COPY_AND_ASSIGN(SurfaceHitTestReadyNotifier);
-};
-
-SurfaceHitTestReadyNotifier::SurfaceHitTestReadyNotifier(
-    RenderWidgetHostViewBase* target_view)
-    : target_view_(target_view) {
-  surface_manager_ = GetFrameSinkManager()->surface_manager();
-}
-
-void SurfaceHitTestReadyNotifier::WaitForSurfaceReady(
-    RenderWidgetHostViewBase* root_view) {
-  while (!ContainsSurfaceId(root_view->GetCurrentSurfaceId())) {
-    // TODO(kenrb): Need a better way to do this. Needs investigation on
-    // whether we can add a callback through RenderWidgetHostViewBaseObserver
-    // from OnSwapCompositorFrame and avoid this busy waiting. A callback on
-    // every compositor frame might be generally undesirable for performance,
-    // however.
-    base::RunLoop run_loop;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
-    run_loop.Run();
-  }
-}
-
-bool SurfaceHitTestReadyNotifier::ContainsSurfaceId(
-    const viz::SurfaceId& container_surface_id) {
-  if (!container_surface_id.is_valid())
-    return false;
-
-  viz::Surface* container_surface =
-      surface_manager_->GetSurfaceForId(container_surface_id);
-  if (!container_surface ||
-      container_surface->active_referenced_surfaces().empty()) {
-    return false;
-  }
-
-  for (const viz::SurfaceId& id :
-       container_surface->active_referenced_surfaces()) {
-    if (id == target_view_->GetCurrentSurfaceId() || ContainsSurfaceId(id))
-      return true;
-  }
-  return false;
-}
-
-// Waits until the cc::Surface associated with a guest/cross-process-iframe
-// has been drawn for the first time. Once this method returns it should be
-// safe to assume that events sent to the top-level RenderWidgetHostView can
-// be expected to properly hit-test to this surface, if appropriate.
-void WaitForGuestSurfaceReady(content::WebContents* guest_web_contents) {
-  RenderWidgetHostViewChildFrame* child_view =
-      static_cast<RenderWidgetHostViewChildFrame*>(
-          guest_web_contents->GetRenderWidgetHostView());
-
-  RenderWidgetHostViewBase* root_view = static_cast<RenderWidgetHostViewBase*>(
-      static_cast<content::WebContentsImpl*>(guest_web_contents)
-          ->GetOuterWebContents()
-          ->GetRenderWidgetHostView());
-
-  SurfaceHitTestReadyNotifier notifier(child_view);
-  notifier.WaitForSurfaceReady(root_view);
-}
-
-// To wait for frame submission see RenderFrameSubmissionObserver.
-// Waits until the cc::Surface associated with a cross-process child frame
-// has been drawn for the first time. Once this method returns it should be
-// safe to assume that events sent to the top-level RenderWidgetHostView can
-// be expected to properly hit-test to this surface, if appropriate.
-void WaitForChildFrameSurfaceReady(content::RenderFrameHost* child_frame) {
-  RenderWidgetHostViewBase* child_view =
-      static_cast<RenderFrameHostImpl*>(child_frame)
-          ->GetRenderWidgetHost()
-          ->GetView();
-  if (!child_view || !child_view->IsRenderWidgetHostViewChildFrame())
-    return;
-
-  RenderWidgetHostViewBase* root_view =
-      static_cast<CrossProcessFrameConnector*>(
-          static_cast<RenderWidgetHostViewChildFrame*>(child_view)
-              ->FrameConnectorForTesting())
-          ->GetRootRenderWidgetHostViewForTesting();
-
-  SurfaceHitTestReadyNotifier notifier(child_view);
-  notifier.WaitForSurfaceReady(root_view);
-}
 
 // Returns a transform from root to |frame_sink_id|. If no HitTestQuery contains
 // |frame_sink_id| then this will return an empty optional.
@@ -149,13 +48,8 @@ void WaitForHitTestDataOrChildSurfaceReady(RenderFrameHost* child_frame) {
           ->GetRenderWidgetHost()
           ->GetView();
 
-  if (features::IsVizHitTestingEnabled()) {
-    HitTestRegionObserver observer(child_view->GetFrameSinkId());
-    observer.WaitForHitTestData();
-    return;
-  }
-
-  WaitForChildFrameSurfaceReady(child_frame);
+  HitTestRegionObserver observer(child_view->GetFrameSinkId());
+  observer.WaitForHitTestData();
 }
 
 void WaitForHitTestDataOrGuestSurfaceReady(WebContents* guest_web_contents) {
@@ -166,13 +60,8 @@ void WaitForHitTestDataOrGuestSurfaceReady(WebContents* guest_web_contents) {
       static_cast<RenderWidgetHostViewChildFrame*>(
           guest_web_contents->GetRenderWidgetHostView());
 
-  if (features::IsVizHitTestingEnabled()) {
-    HitTestRegionObserver observer(child_view->GetFrameSinkId());
-    observer.WaitForHitTestData();
-    return;
-  }
-
-  WaitForGuestSurfaceReady(guest_web_contents);
+  HitTestRegionObserver observer(child_view->GetFrameSinkId());
+  observer.WaitForHitTestData();
 }
 
 HitTestRegionObserver::HitTestRegionObserver(
@@ -235,10 +124,6 @@ HitTestTransformChangeObserver::~HitTestTransformChangeObserver() = default;
 void HitTestTransformChangeObserver::WaitForHitTestDataChange() {
   DCHECK(!run_loop_);
 
-  // TODO(kylechar): Remove when VizHitTesting is enabled everywhere.
-  if (!features::IsVizHitTestingEnabled())
-    return;
-
   // If the transform has already changed then don't run RunLoop.
   base::Optional<gfx::Transform> transform =
       GetRootToTargetTransform(target_frame_sink_id_);

content/public/test/hit_test_region_observer.h

@@ -21,9 +21,8 @@ class WebContents;
 // TODO(jonross): Remove these once Viz Hit Testing is on by default and the
 // legacy content::browser_test_utils fallbacks are no longer needed.
 //
-// When Viz Hit Testing is available, waits until hit test data for
-// |child_frame| has been submitted, see WaitForHitTestData. Otherwise waits
-// until the cc::Surface associated with |child_frame| has been activated.
+// Waits until hit test data for |child_frame| has been submitted, see
+// WaitForHitTestData.
 void WaitForHitTestDataOrChildSurfaceReady(RenderFrameHost* child_frame);
 void WaitForHitTestDataOrGuestSurfaceReady(WebContents* guest_web_contents);