Skip to content

Commit

Permalink
[Merge to M-103]capture_mode: Make select capture region work properly
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 7674929)

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-Original-Commit-Position: refs/heads/main@{#1002973}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648909
Commit-Queue: Connie Xu <conniekxu@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#18}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
conniekxu authored and Chromium LUCI CQ committed May 16, 2022
1 parent 0dfb733 commit 45e34cd
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 34 deletions.
38 changes: 12 additions & 26 deletions ash/capture_mode/capture_mode_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions ash/capture_mode/capture_mode_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
25 changes: 25 additions & 0 deletions ash/capture_mode/capture_mode_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 45e34cd

Please sign in to comment.