From 540cdc0d93659aa5b556f34bd973d3ad4a73213a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= <verdre@v0yd.nl>
Date: Sun, 27 Aug 2023 13:44:06 +0200
Subject: [PATCH] clutter/stage: Don't set implicit_grab_cancelled to TRUE if
 there are none

The assertion for !implicit_grab_cancelled in the
`grab_actor == old_grab_actor` case of
clutter_stage_notify_grab_on_pointer_entry() is meant to do a simple
sanity-check to ensure the grab machinery is working as intended: During a
seat grab, all input gets delivered to the tree inside the grab, and all
implicit grabs outside of that tree are cancelled.

When a new seat grab on the same actor as the existing one happens, we run
through the cancellation machinery for implicit grabs anyway, so we might as
well check that the assumption mentioned above holds true: By asserting that
no implicit grabs were cancelled, we know that no implicit grabs exist
outside of the existing seat grab tree.

This assertion is slightly over-eager though due to the way we set
implicit_grab_cancelled: We initialize it to TRUE in the
entry->press_count > 0 case and then only set it to FALSE once we find an
implicit grab that may remain active. If there are no implicit grabs though
(while entry->press_count is still >0), we never set implicit_grab_cancelled
to FALSE, triggering the assertion in question even though no implicit grabs
got cancelled.

There's two possible solutions for this: Either dropping the assertion, or
refactoring it so it observes the situation where the implicit grabs were
already undone. This commit implements the latter.

Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/2700

Fixes: debbd88f8c8 ("clutter/stage: Cancel parts of implicit grabs when ClutterGrabs happen")
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3216>
---
 clutter/clutter/clutter-stage.c | 35 ++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c
index 6736e4d5c..bbe7a2116 100644
--- a/clutter/clutter/clutter-stage.c
+++ b/clutter/clutter/clutter-stage.c
@@ -3668,6 +3668,7 @@ clutter_stage_notify_grab_on_pointer_entry (ClutterStage       *stage,
 {
   gboolean pointer_in_grab, pointer_in_old_grab;
   gboolean implicit_grab_cancelled = FALSE;
+  unsigned int implicit_grab_n_removed = 0, implicit_grab_n_remaining = 0;
   ClutterEventType event_type = CLUTTER_NOTHING;
   ClutterActor *topmost, *deepmost;
 
@@ -3688,9 +3689,6 @@ clutter_stage_notify_grab_on_pointer_entry (ClutterStage       *stage,
       ClutterInputDevice *device = entry->device;
       ClutterEventSequence *sequence = entry->sequence;
       unsigned int i;
-      unsigned int n_removed = 0;
-
-      implicit_grab_cancelled = TRUE;
 
       for (i = 0; i < entry->event_emission_chain->len; i++)
         {
@@ -3702,11 +3700,11 @@ clutter_stage_notify_grab_on_pointer_entry (ClutterStage       *stage,
               if (!clutter_actor_contains (grab_actor, receiver->actor))
                 {
                   g_clear_object (&receiver->actor);
-                  n_removed++;
+                  implicit_grab_n_removed++;
                 }
               else
                 {
-                  implicit_grab_cancelled = FALSE;
+                  implicit_grab_n_remaining++;
                 }
             }
           else if (receiver->action)
@@ -3720,23 +3718,27 @@ clutter_stage_notify_grab_on_pointer_entry (ClutterStage       *stage,
                                                      device,
                                                      sequence);
                   g_clear_object (&receiver->action);
-                  n_removed++;
+                  implicit_grab_n_removed++;
                 }
               else
                 {
-                  implicit_grab_cancelled = FALSE;
+                  implicit_grab_n_remaining++;
                 }
             }
         }
 
-      if (n_removed > 0)
-        {
-          CLUTTER_NOTE (GRABS,
-                        "[grab=%p device=%p sequence=%p implicit_grab_cancelled=%d] "
-                        "Cancelled %u actors and actions on implicit grab due to new seat grab",
-                        stage->priv->topmost_grab, device, sequence, implicit_grab_cancelled,
-                        n_removed);
-        }
+      /* Seat grabs win over implicit grabs, so we default to cancel the ongoing
+       * implicit grab. If the seat grab contains one or more actors from
+       * the implicit grab though, the implicit grab remains in effect.
+       */
+      implicit_grab_cancelled = implicit_grab_n_remaining == 0;
+
+      CLUTTER_NOTE (GRABS,
+                    "[grab=%p device=%p sequence=%p implicit_grab_cancelled=%d] "
+                    "Cancelled %u actors and actions (%u remaining) on implicit "
+                    "grab due to new seat grab",
+                    stage->priv->topmost_grab, device, sequence, implicit_grab_cancelled,
+                    implicit_grab_n_removed, implicit_grab_n_remaining);
     }
 
   /* Equate NULL actors to the stage here, to ease calculations further down. */
@@ -3747,7 +3749,8 @@ clutter_stage_notify_grab_on_pointer_entry (ClutterStage       *stage,
 
   if (grab_actor == old_grab_actor)
     {
-      g_assert (!implicit_grab_cancelled);
+      g_assert ((implicit_grab_n_removed == 0 && implicit_grab_n_remaining == 0) ||
+                !implicit_grab_cancelled);
       return;
     }