Commit f0d3c74e authored by Sammie Quon's avatar Sammie Quon Committed by Chromium LUCI CQ

capture_mode: Fix crash with clicking capture label in window mode.

On clicking, capture mode session will end and we try to snap a deleted
window. This CL ignores the capture label window so that the window
finder algorithm can find the next top window. That next top window will
be the snapped window instead and will persist when capture session
gets destroyed.

This can happen to any capture session widget/window, but only the
capture label is visible or created during window mode.

Fixed: 1152938
Test: ash_unittests CaptureModeTest.TabletTouchCaptureLabelWidgetWindowMode
Change-Id: I6fe8c2fea34c4738ef41dbafca2609b0b6913589
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566450
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832197}
parent b83caed9
...@@ -693,8 +693,15 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event, ...@@ -693,8 +693,15 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
case ui::ET_TOUCH_PRESSED: case ui::ET_TOUCH_PRESSED:
case ui::ET_TOUCH_MOVED: { case ui::ET_TOUCH_MOVED: {
if (is_capture_window) { if (is_capture_window) {
// Make sure the capture label widget will not get picked up by the
// get topmost window algorithm otherwise a crash will happen since
// the snapshot code tries snap a deleted window.
std::set<aura::Window*> ignore_windows;
if (capture_label_widget_)
ignore_windows.insert(capture_label_widget_->GetNativeWindow());
capture_window_observer_->UpdateSelectedWindowAtPosition( capture_window_observer_->UpdateSelectedWindowAtPosition(
screen_location); screen_location, ignore_windows);
} }
UpdateCursor(screen_location, is_touch); UpdateCursor(screen_location, is_touch);
break; break;
......
...@@ -1791,6 +1791,31 @@ TEST_F(CaptureModeTest, ScreenshotConfigurationHistogram) { ...@@ -1791,6 +1791,31 @@ TEST_F(CaptureModeTest, ScreenshotConfigurationHistogram) {
kTabletHistogram, CaptureModeConfiguration::kWindowScreenshot, 1); kTabletHistogram, CaptureModeConfiguration::kWindowScreenshot, 1);
} }
// Tests that there is no crash when touching the capture label widget in tablet
// mode when capturing a window. Regression test for https://crbug.com/1152938.
TEST_F(CaptureModeTest, TabletTouchCaptureLabelWidgetWindowMode) {
TabletModeControllerTestApi tablet_mode_controller_test_api;
tablet_mode_controller_test_api.EnterTabletMode();
// Enter capture window mode.
CaptureModeController* controller =
StartCaptureSession(CaptureModeSource::kWindow, CaptureModeType::kImage);
ASSERT_TRUE(controller->IsActive());
// Press and release on where the capture label widget would be.
auto* event_generator = GetEventGenerator();
CaptureModeSessionTestApi test_api(controller->capture_mode_session());
DCHECK(test_api.capture_label_widget());
event_generator->set_current_screen_location(
test_api.capture_label_widget()->GetWindowBoundsInScreen().CenterPoint());
event_generator->PressTouch();
event_generator->ReleaseTouch();
// There are no windows so the window finder algorithm will find the app list
// window and take a picture of that, ending capture mode.
EXPECT_FALSE(controller->IsActive());
}
// A test class that uses a mock time task environment. // A test class that uses a mock time task environment.
class CaptureModeMockTimeTest : public CaptureModeTest { class CaptureModeMockTimeTest : public CaptureModeTest {
public: public:
......
...@@ -25,8 +25,26 @@ CaptureWindowObserver::~CaptureWindowObserver() { ...@@ -25,8 +25,26 @@ CaptureWindowObserver::~CaptureWindowObserver() {
} }
void CaptureWindowObserver::UpdateSelectedWindowAtPosition( void CaptureWindowObserver::UpdateSelectedWindowAtPosition(
const gfx::Point& location_in_screen) { const gfx::Point& location_in_screen,
UpdateSelectedWindowAtPosition(location_in_screen, /*ignore_windows=*/{}); const std::set<aura::Window*>& ignore_windows) {
location_in_screen_ = location_in_screen;
// Find the toplevel window under the mouse/touch position.
aura::Window* window =
GetTopmostWindowAtPoint(location_in_screen_, ignore_windows);
if (window_ == window)
return;
// Don't capture wallpaper window.
if (window && window->parent() &&
window->parent()->id() == kShellWindowId_WallpaperContainer) {
window = nullptr;
}
// Stop observing the current selected window if there is one.
StopObserving();
if (window)
StartObserving(window);
RepaintCaptureRegion();
} }
void CaptureWindowObserver::OnWindowBoundsChanged( void CaptureWindowObserver::OnWindowBoundsChanged(
...@@ -76,29 +94,6 @@ void CaptureWindowObserver::StopObserving() { ...@@ -76,29 +94,6 @@ void CaptureWindowObserver::StopObserving() {
} }
} }
void CaptureWindowObserver::UpdateSelectedWindowAtPosition(
const gfx::Point& location_in_screen,
const std::set<aura::Window*>& ignore_windows) {
location_in_screen_ = location_in_screen;
// Find the toplevel window under the mouse/touch position.
aura::Window* window =
GetTopmostWindowAtPoint(location_in_screen_, ignore_windows);
if (window_ == window)
return;
// Don't capture wallpaper window.
if (window && window->parent() &&
window->parent()->id() == kShellWindowId_WallpaperContainer) {
window = nullptr;
}
// Stop observing the current selected window if there is one.
StopObserving();
if (window)
StartObserving(window);
RepaintCaptureRegion();
}
void CaptureWindowObserver::RepaintCaptureRegion() { void CaptureWindowObserver::RepaintCaptureRegion() {
ui::Layer* layer = capture_mode_session_->layer(); ui::Layer* layer = capture_mode_session_->layer();
layer->SchedulePaint(layer->bounds()); layer->SchedulePaint(layer->bounds());
......
...@@ -33,10 +33,12 @@ class ASH_EXPORT CaptureWindowObserver : public aura::WindowObserver, ...@@ -33,10 +33,12 @@ class ASH_EXPORT CaptureWindowObserver : public aura::WindowObserver,
~CaptureWindowObserver() override; ~CaptureWindowObserver() override;
// Updates selected window depending on the mouse/touch event location. If // Updates selected window depending on the mouse/touch event location,
// there is an eligible window under the current mouse/touch event location, // ignoring |ignore_windows|. If there is an eligible window under the current
// its bounds will be highlighted. // mouse/touch event location, its bounds will be highlighted.
void UpdateSelectedWindowAtPosition(const gfx::Point& location_in_screen); void UpdateSelectedWindowAtPosition(
const gfx::Point& location_in_screen,
const std::set<aura::Window*>& ignore_windows);
// aura::WindowObserver: // aura::WindowObserver:
void OnWindowBoundsChanged(aura::Window* window, void OnWindowBoundsChanged(aura::Window* window,
...@@ -57,12 +59,6 @@ class ASH_EXPORT CaptureWindowObserver : public aura::WindowObserver, ...@@ -57,12 +59,6 @@ class ASH_EXPORT CaptureWindowObserver : public aura::WindowObserver,
void StartObserving(aura::Window* window); void StartObserving(aura::Window* window);
void StopObserving(); void StopObserving();
// Updates selected window depending on the mouse/touch event location with
// ignoring |ignore_windows|.
void UpdateSelectedWindowAtPosition(
const gfx::Point& location_in_screen,
const std::set<aura::Window*>& ignore_windows);
// Repaints the window capture region. // Repaints the window capture region.
void RepaintCaptureRegion(); void RepaintCaptureRegion();
......
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