[mutter] clutter/stage: Repick when pointer actor goes unmapped

[Marge Bot <marge-bot src gnome org>]

commit cfdca246f26e2d6f73f0459a049421f926b21ec1
Author: Jonas Dreßler <verdre v0yd nl>
Date:   Wed Mar 9 15:53:11 2022 +0100

    clutter/stage: Repick when pointer actor goes unmapped
    
    I've overseen quite an important case in commit
    98a5cb37d9159737f8f1af4196420db90bfcf879: Repicking only when actors get
    destroyed is not enough, we actually need to repick when actors go
    hidden/unmapped.
    
    While we could also listen to notify::mapped just like we listen to
    notify::reactive, it seems better to avoid using property notifications
    here due to the usage of g_object_freeze/thaw_notify() in ClutterActor.
    It can lead to the stage receiving a notify::mapped with mapped = true
    for a pointer actor, which really shouldn't happen (just like
    notify::reactive with reactive = true shouldn't happen).
    
    Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5124
    Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2333>

 clutter/clutter/clutter-actor.c         |  18 ++++++
 clutter/clutter/clutter-stage-private.h |   3 +
 clutter/clutter/clutter-stage.c         | 105 +++++++++++++++-----------------
 3 files changed, 69 insertions(+), 57 deletions(-)
---
diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c
index 4279d3476c..b31a4d01d6 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -1701,6 +1701,13 @@ clutter_actor_real_unmap (ClutterActor *self)
    */
   g_object_notify_by_pspec (G_OBJECT (self), obj_props[PROP_MAPPED]);
 
+  if (priv->has_pointer)
+    {
+      ClutterActor *stage = _clutter_actor_get_stage_internal (self);
+
+      clutter_stage_invalidate_focus (CLUTTER_STAGE (stage), self);
+    }
+
   /* relinquish keyboard focus if we were unmapped while owning it */
   if (!CLUTTER_ACTOR_IS_TOPLEVEL (self))
     maybe_unset_key_focus (self);
@@ -12440,8 +12447,12 @@ void
 clutter_actor_set_reactive (ClutterActor *actor,
                             gboolean      reactive)
 {
+  ClutterActorPrivate *priv;
+
   g_return_if_fail (CLUTTER_IS_ACTOR (actor));
 
+  priv = actor->priv;
+
   if (reactive == CLUTTER_ACTOR_IS_REACTIVE (actor))
     return;
 
@@ -12451,6 +12462,13 @@ clutter_actor_set_reactive (ClutterActor *actor,
     CLUTTER_ACTOR_UNSET_FLAGS (actor, CLUTTER_ACTOR_REACTIVE);
 
   g_object_notify_by_pspec (G_OBJECT (actor), obj_props[PROP_REACTIVE]);
+
+  if (!CLUTTER_ACTOR_IS_REACTIVE (actor) && priv->has_pointer)
+    {
+      ClutterActor *stage = _clutter_actor_get_stage_internal (actor);
+
+      clutter_stage_invalidate_focus (CLUTTER_STAGE (stage), actor);
+    }
 }
 
 /**
diff --git a/clutter/clutter/clutter-stage-private.h b/clutter/clutter/clutter-stage-private.h
index efa341cc19..e9ba5707ed 100644
--- a/clutter/clutter/clutter-stage-private.h
+++ b/clutter/clutter/clutter-stage-private.h
@@ -154,6 +154,9 @@ ClutterActor * clutter_stage_pick_and_update_device (ClutterStage             *s
 void clutter_stage_unlink_grab (ClutterStage *self,
                                 ClutterGrab  *grab);
 
+void clutter_stage_invalidate_focus (ClutterStage *self,
+                                     ClutterActor *actor);
+
 G_END_DECLS
 
 #endif /* __CLUTTER_STAGE_PRIVATE_H__ */
diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c
index 5cd09a2c44..705bd92a8f 100644
--- a/clutter/clutter/clutter-stage.c
+++ b/clutter/clutter/clutter-stage.c
@@ -3150,53 +3150,62 @@ clutter_stage_set_actor_needs_immediate_relayout (ClutterStage *stage)
   priv->actor_needs_immediate_relayout = TRUE;
 }
 
