Commit 2fb481c0 authored by Prabir Pradhan's avatar Prabir Pradhan Committed by Chromium LUCI CQ

Reland "exo: Allow Pointer Capture when window is active"

This reverts commit b89a947a.

Reason for revert: The CTS failures caused by landing this change
for P are being addressed in ag/13257099. For changes required for R,
see b/176433665.

Original change's description:
> Revert "exo: Allow Pointer Capture when window is active"
>
> This reverts commit 43be36f0.
>
> Reason for revert: Multiple CTS failures
>
> Bug: b:175379280
> Bug: b:175514308
> Bug: b:175829322
>
> Original change's description:
> > exo: Allow Pointer Capture when window is active
> >
> > Pointer Capture was being rejected by chrome when an app came into
> > focus using keyboard shortcuts (Alt-Tab), but was working as intended
> > when an app came into focus using the mouse or touchscreen. It was being
> > rejected because the capture_surface was the sub-surface of the current
> > focused surface.
> >
> > Since the client is responsible for providing the capture surface
> > through the wayland pointer constraints protocol, it is possible
> > for the client to request capture on a sub-surface of the current
> > active surface. Requesting capture on a sub-surface is a valid request
> > for capture. Rather than ensuring that the capture surface is focused
> > (used for directing key events), we verify that the active window
> > contains the window of the capture surface in its hierarchy before we
> > allow the pointer to be constrained to the surface.
> >
> > This also allows the client to change the cursor while the client has
> > capture, even when the mouse cursor is not currently over a client
> > window.
> >
> > BUG=b:153973515
> > TEST=manual: request pointer capture, leave app, return to app using
> > Alt+Tab, verify app has pointer capture.
> > TEST=exo_unittests
> >
> > Change-Id: I56e76469f35ea43100748ebe27966464e32805fc
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2559255
> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> > Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
> > Commit-Queue: Prabir Pradhan <prabirmsp@chromium.org>
> > Auto-Submit: Prabir Pradhan <prabirmsp@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#832613}
>
> TBR=oshima@chromium.org,tetsui@chromium.org,prabirmsp@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: b:153973515
> Change-Id: I582f775f37c34149fb7f0c2a6b9162b5e72cbf78
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596065
> Commit-Queue: Prabir Pradhan <prabirmsp@chromium.org>
> Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
> Auto-Submit: Prabir Pradhan <prabirmsp@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#837908}

