Skip to content

Commit

Permalink
exo: Restrict pointer lock per-client. Add Overview prompt to Crostini.
Browse files Browse the repository at this point in the history
* ARC++ continues to permit pointer lock regardless of flags.
* Other clients must have kUseOverviewToExitPointerLock==true set as a
  window property, as well as chrome://flags#exo-pointer-lock enabled.
* Enable kUseOverviewToExitPointerLock for Crostini (and Bruschetta, as
  it doesn't identify its windows separately).

The "Press Overview" prompts are therefore displayed in Crostini and
Bruschetta in addition to Borealis (when the Chrome flag is enabled).

Further, we ensure that no Exo client is able to lock the pointer unless
the "Press Overview" prompt is displayed. The only exceptions to this
rule are ARC++ (through long-standing precedent) and Lacros (which uses
a different mechanism to lock the pointer).

See http://go/exo-pointer-lock-2022 for the rationale and next steps.

BUG=b:203622329

Change-Id: I77cd78b632ad53f6ec5607bff2bfbd2045f89138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3469745
Auto-Submit: Chloe Pelling <cpelling@google.com>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972362}
  • Loading branch information
cpelling authored and Chromium LUCI CQ committed Feb 17, 2022
1 parent 334a197 commit 9eba497
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "chrome/browser/ash/crosapi/browser_util.h"
#include "chrome/browser/ash/crostini/crostini_features.h"
#include "chrome/browser/ash/crostini/crostini_shelf_utils.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
#include "chrome/browser/ash/plugin_vm/plugin_vm_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
Expand Down Expand Up @@ -591,6 +592,9 @@ void AppServiceAppWindowShelfController::RegisterWindow(
} else if (borealis::BorealisWindowManager::IsBorealisWindow(window)) {
window->SetProperty(chromeos::kUseOverviewToExitFullscreen, true);
window->SetProperty(chromeos::kUseOverviewToExitPointerLock, true);
} else if (crostini::IsCrostiniWindow(window)) {
// Permit pointer lock in Crostini (and Bruschetta).
window->SetProperty(chromeos::kUseOverviewToExitPointerLock, true);
}

AddWindowToShelf(window, shelf_id);
Expand Down
7 changes: 4 additions & 3 deletions chromeos/ui/base/window_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ extern const ui::ClassProperty<bool>* const kEscHoldToExitFullscreen;
COMPONENT_EXPORT(CHROMEOS_UI_BASE)
extern const ui::ClassProperty<bool>* const kUseOverviewToExitFullscreen;

// Whether to promote users to use Overview to exit pointer lock.
// Borealis apps set this since they permit pointer lock and
// don't have other visible affordances to let the user break the lock.
// If true, Exo clients may request pointer lock for this window.
// When the lock activates, users will be notified to use Overview to exit
// pointer lock.
// Only ARC++ and Lacros may use pointer lock without this property being set.
COMPONENT_EXPORT(CHROMEOS_UI_BASE)
extern const ui::ClassProperty<bool>* const kUseOverviewToExitPointerLock;

