[ofono] Resolved a few issues with cell info notifications. JB#50816

1. Disable notifications from modem on unsibscribe 2. Made all signals unicast 3. Fixed a memory leak
sailfishos · Jun 10, 2021 · 33c067a · 33c067a
1 parent 29616c0
commit 33c067a
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 13 deletions.
diff --git a/ofono/plugins/sailfish_manager/sailfish_cell_info_dbus.c b/ofono/plugins/sailfish_manager/sailfish_cell_info_dbus.c
@@ -341,7 +341,7 @@ static void sailfish_cell_info_dbus_emit_path_list
 		}
 		dbus_message_iter_close_container(&it, &a);
 		ofono_dbus_clients_signal(dbus->clients, signal);
-		dbus_message_ref(signal);
+		dbus_message_unref(signal);
 	}
 }
 
@@ -372,6 +372,23 @@ static int sailfish_cell_info_dbus_compare(const struct sailfish_cell *c1,
 	}
 }
 
+static void sailfish_cell_info_dbus_emit_signal
+		(struct sailfish_cell_info_dbus *dbus,
+				const char *path, const char *intf,
+				const char *name, int type, ...)
+{
+	if (ofono_dbus_clients_count(dbus->clients)) {
+		va_list args;
+		DBusMessage *signal = dbus_message_new_signal(path, intf, name);
+
+		va_start(args, type);
+		dbus_message_append_args_valist(signal, type, args);
+		ofono_dbus_clients_signal(dbus->clients, signal);
+		dbus_message_unref(signal);
+		va_end(args);
+	}
+}
+
 static void sailfish_cell_info_dbus_property_changed
 		(struct sailfish_cell_info_dbus *dbus,
 			const struct sailfish_cell_entry *entry, int mask)
@@ -383,7 +400,8 @@ static void sailfish_cell_info_dbus_property_changed
 
 	if (mask & SAILFISH_CELL_PROPERTY_REGISTERED) {
 		const dbus_bool_t registered = (cell->registered != FALSE);
-		g_dbus_emit_signal(dbus->conn, entry->path,
+
+		sailfish_cell_info_dbus_emit_signal(dbus, entry->path,
 			CELL_DBUS_INTERFACE,
 			CELL_DBUS_REGISTERED_CHANGED_SIGNAL,
 			DBUS_TYPE_BOOLEAN, &registered, DBUS_TYPE_INVALID);
@@ -418,7 +436,7 @@ static void sailfish_cell_info_dbus_update_entries
 						sailfish_cell_compare_func)) {
 			DBG("%s removed", entry->path);
 			dbus->entries = g_slist_delete_link(dbus->entries, l);
-			g_dbus_emit_signal(dbus->conn, entry->path,
+			sailfish_cell_info_dbus_emit_signal(dbus, entry->path,
 					CELL_DBUS_INTERFACE,
 					CELL_DBUS_REMOVED_SIGNAL,
 					DBUS_TYPE_INVALID);
@@ -544,6 +562,9 @@ static DBusMessage *sailfish_cell_info_dbus_unsubscribe(DBusConnection *conn,
 			CELL_INFO_DBUS_INTERFACE,
 			CELL_INFO_DBUS_UNSUBSCRIBED_SIGNAL);
 
+		if (!ofono_dbus_clients_count(dbus->clients)) {
+			sailfish_cell_info_set_enabled(dbus->info, FALSE);
+		}
 		dbus_message_set_destination(msg, sender);
 		g_dbus_send_message(dbus->conn, signal);
 		return dbus_message_new_method_return(msg);

diff --git a/ofono/unit/test-sailfish_cell_info_dbus.c b/ofono/unit/test-sailfish_cell_info_dbus.c
@@ -827,7 +827,7 @@ struct test_registered_changed_data {
 	const char *cell_path;
 };
 
-static void test_registered_changed_reply(DBusPendingCall *call, void *data)
+static void test_registered_changed_reply2(DBusPendingCall *call, void *data)
 {
 	struct test_registered_changed_data *test = data;
 
@@ -838,26 +838,39 @@ static void test_registered_changed_reply(DBusPendingCall *call, void *data)
 	test_loop_quit_later(test->context.loop);
 }
 
-static void test_registered_changed_start(struct test_dbus_context *context)
+static void test_registered_changed_reply1(DBusPendingCall *call, void *data)
 {
-	struct test_registered_changed_data *test =
-		G_CAST(context, struct test_registered_changed_data, context);
+	struct test_registered_changed_data *test = data;
 	struct sailfish_cell *first_cell;
 
 	DBG("");
-	test->info = fake_cell_info_new();
-	fake_cell_info_add_cell(test->info, &test->cell);
-	test->dbus = sailfish_cell_info_dbus_new(&test->modem, test->info);
-	g_assert(test->dbus);
+	test_check_get_cells_reply(call, test->cell_path, NULL);
+	dbus_pending_call_unref(call);
 
 	/* Trigger "RegisteredChanged" signal */
 	first_cell = test->info->cells->data;
 	test->cell.registered =
 	first_cell->registered = !first_cell->registered;
 	fake_cell_info_cells_changed(test->info);
 
-	test_submit_get_all_call(context->client_connection, test->cell_path,
-				test_registered_changed_reply, test);
+	test_submit_get_all_call(test->context.client_connection,
+		test->cell_path, test_registered_changed_reply2, test);
+}
+
+static void test_registered_changed_start(struct test_dbus_context *context)
+{
+	struct test_registered_changed_data *test =
+		G_CAST(context, struct test_registered_changed_data, context);
+
+	DBG("");
+	test->info = fake_cell_info_new();
+	fake_cell_info_add_cell(test->info, &test->cell);
+	test->dbus = sailfish_cell_info_dbus_new(&test->modem, test->info);
+	g_assert(test->dbus);
+
+	/* Submit GetCells to enable "RegisteredChanged" signals */
+	test_submit_cell_info_call(test->context.client_connection, "GetCells",
+					test_registered_changed_reply1, test);
 }
 
 static void test_registered_changed(void)