TBR=oshima@chromium.org,tetsui@chromium.org,prabirmsp@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b:175379280
Bug: b:175514308
Bug: b:175829322
Bug: b:153973515
Change-Id: I967663e2936a7d117ee2dce9edb8967ce922dd91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2605465Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Reviewed-by: default avatarTetsui Ohkubo <tetsui@chromium.org>
Commit-Queue: Prabir Pradhan <prabirmsp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843233}
parent a481610b
...@@ -155,8 +155,7 @@ Pointer::~Pointer() { ...@@ -155,8 +155,7 @@ Pointer::~Pointer() {
} }
void Pointer::SetCursor(Surface* surface, const gfx::Point& hotspot) { void Pointer::SetCursor(Surface* surface, const gfx::Point& hotspot) {
// Early out if the pointer doesn't have a surface in focus. if (!focus_surface_ && !capture_window_)
if (!focus_surface_)
return; return;
// This is used to avoid unnecessary cursor changes. // This is used to avoid unnecessary cursor changes.
...@@ -242,6 +241,8 @@ bool Pointer::ConstrainPointer(PointerConstraintDelegate* delegate) { ...@@ -242,6 +241,8 @@ bool Pointer::ConstrainPointer(PointerConstraintDelegate* delegate) {
// lock support unless we are on chromeos. // lock support unless we are on chromeos.
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
Surface* constrained_surface = delegate->GetConstrainedSurface(); Surface* constrained_surface = delegate->GetConstrainedSurface();
if (!constrained_surface)
return false;
// Pointer lock should be enabled for ARC by default. The kExoPointerLock // Pointer lock should be enabled for ARC by default. The kExoPointerLock
// should only apply to Crostini windows. // should only apply to Crostini windows.
bool is_arc_window = bool is_arc_window =
...@@ -272,17 +273,17 @@ bool Pointer::EnablePointerCapture(Surface* capture_surface) { ...@@ -272,17 +273,17 @@ bool Pointer::EnablePointerCapture(Surface* capture_surface) {
return false; return false;
} }
if (capture_surface->window() != aura::Window* window = capture_surface->window();
WMHelper::GetInstance()->GetFocusedWindow()) { aura::Window* active_window = WMHelper::GetInstance()->GetActiveWindow();
LOG(ERROR) if (!active_window || !active_window->Contains(window)) {
<< "Cannot enable pointer capture on a window that is not focused."; LOG(ERROR) << "Cannot enable pointer capture on an inactive window.";
return false; return false;
} }
if (!capture_surface->HasSurfaceObserver(this)) if (!capture_surface->HasSurfaceObserver(this))
capture_surface->AddSurfaceObserver(this); capture_surface->AddSurfaceObserver(this);
capture_window_ = capture_surface->window(); capture_window_ = window;
// Add a pre-target handler that can consume all mouse events before it gets // Add a pre-target handler that can consume all mouse events before it gets
// sent to other targets. // sent to other targets.
......
...@@ -1200,7 +1200,7 @@ TEST_F(PointerTest, ConstrainPointer) { ...@@ -1200,7 +1200,7 @@ TEST_F(PointerTest, ConstrainPointer) {
#endif #endif
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotFocused) { TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotActive) {
auto scoped_feature_list = std::make_unique<base::test::ScopedFeatureList>(); auto scoped_feature_list = std::make_unique<base::test::ScopedFeatureList>();
scoped_feature_list->InitAndEnableFeature( scoped_feature_list->InitAndEnableFeature(
chromeos::features::kExoPointerLock); chromeos::features::kExoPointerLock);
...@@ -1219,7 +1219,6 @@ TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotFocused) { ...@@ -1219,7 +1219,6 @@ TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotFocused) {
auto pointer = std::make_unique<Pointer>(&delegate, &seat); auto pointer = std::make_unique<Pointer>(&delegate, &seat);
aura::client::FocusClient* focus_client = aura::client::FocusClient* focus_client =
aura::client::GetFocusClient(ash::Shell::GetPrimaryRootWindow()); aura::client::GetFocusClient(ash::Shell::GetPrimaryRootWindow());
focus_client->FocusWindow(nullptr);
ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow());
EXPECT_CALL(delegate, CanAcceptPointerEventsForSurface(surface.get())) EXPECT_CALL(delegate, CanAcceptPointerEventsForSurface(surface.get()))
...@@ -1227,24 +1226,20 @@ TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotFocused) { ...@@ -1227,24 +1226,20 @@ TEST_F(PointerTest, ConstrainPointerFailsWhenSurfaceIsNotFocused) {
EXPECT_CALL(constraint_delegate, GetConstrainedSurface()) EXPECT_CALL(constraint_delegate, GetConstrainedSurface())
.WillRepeatedly(testing::Return(surface.get())); .WillRepeatedly(testing::Return(surface.get()));
EXPECT_FALSE(pointer->ConstrainPointer(&constraint_delegate));
auto child_surface = std::make_unique<Surface>(); auto second_surface = std::make_unique<Surface>();
auto child_shell_surface = std::make_unique<ShellSurface>( auto second_shell_surface =
child_surface.get(), gfx::Point(), true, false, std::make_unique<ShellSurface>(second_surface.get());
ash::desks_util::GetActiveDeskContainerId()); auto second_buffer = std::make_unique<Buffer>(
child_shell_surface->DisableMovement(); exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
child_shell_surface->SetParent(shell_surface.get()); second_surface->Attach(second_buffer.get());
gfx::Size child_buffer_size(15, 15); second_surface->Commit();
auto child_buffer = std::make_unique<Buffer>(
exo_test_helper()->CreateGpuMemoryBuffer(child_buffer_size));
child_surface->Attach(child_buffer.get());
child_surface->Commit();
EXPECT_CALL(delegate, CanAcceptPointerEventsForSurface(child_surface.get())) EXPECT_CALL(delegate, CanAcceptPointerEventsForSurface(second_surface.get()))
.WillRepeatedly(testing::Return(true)); .WillRepeatedly(testing::Return(true));
focus_client->FocusWindow(child_surface->window()); // Setting the focused window also makes it activated.
focus_client->FocusWindow(second_surface->window());
EXPECT_FALSE(pointer->ConstrainPointer(&constraint_delegate)); EXPECT_FALSE(pointer->ConstrainPointer(&constraint_delegate));
focus_client->FocusWindow(surface->window()); focus_client->FocusWindow(surface->window());
......
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