capture_mode: Make select capture region work properly

This CL fixed the issue that capture region is not selectable nor
draggable after start pressing on the capture bar or settings and then
release the press outsiede of it. To fix it, we should let the event
targeted on the capture bar or settings go through and to be handled
by it instead of setting the event as handled and stopping propagation.

Test: Manual, added a regression test
Change-Id: I8f5fbbd30d95f0fd520a67a32fcac5c9a04f9869
Fixed: 1325028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3645696
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002973}

ash/capture_mode/capture_mode_session.cc

@@ -1966,29 +1966,6 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
   }
 
   DCHECK(is_capture_region);
-  // Allow events that are located on the capture mode bar or settings menu to
-  // pass through so we can click the buttons.
-  if (!is_event_on_capture_bar &&
-      !(capture_mode_settings_widget_ &&
-        capture_mode_settings_widget_->GetWindowBoundsInScreen().Contains(
-            screen_location))) {
-    if (capture_mode_settings_widget_ &&
-        located_press_event_on_settings_menu_) {
-      capture_mode_settings_widget_->GetNativeWindow()->delegate()->OnEvent(
-          event);
-    }
-    event->SetHandled();
-    event->StopPropagation();
-  }
-
-  // OnLocatedEventPressed() and OnLocatedEventDragged used root locations since
-  // CaptureModeController::user_capture_region() is stored in root
-  // coordinates..
-  // TODO(sammiequon): Update CaptureModeController::user_capture_region() to
-  // store screen coordinates.
-  gfx::Point location_in_root = event->location();
-  aura::Window::ConvertPointToTarget(event_target, current_root_,
-                                     &location_in_root);
 
   // Early return if the given event is targeted on the capture bar or settings
   // menu, since the switch block below is for updating the user capture region
@@ -2003,11 +1980,21 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
     return;
   }
 
+  event->SetHandled();
+  event->StopPropagation();
+
+  // OnLocatedEventPressed() and OnLocatedEventDragged used root locations since
+  // CaptureModeController::user_capture_region() is stored in root
+  // coordinates..
+  // TODO(sammiequon): Update CaptureModeController::user_capture_region() to
+  // store screen coordinates.
+  gfx::Point location_in_root = event->location();
+  aura::Window::ConvertPointToTarget(event_target, current_root_,
+                                     &location_in_root);
+
   switch (event->type()) {
     case ui::ET_MOUSE_PRESSED:
     case ui::ET_TOUCH_PRESSED:
-      if (is_event_on_settings_menu)
-        located_press_event_on_settings_menu_ = true;
       old_mouse_warp_status_ = SetMouseWarpEnabled(false);
       OnLocatedEventPressed(location_in_root, is_touch);
       break;
@@ -2022,7 +2009,6 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
         SetMouseWarpEnabled(*old_mouse_warp_status_);
       old_mouse_warp_status_.reset();
       OnLocatedEventReleased(location_in_root);
-      located_press_event_on_settings_menu_ = false;
       break;
     default:
       break;

ash/capture_mode/capture_mode_session.h

@@ -538,14 +538,6 @@ class ASH_EXPORT CaptureModeSession
   // The object which handles tab focus while in a capture session.
   std::unique_ptr<CaptureModeSessionFocusCycler> focus_cycler_;
 
-  // This helps indicating whether located events should be handled by the
-  // capture mode settings menu view or the capture mode Pre-EventHandler. When
-  // it's true, settings menu view should handle the event. Set it to true when
-  // the event is a press event and is located on the settings menu view. Set it
-  // to false when the event is a release event and "event_on_settings_menu_" is
-  // true.
-  bool located_press_event_on_settings_menu_ = false;
-
   // True if a located event should be passed to camera preview to be handled.
   bool should_pass_located_event_to_camera_preview_ = false;
 

ash/capture_mode/capture_mode_unittests.cc

@@ -1064,6 +1064,31 @@ TEST_F(CaptureModeTest, CaptureRegionCaptureButtonDoesNotIntersectCaptureBar) {
   EXPECT_GT(GetCaptureModeLabelWidget()->GetWindowBoundsInScreen().x(), 20);
 }
 
+// Tests that pressing on the capture bar and releasing the press outside of the
+// capture bar, the capture region could still be draggable and set. Regression
+// test for https://crbug.com/1325028.
+TEST_F(CaptureModeTest, SetCaptureRegionAfterPressOnCaptureBar) {
+  UpdateDisplay("800x600");
+
+  auto* controller =
+      StartCaptureSession(CaptureModeSource::kRegion, CaptureModeType::kVideo);
+  auto* settings_button = GetSettingsButton();
+
+  // Press on the settings button without release.
+  auto* event_generator = GetEventGenerator();
+  event_generator->MoveMouseTo(
+      settings_button->GetBoundsInScreen().CenterPoint());
+  event_generator->PressLeftButton();
+  // Move mouse to the outside of the capture bar and then release the press.
+  event_generator->MoveMouseTo({300, 300});
+  event_generator->ReleaseLeftButton();
+
+  // Set the capture region, and verify it's set successfully.
+  const gfx::Rect region_bounds(100, 100, 200, 200);
+  SelectRegion(region_bounds);
+  EXPECT_EQ(controller->user_capture_region(), region_bounds);
+}
+
 TEST_F(CaptureModeTest, WindowCapture) {
   // Create 2 windows that overlap with each other.
   const gfx::Rect bounds1(0, 0, 200, 200);