in notifier.c, fix potential callchain loop when closing a notification

2 files changed, 70 insertions, 31 deletions

diff --git a/src/notifier.c b/src/notifier.c
index 1ac7e32..dc3a186 100644
--- a/src/notifier.c
+++ b/src/notifier.c
@@ -90,55 +90,71 @@ static void set_power_level_property (IndicatorPowerNotifier*,
 ***/
 
 static void
+on_notify_notification_finalized (gpointer gself, GObject * dead)
+{
+  IndicatorPowerNotifier * const self = INDICATOR_POWER_NOTIFIER(gself);
+  priv_t * const p = get_priv(self);
+  g_return_if_fail ((void*)(p->notify_notification) == (void*)dead);
+  p->notify_notification = NULL;
+  set_is_warning_property (self, FALSE);
+}
+
+static void
 notification_clear (IndicatorPowerNotifier * self)
 {
   priv_t * const p = get_priv(self);
+  NotifyNotification * nn;
 
-  if (p->notify_notification != NULL)
+  if ((nn = p->notify_notification))
     {
-      set_is_warning_property (self, FALSE);
+      GError * error = NULL;
+
+      g_object_weak_unref(G_OBJECT(nn), on_notify_notification_finalized, self);
 
-      notify_notification_clear_actions(p->notify_notification);
-      g_signal_handlers_disconnect_by_data(p->notify_notification, self);
-      g_clear_object(&p->notify_notification);
+      if (!notify_notification_close(nn, &error))
+        {
+          g_warning("Unable to close notification: %s", error->message);
+          g_error_free(error);
+        }
+
+      p->notify_notification = NULL;
+      set_is_warning_property(self, FALSE);
     }
 }
 
 static void
 notification_show(IndicatorPowerNotifier * self)
 {
-  priv_t * p;
+  priv_t * const p = get_priv(self);
+  gdouble pct;
   char * body;
+  NotifyNotification * nn;
   GError * error;
 
-  notification_clear (self);
-
-  p = get_priv (self);
+  notification_clear(self);
 
   /* create the notification */
-  body = g_strdup_printf(_("%.0f%% charge remaining"),
-                         indicator_power_device_get_percentage(p->battery));
-  p->notify_notification = notify_notification_new(_("Battery Low"), body, NULL);
-  notify_notification_set_hint(p->notify_notification,
-                               HINT_INTERACTIVE,
-                               g_variant_new_boolean(TRUE));
-  g_signal_connect_swapped(p->notify_notification, "closed",
-                           G_CALLBACK(notification_clear), self);
-
-  /* show the notification */
+  pct = indicator_power_device_get_percentage(p->battery);
+  body = g_strdup_printf(_("%.0f%% charge remaining"), pct);
+  nn = notify_notification_new(_("Battery Low"), body, NULL);
+  g_free (body);
+  notify_notification_set_hint(nn, HINT_INTERACTIVE, g_variant_new_boolean(TRUE));
+
+  /* if we can show it, keep it */
   error = NULL;
-  notify_notification_show(p->notify_notification, &error);
-  if (error != NULL)
+  if (notify_notification_show(nn, &error))
     {
-      g_critical("Unable to show snap decision for '%s': %s", body, error->message);
-      g_error_free(error);
+      p->notify_notification = nn;
+      g_signal_connect(nn, "closed", G_CALLBACK(g_object_unref), NULL);
+      g_object_weak_ref(G_OBJECT(nn), on_notify_notification_finalized, self);
+      set_is_warning_property(self, TRUE);
     }
   else
     {
-      set_is_warning_property (self, TRUE);
+      g_critical("Unable to show snap decision for '%s': %s", body, error->message);
+      g_error_free(error);
+      g_object_unref(nn);
     }
-
-  g_free (body);
 }
 
 /***
diff --git a/tests/test-notify.cc b/tests/test-notify.cc
index 1fc843e..a8d66d3 100644
--- a/tests/test-notify.cc
+++ b/tests/test-notify.cc
@@ -54,7 +54,7 @@ protected:
   DbusTestDbusMockObject * obj = nullptr;
   GDBusConnection * bus = nullptr;
 
-  static constexpr int NOTIFY_ID {1234};
+  static constexpr int FIRST_NOTIFY_ID {1234};
 
   static constexpr int NOTIFICATION_CLOSED_EXPIRED   {1};
   static constexpr int NOTIFICATION_CLOSED_DISMISSED {2};
@@ -63,9 +63,11 @@ protected:
 
   static constexpr char const * APP_NAME {"indicator-power-service"};
 
+  static constexpr char const * METHOD_CLOSE {"CloseNotification"};
   static constexpr char const * METHOD_NOTIFY {"Notify"};
   static constexpr char const * METHOD_GET_CAPS {"GetCapabilities"};
   static constexpr char const * METHOD_GET_INFO {"GetServerInformation"};
+  static constexpr char const * SIGNAL_CLOSED {"NotificationClosed"};
 
   static constexpr char const * HINT_TIMEOUT {"x-canonical-snap-decisions-timeout"};
 
@@ -86,7 +88,8 @@ protected:
                                          NOTIFY_INTERFACE,
                                          &error);
     g_assert_no_error (error);
-    
+  
+    // METHOD_GET_INFO 
     dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_GET_INFO,
                                           nullptr,
                                           G_VARIANT_TYPE("(ssss)"),
@@ -94,14 +97,34 @@ protected:
                                           &error);
     g_assert_no_error (error);
 
-    auto python_str = g_strdup_printf ("ret = %d", NOTIFY_ID);
+    // METHOD_NOTIFY
+    auto str = g_strdup_printf("try:\n"
+                               "  self.NextNotifyId\n"
+                               "except AttributeError:\n"
+                               "  self.NextNotifyId = %d\n"
+                               "ret = self.NextNotifyId\n"
+                               "self.NextNotifyId += 1\n",
+                               FIRST_NOTIFY_ID);
     dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_NOTIFY,
                                           G_VARIANT_TYPE("(susssasa{sv}i)"),
                                           G_VARIANT_TYPE_UINT32,
-                                          python_str,
+                                          str,
                                           &error);
-    g_free (python_str);
     g_assert_no_error (error);
+    g_free (str);
+
+    // METHOD_CLOSE 
+    str = g_strdup_printf("self.EmitSignal('%s', '%s', 'uu', [ args[0], %d ])",
+                          NOTIFY_INTERFACE,
+                          SIGNAL_CLOSED,
+                          NOTIFICATION_CLOSED_API);
+    dbus_test_dbus_mock_object_add_method(mock, obj, METHOD_CLOSE,
+                                          G_VARIANT_TYPE("(u)"),
+                                          nullptr,
+                                          str,
+                                          &error);
+    g_assert_no_error (error);
+    g_free (str);
 
     dbus_test_service_add_task(service, DBUS_TEST_TASK(mock));
     dbus_test_service_start_tasks(service);