-static void
-on_device_actor_reactive_changed (ClutterActor       *actor,
-                                  GParamSpec         *pspec,
-                                  PointerDeviceEntry *entry)
+void
+clutter_stage_invalidate_focus (ClutterStage *self,
+                                ClutterActor *actor)
 {
-  ClutterStage *self = entry->stage;
+  ClutterStagePrivate *priv = self->priv;
+  GHashTableIter iter;
+  gpointer value;
 
-  g_assert (!clutter_actor_get_reactive (actor));
+  if (CLUTTER_ACTOR_IN_DESTRUCTION (self))
+    return;
 
-  clutter_stage_pick_and_update_device (self,
-                                        entry->device,
-                                        entry->sequence,
-                                        CLUTTER_DEVICE_UPDATE_IGNORE_CACHE |
-                                        CLUTTER_DEVICE_UPDATE_EMIT_CROSSING,
-                                        entry->coords,
-                                        CLUTTER_CURRENT_TIME);
-}
+  g_assert (!clutter_actor_is_mapped (actor) || !clutter_actor_get_reactive (actor));
 
-static void
-on_device_actor_destroyed (ClutterActor       *actor,
-                           PointerDeviceEntry *entry)
-{
-  /* Simply unset the current_actor pointer here, there's no need to
-   * unset has_pointer or to disconnect any signals because the actor
-   * is gone anyway.
-   */
-  entry->current_actor = NULL;
-  g_clear_pointer (&entry->clear_area, cairo_region_destroy);
-  clutter_stage_repick_device (entry->stage, entry->device);
+  g_hash_table_iter_init (&iter, priv->pointer_devices);
+  while (g_hash_table_iter_next (&iter, NULL, &value))
+    {
+      PointerDeviceEntry *entry = value;
+
+      if (entry->current_actor != actor)
+        continue;
+
+      clutter_stage_pick_and_update_device (self,
+                                            entry->device,
+                                            NULL,
+                                            CLUTTER_DEVICE_UPDATE_IGNORE_CACHE |
+                                            CLUTTER_DEVICE_UPDATE_EMIT_CROSSING,
+                                            entry->coords,
+                                            CLUTTER_CURRENT_TIME);
+    }
+
+  g_hash_table_iter_init (&iter, priv->touch_sequences);
+  while (g_hash_table_iter_next (&iter, NULL, &value))
+    {
+      PointerDeviceEntry *entry = value;
+
+      if (entry->current_actor != actor)
+        continue;
+
+      clutter_stage_pick_and_update_device (self,
+                                            entry->device,
+                                            entry->sequence,
+                                            CLUTTER_DEVICE_UPDATE_IGNORE_CACHE |
+                                            CLUTTER_DEVICE_UPDATE_EMIT_CROSSING,
+                                            entry->coords,
+                                            CLUTTER_CURRENT_TIME);
+    }
+
+  if (actor != CLUTTER_ACTOR (self))
+    g_assert (!clutter_actor_has_pointer (actor));
 }
 
 static void
 free_pointer_device_entry (PointerDeviceEntry *entry)
 {
   if (entry->current_actor)
-    {
-      ClutterActor *actor = entry->current_actor;
-
-      g_signal_handlers_disconnect_by_func (actor,
-                                            G_CALLBACK (on_device_actor_reactive_changed),
-                                            entry);
-      g_signal_handlers_disconnect_by_func (actor,
-                                            G_CALLBACK (on_device_actor_destroyed),
-                                            entry);
-
-      _clutter_actor_set_has_pointer (actor, FALSE);
-   }
+    _clutter_actor_set_has_pointer (entry->current_actor, FALSE);
 
   g_clear_pointer (&entry->clear_area, cairo_region_destroy);
 
@@ -3240,30 +3249,12 @@ clutter_stage_update_device_entry (ClutterStage         *self,
   if (entry->current_actor != actor)
     {
       if (entry->current_actor)
-        {
-          ClutterActor *old_actor = entry->current_actor;
-
-          g_signal_handlers_disconnect_by_func (old_actor,
-                                                G_CALLBACK (on_device_actor_reactive_changed),
-                                                entry);
-          g_signal_handlers_disconnect_by_func (old_actor,
-                                                G_CALLBACK (on_device_actor_destroyed),
-                                                entry);
-
-          _clutter_actor_set_has_pointer (old_actor, FALSE);
-        }
+        _clutter_actor_set_has_pointer (entry->current_actor, FALSE);
 
       entry->current_actor = actor;
 
       if (actor)
-        {
-          g_signal_connect (actor, "notify::reactive",
-                            G_CALLBACK (on_device_actor_reactive_changed), entry);
-          g_signal_connect (actor, "destroy",
-                            G_CALLBACK (on_device_actor_destroyed), entry);
-
-          _clutter_actor_set_has_pointer (actor, TRUE);
-        }
+        _clutter_actor_set_has_pointer (actor, TRUE);
     }
 
   g_clear_pointer (&entry->clear_area, cairo_region_destroy);