Expand Down
17 changes: 11 additions & 6 deletions components/exo/pointer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/trace_event/trace_event.h"
#include "build/chromeos_buildflags.h"
#include "chromeos/ui/base/window_properties.h"
#include "components/exo/input_trace.h"
#include "components/exo/pointer_constraint_delegate.h"
#include "components/exo/pointer_delegate.h"
Expand Down Expand Up @@ -262,12 +263,16 @@ bool Pointer::ConstrainPointer(PointerConstraintDelegate* delegate) {
delegate->OnDefunct();
return false;
}
// Pointer lock should be enabled for ARC by default. The kExoPointerLock
// should only apply to Crostini windows.
bool is_arc_window =
ash::IsArcWindow(constrained_surface->window()->GetToplevelWindow());
if (!is_arc_window &&
!base::FeatureList::IsEnabled(chromeos::features::kExoPointerLock)) {

// Pointer lock is permitted for ARC windows, and for windows configured to
// notify the user on lock activation. In the latter case, the
// kExoPointerLock feature must be enabled.
aura::Window* toplevel = constrained_surface->window()->GetToplevelWindow();
bool permitted =
ash::IsArcWindow(toplevel) ||
(base::FeatureList::IsEnabled(chromeos::features::kExoPointerLock) &&
toplevel->GetProperty(chromeos::kUseOverviewToExitPointerLock));
if (!permitted) {
delegate->OnDefunct();
return false;
}
Expand Down
39 changes: 34 additions & 5 deletions components/exo/pointer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "build/chromeos_buildflags.h"
#include "chromeos/ui/base/window_properties.h"
#include "components/exo/buffer.h"
#include "components/exo/data_source.h"
#include "components/exo/data_source_delegate.h"
Expand Down Expand Up @@ -183,7 +184,7 @@ class PointerConstraintTest : public PointerTest {
PointerTest::SetUp();
feature_list_.InitAndEnableFeature(chromeos::features::kExoPointerLock);

shell_surface_ = test::ShellSurfaceBuilder({10, 10}).BuildShellSurface();
shell_surface_ = BuildShellSurfaceWhichPermitsPointerLock();
surface_ = shell_surface_->surface_for_testing();
seat_ = std::make_unique<Seat>();
pointer_ = std::make_unique<Pointer>(&delegate_, seat_.get());
Expand Down Expand Up @@ -216,6 +217,18 @@ class PointerConstraintTest : public PointerTest {
PointerTest::TearDown();
}

std::unique_ptr<ShellSurface> BuildShellSurfaceWhichPermitsPointerLock() {
std::unique_ptr<ShellSurface> shell_surface =
test::ShellSurfaceBuilder({10, 10}).BuildShellSurface();

shell_surface->surface_for_testing()
->window()
->GetToplevelWindow()
->SetProperty(chromeos::kUseOverviewToExitPointerLock, true);

return shell_surface;
}

std::unique_ptr<ui::test::EventGenerator> generator_;
std::unique_ptr<Pointer> pointer_;
std::unique_ptr<Seat> seat_;
Expand Down Expand Up @@ -1362,6 +1375,24 @@ TEST_F(PointerConstraintTest, ConstrainPointer) {
}
#endif

#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(PointerConstraintTest, CanOnlyConstrainPermittedWindows) {
std::unique_ptr<ShellSurface> shell_surface =
test::ShellSurfaceBuilder({10, 10}).BuildShellSurface();
EXPECT_CALL(constraint_delegate_, GetConstrainedSurface())
.WillRepeatedly(testing::Return(shell_surface->surface_for_testing()));
// Called once when ConstrainPointer is denied, and again when the delegate
// is destroyed.
EXPECT_CALL(constraint_delegate_, OnDefunct()).Times(2);

EXPECT_FALSE(pointer_->ConstrainPointer(&constraint_delegate_));

pointer_->OnPointerConstraintDelegateDestroying(&constraint_delegate_);
EXPECT_CALL(delegate_, OnPointerDestroying(pointer_.get()));
pointer_.reset();
}
#endif

#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(PointerConstraintTest, OneConstraintPerSurface) {
ON_CALL(constraint_delegate_, IsPersistent())
Expand Down Expand Up @@ -1390,8 +1421,7 @@ TEST_F(PointerConstraintTest, OneConstraintPerSurface) {

#if BUILDFLAG(IS_CHROMEOS_ASH)
TEST_F(PointerConstraintTest, OneShotConstraintActivatedOnFirstFocus) {
auto second_shell_surface =
test::ShellSurfaceBuilder({10, 10}).BuildShellSurface();
auto second_shell_surface = BuildShellSurfaceWhichPermitsPointerLock();
Surface* second_surface = second_shell_surface->surface_for_testing();

EXPECT_CALL(delegate_, CanAcceptPointerEventsForSurface(second_surface))
Expand Down Expand Up @@ -1493,8 +1523,7 @@ TEST_F(PointerConstraintTest, MultipleSurfacesCanBeConstrained) {
EXPECT_EQ(constraint_delegate_.activated_count, 1);

// Arrange: Second surface + persistent constraint
auto second_shell_surface =
test::ShellSurfaceBuilder({10, 10}).BuildShellSurface();
auto second_shell_surface = BuildShellSurfaceWhichPermitsPointerLock();
Surface* second_surface = second_shell_surface->surface_for_testing();
focus_client_->FocusWindow(second_surface->window());
EXPECT_CALL(delegate_, CanAcceptPointerEventsForSurface(second_surface))
Expand Down

0 comments on commit 9eba497

Please sign in to comment.