Exo: Undo conflation of relative-pointer and pointer-constraints

When it was first implemented, adding a relative-pointer-delegate would
also enable pointer capture (locking the pointer).

This change makes it so that the pointer can only be locked through the
pointer-constraints protocol.

Bug: b/124059008
Test: exo_unittests, manual testting using ArcPointerCaptureTestApp
Change-Id: Ied0f1c658aa713026e622f4ce26009e261d4f653
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992355Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Prabir Pradhan <prabirmsp@chromium.org>
Auto-Submit: Prabir Pradhan <prabirmsp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733497}

components/exo/pointer.cc

@@ -225,62 +225,6 @@ void Pointer::UnregisterRelativePointerDelegate(
   relative_pointer_delegate_ = nullptr;
 }
 
-bool Pointer::EnablePointerCapture() {
-  if (!base::FeatureList::IsEnabled(kPointerCapture))
-    return false;
-
-  // You are not allowed to have more than one capture active.
-  if (capture_window_)
-    return false;
-
-  aura::Window* active_window = WMHelper::GetInstance()->GetActiveWindow();
-  if (!active_window) {
-    LOG(ERROR) << "Failed to enable pointer capture: "
-                  "active window not found";
-    return false;
-  }
-  auto* top_level_widget =
-      views::Widget::GetTopLevelWidgetForNativeView(active_window);
-
-  if (!top_level_widget) {
-    LOG(ERROR) << "Failed to enable pointer capture: "
-                  "active window does not have associated widget";
-    return false;
-  }
-  Surface* root_surface =
-      GetShellMainSurface(top_level_widget->GetNativeWindow());
-  if (!root_surface ||
-      !delegate_->CanAcceptPointerEventsForSurface(root_surface)) {
-    LOG(ERROR) << "Failed to enable pointer capture: "
-                  "cannot find window for capture";
-    return false;
-  }
-  return EnablePointerCapture(root_surface);
-}
-
-void Pointer::DisablePointerCapture() {
-  // Early out if pointer capture is not enabled.
-  if (!capture_window_)
-    return;
-
-  // Remove the pre-target handler that consumes all mouse events.
-  aura::Env::GetInstance()->RemovePreTargetHandler(this);
-
-  auto* cursor_client = WMHelper::GetInstance()->GetCursorClient();
-  cursor_client->UnlockCursor();
-  cursor_client->ShowCursor();
-
-  aura::Window* root = capture_window_->GetRootWindow();
-  gfx::Point p = location_when_pointer_capture_enabled_
-                     ? *location_when_pointer_capture_enabled_
-                     : root->bounds().CenterPoint();
-  root->MoveCursorTo(p);
-
-  capture_window_ = nullptr;
-  location_when_pointer_capture_enabled_.reset();
-  UpdateCursor();
-}
-
 bool Pointer::ConstrainPointer(PointerConstraintDelegate* delegate) {
   // Pointer lock is a chromeos-only feature (i.e. the chromeos::features
   // namespace only exists in chromeos builds). So we do not compile pointer
@@ -345,6 +289,29 @@ bool Pointer::EnablePointerCapture(Surface* capture_surface) {
   return true;
 }
 
+void Pointer::DisablePointerCapture() {
+  // Early out if pointer capture is not enabled.
+  if (!capture_window_)
+    return;
+
+  // Remove the pre-target handler that consumes all mouse events.
+  aura::Env::GetInstance()->RemovePreTargetHandler(this);
+
+  auto* cursor_client = WMHelper::GetInstance()->GetCursorClient();
+  cursor_client->UnlockCursor();
+  cursor_client->ShowCursor();
+
+  aura::Window* root = capture_window_->GetRootWindow();
+  gfx::Point p = location_when_pointer_capture_enabled_
+                     ? *location_when_pointer_capture_enabled_
+                     : root->bounds().CenterPoint();
+  root->MoveCursorTo(p);
+
+  capture_window_ = nullptr;
+  location_when_pointer_capture_enabled_.reset();
+  UpdateCursor();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 

components/exo/pointer.h

@@ -92,17 +92,6 @@ class Pointer : public SurfaceTreeHost,
   void RegisterRelativePointerDelegate(RelativePointerDelegate* delegate);
   void UnregisterRelativePointerDelegate(RelativePointerDelegate* delegate);
 
-  // Capture the pointer for the top-most surface. Returns true iff the capture
-  // succeeded.
-  //
-  // TODO(b/124059008): Historically, exo needed to guess what the correct
-  // capture window was, as it did not implement wayland's pointer capture
-  // protocol.
-  bool EnablePointerCapture();
-
-  // Remove the currently active pointer capture (if there is one).
-  void DisablePointerCapture();
-
   // Enable the pointer constraint on the given surface. Returns true if the
   // lock was granted, false otherwise.
   //
@@ -121,6 +110,9 @@ class Pointer : public SurfaceTreeHost,
   // succeeded.
   bool EnablePointerCapture(Surface* capture_surface);
 
+  // Remove the currently active pointer capture (if there is one).
+  void DisablePointerCapture();
+
   // Returns the effective target for |event|.
   Surface* GetEffectiveTargetForEvent(ui::LocatedEvent* event) const;
 

components/exo/wayland/zwp_relative_pointer_manager.cc

@@ -27,21 +27,12 @@ class WaylandRelativePointerDelegate : public RelativePointerDelegate {
   WaylandRelativePointerDelegate(wl_resource* resource, Pointer* pointer)
       : resource_(resource), pointer_(pointer) {
     pointer->RegisterRelativePointerDelegate(this);
-    // TODO(b/124059008): See below, when requesting relative motion we will
-    // also try to gain pointer lock even though the client hasn't asked for it
-    // yet...
-    pointer_->EnablePointerCapture();
   }
 
   ~WaylandRelativePointerDelegate() override {
-    if (pointer_) {
-      // TODO(b/124059008): For whatever reason, exo conflates pointer capture
-      // and relative motion. Normally in wayland, removing the relative pointer
-      // would not break pointer capture, but in exo that is the case.
-      pointer_->DisablePointerCapture();
+    if (pointer_)
       pointer_->UnregisterRelativePointerDelegate(this);
   }
-  }
   void OnPointerDestroying(Pointer* pointer) override { pointer_ = nullptr; }
   void OnPointerRelativeMotion(base::TimeTicks time_stamp,
                                const gfx::PointF& relativeMotion) override {