From 30210c7087bd635e6a84db33b37d10d83684ebad Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 19 Oct 2020 17:44:50 +0100 Subject: [PATCH] a11y: Move ATContext to an explicit realization model We are doing too much work during the construction phase of the AT-SPI backend for GtkATContext. Instead of having the AtSpiContext register itself at construction time, let's add explicit realize and unrealize operations, and connect the ATContext realization to the rooting operation of a GtkWidget. --- gtk/a11y/gtkatspicontext.c | 192 ++++++++++++++++++++++----------- gtk/a11y/gtkatspiroot.c | 46 ++++++-- gtk/a11y/gtkatspirootprivate.h | 3 + gtk/gtkatcontext.c | 54 ++++++++++ gtk/gtkatcontextprivate.h | 9 ++ gtk/gtkwidget.c | 13 +++ 6 files changed, 244 insertions(+), 73 deletions(-) diff --git a/gtk/a11y/gtkatspicontext.c b/gtk/a11y/gtkatspicontext.c index 7bcdc57862..750b759af4 100644 --- a/gtk/a11y/gtkatspicontext.c +++ b/gtk/a11y/gtkatspicontext.c @@ -307,11 +307,8 @@ collect_relations (GtkAtSpiContext *self, GtkAccessibleValue *value; GList *list, *l; GtkATContext *target_ctx; - const char *unique_name; int i; - unique_name = g_dbus_connection_get_unique_name (self->connection); - for (i = 0; i < G_N_ELEMENTS (map); i++) { if (!gtk_at_context_has_accessible_relation (ctx, map[i].r)) @@ -325,9 +322,12 @@ collect_relations (GtkAtSpiContext *self, for (l = list; l; l = l->next) { target_ctx = gtk_accessible_get_at_context (GTK_ACCESSIBLE (l->data)); - g_variant_builder_add (&b, "(so)", - unique_name, - GTK_AT_SPI_CONTEXT (target_ctx)->context_path); + + /* Realize the ATContext of the target, so we can ask for its ref */ + gtk_at_context_realize (target_ctx); + + g_variant_builder_add (&b, "@(so)", + gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (target_ctx))); } g_variant_builder_add (builder, "(ua(so))", map[i].s, &b); @@ -457,8 +457,6 @@ handle_accessible_method (GDBusConnection *connection, GtkATContext *context = NULL; GtkAccessible *accessible; int idx, real_idx = 0; - const char *name; - const char *path; g_variant_get (parameters, "(i)", &idx); @@ -512,10 +510,12 @@ handle_accessible_method (GDBusConnection *connection, return; } - name = g_dbus_connection_get_unique_name (self->connection); - path = gtk_at_spi_context_get_context_path (GTK_AT_SPI_CONTEXT (context)); + /* Realize the child ATContext in order to get its ref */ + gtk_at_context_realize (context); - g_dbus_method_invocation_return_value (invocation, g_variant_new ("((so))", name, path)); + GVariant *ref = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (context)); + + g_dbus_method_invocation_return_value (invocation, g_variant_new ("(@(so))", ref)); } else if (g_strcmp0 (method_name, "GetChildren") == 0) { @@ -536,10 +536,13 @@ handle_accessible_method (GDBusConnection *connection, GtkATContext *context = gtk_accessible_get_at_context (GTK_ACCESSIBLE (child)); - const char *name = g_dbus_connection_get_unique_name (self->connection); - const char *path = gtk_at_spi_context_get_context_path (GTK_AT_SPI_CONTEXT (context)); + /* Realize the child ATContext in order to get its ref */ + gtk_at_context_realize (context); - g_variant_builder_add (&builder, "(so)", name, path); + GVariant *ref = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (context)); + + if (ref != NULL) + g_variant_builder_add (&builder, "@(so)", ref); } } else if (GTK_IS_STACK_PAGE (accessible)) @@ -549,9 +552,14 @@ handle_accessible_method (GDBusConnection *connection, if (gtk_accessible_should_present (GTK_ACCESSIBLE (child))) { GtkATContext *context = gtk_accessible_get_at_context (GTK_ACCESSIBLE (child)); - const char *name = g_dbus_connection_get_unique_name (self->connection); - const char *path = gtk_at_spi_context_get_context_path (GTK_AT_SPI_CONTEXT (context)); - g_variant_builder_add (&builder, "(so)", name, path); + + /* Realize the child ATContext in order to get its ref */ + gtk_at_context_realize (context); + + GVariant *ref = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (context)); + + if (ref != NULL) + g_variant_builder_add (&builder, "@(so)", ref); } } @@ -579,7 +587,7 @@ handle_accessible_method (GDBusConnection *connection, else if (g_strcmp0 (method_name, "GetRelationSet") == 0) { GVariantBuilder builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE ("a(ua(so))")); - collect_relations (self, &builder); + collect_relations (self, &builder); g_dbus_method_invocation_return_value (invocation, g_variant_new ("(a(ua(so)))", &builder)); } else if (g_strcmp0 (method_name, "GetInterfaces") == 0) @@ -637,9 +645,7 @@ handle_accessible_get_property (GDBusConnection *connection, gtk_accessible_get_at_context (GTK_ACCESSIBLE (page)); if (parent_context != NULL) - res = g_variant_new ("(so)", - g_dbus_connection_get_unique_name (self->connection), - GTK_AT_SPI_CONTEXT (parent_context)->context_path); + res = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (parent_context)); } else { @@ -647,9 +653,7 @@ handle_accessible_get_property (GDBusConnection *connection, gtk_accessible_get_at_context (GTK_ACCESSIBLE (parent)); if (parent_context != NULL) - res = g_variant_new ("(so)", - g_dbus_connection_get_unique_name (self->connection), - GTK_AT_SPI_CONTEXT (parent_context)->context_path); + res = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (parent_context)); } } else if (GTK_IS_STACK_PAGE (accessible)) @@ -659,9 +663,7 @@ handle_accessible_get_property (GDBusConnection *connection, gtk_accessible_get_at_context (GTK_ACCESSIBLE (parent)); if (parent_context != NULL) - res = g_variant_new ("(so)", - g_dbus_connection_get_unique_name (self->connection), - GTK_AT_SPI_CONTEXT (parent_context)->context_path); + res = gtk_at_spi_context_to_ref (GTK_AT_SPI_CONTEXT (parent_context)); } if (res == NULL) @@ -713,6 +715,9 @@ emit_text_changed (GtkAtSpiContext *self, int end, const char *text) { + if (self->connection == NULL) + return; + g_dbus_connection_emit_signal (self->connection, NULL, self->context_path, @@ -728,6 +733,9 @@ emit_text_selection_changed (GtkAtSpiContext *self, const char *kind, int cursor_position) { + if (self->connection == NULL) + return; + if (strcmp (kind, "text-caret-moved") == 0) g_dbus_connection_emit_signal (self->connection, NULL, @@ -752,6 +760,9 @@ static void emit_selection_changed (GtkAtSpiContext *self, const char *kind) { + if (self->connection == NULL) + return; + g_dbus_connection_emit_signal (self->connection, NULL, self->context_path, @@ -767,6 +778,9 @@ emit_state_changed (GtkAtSpiContext *self, const char *name, gboolean enabled) { + if (self->connection == NULL) + return; + g_dbus_connection_emit_signal (self->connection, NULL, self->context_path, @@ -777,11 +791,29 @@ emit_state_changed (GtkAtSpiContext *self, NULL); } +static void +emit_defunct (GtkAtSpiContext *self) +{ + if (self->connection == NULL) + return; + + g_dbus_connection_emit_signal (self->connection, + NULL, + self->context_path, + "org.a11y.atspi.Event.Object", + "StateChanged", + g_variant_new ("(siiva{sv})", "defunct", TRUE, 0, g_variant_new_string ("0"), NULL), + NULL); +} + static void emit_property_changed (GtkAtSpiContext *self, const char *name, GVariant *value) { + if (self->connection == NULL) + return; + g_dbus_connection_emit_signal (self->connection, NULL, self->context_path, @@ -799,6 +831,9 @@ emit_bounds_changed (GtkAtSpiContext *self, int width, int height) { + if (self->connection == NULL) + return; + g_dbus_connection_emit_signal (self->connection, NULL, self->context_path, @@ -815,8 +850,12 @@ emit_children_changed (GtkAtSpiContext *self, int idx, GtkAccessibleChildState state) { - GVariant *child_ref = gtk_at_spi_context_to_ref (child_context); + /* If we don't have a connection on either contexts, we cannot emit a signal */ + if (self->connection == NULL || child_context->connection == NULL) + return; + GVariant *context_ref = gtk_at_spi_context_to_ref (self); + GVariant *child_ref = gtk_at_spi_context_to_ref (child_context); gtk_at_spi_emit_children_changed (self->connection, self->context_path, @@ -1312,6 +1351,10 @@ gtk_at_spi_context_register_object (GtkAtSpiContext *self) } self->interfaces = g_variant_ref_sink (g_variant_builder_end (&interfaces)); + + GTK_NOTE (A11Y, g_message ("Registered %d interfaces on object path '%s'", + self->n_registered_objects, + self->context_path)); } static void @@ -1327,24 +1370,13 @@ gtk_at_spi_context_unregister_object (GtkAtSpiContext *self) } /* }}} */ /* {{{ GObject boilerplate */ -static void -gtk_at_spi_context_dispose (GObject *gobject) -{ - GtkAtSpiContext *self = GTK_AT_SPI_CONTEXT (gobject); - GtkAccessible *accessible = gtk_at_context_get_accessible (GTK_AT_CONTEXT (self)); - - gtk_at_spi_context_unregister_object (self); - gtk_atspi_disconnect_text_signals (accessible); - gtk_atspi_disconnect_selection_signals (accessible); - - G_OBJECT_CLASS (gtk_at_spi_context_parent_class)->dispose (gobject); -} - static void gtk_at_spi_context_finalize (GObject *gobject) { GtkAtSpiContext *self = GTK_AT_SPI_CONTEXT (gobject); + gtk_at_spi_context_unregister_object (self); + g_free (self->bus_address); g_free (self->context_path); g_clear_pointer (&self->interfaces, g_variant_unref); @@ -1394,29 +1426,10 @@ static void gtk_at_spi_context_constructed (GObject *gobject) { GtkAtSpiContext *self = GTK_AT_SPI_CONTEXT (gobject); - GdkDisplay *display; + GdkDisplay *display = gtk_at_context_get_display (GTK_AT_CONTEXT (self)); g_assert (self->bus_address); - /* Every GTK application has a single root AT-SPI object, which - * handles all the global state, including the cache of accessible - * objects. We use the GdkDisplay to store it, so it's guaranteed - * to be unique per-display connection - */ - display = gtk_at_context_get_display (GTK_AT_CONTEXT (self)); - self->root = - g_object_get_data (G_OBJECT (display), "-gtk-atspi-root"); - - if (self->root == NULL) - { - self->root = gtk_at_spi_root_new (self->bus_address); - g_object_set_data_full (G_OBJECT (display), "-gtk-atspi-root", - self->root, - g_object_unref); - } - - self->connection = gtk_at_spi_root_get_connection (self->root); - /* We use the application's object path to build the path of each * accessible object exposed on the accessibility bus; the path is * also used to access the object cache @@ -1456,7 +1469,39 @@ gtk_at_spi_context_constructed (GObject *gobject) g_free (base_path); g_free (uuid); - GtkAccessible *accessible = gtk_at_context_get_accessible (GTK_AT_CONTEXT (self)); + /* Every GTK application has a single root AT-SPI object, which + * handles all the global state, including the cache of accessible + * objects. We use the GdkDisplay to store it, so it's guaranteed + * to be unique per-display connection + */ + self->root = + g_object_get_data (G_OBJECT (display), "-gtk-atspi-root"); + + if (self->root == NULL) + { + self->root = gtk_at_spi_root_new (self->bus_address); + g_object_set_data_full (G_OBJECT (display), "-gtk-atspi-root", + self->root, + g_object_unref); + } + + G_OBJECT_CLASS (gtk_at_spi_context_parent_class)->constructed (gobject); +} + +static void +gtk_at_spi_context_realize (GtkATContext *context) +{ + GtkAtSpiContext *self = GTK_AT_SPI_CONTEXT (context); + + self->connection = gtk_at_spi_root_get_connection (self->root); + if (self->connection == NULL) + return; + + GtkAccessible *accessible = gtk_at_context_get_accessible (context); + GTK_NOTE (A11Y, g_message ("Realizing ATSPI context at '%s' for accessible '%s'", + self->context_path, + G_OBJECT_TYPE_NAME (accessible))); + gtk_atspi_connect_text_signals (accessible, (GtkAtspiTextChangedCallback *)emit_text_changed, (GtkAtspiTextSelectionCallback *)emit_text_selection_changed, @@ -1466,7 +1511,25 @@ gtk_at_spi_context_constructed (GObject *gobject) self); gtk_at_spi_context_register_object (self); - G_OBJECT_CLASS (gtk_at_spi_context_parent_class)->constructed (gobject); + gtk_at_spi_root_queue_register (self->root); +} + +static void +gtk_at_spi_context_unrealize (GtkATContext *context) +{ + GtkAtSpiContext *self = GTK_AT_SPI_CONTEXT (context); + GtkAccessible *accessible = gtk_at_context_get_accessible (context); + + GTK_NOTE (A11Y, g_message ("Unrealizing ATSPI context at '%s' for accessible '%s'", + self->context_path, + G_OBJECT_TYPE_NAME (accessible))); + + /* Notify ATs that the accessible object is going away */ + emit_defunct (self); + + gtk_atspi_disconnect_text_signals (accessible); + gtk_atspi_disconnect_selection_signals (accessible); + gtk_at_spi_context_unregister_object (self); } static void @@ -1479,8 +1542,9 @@ gtk_at_spi_context_class_init (GtkAtSpiContextClass *klass) gobject_class->set_property = gtk_at_spi_context_set_property; gobject_class->get_property = gtk_at_spi_context_get_property; gobject_class->finalize = gtk_at_spi_context_finalize; - gobject_class->dispose = gtk_at_spi_context_dispose; + context_class->realize = gtk_at_spi_context_realize; + context_class->unrealize = gtk_at_spi_context_unrealize; context_class->state_change = gtk_at_spi_context_state_change; context_class->platform_change = gtk_at_spi_context_platform_change; context_class->bounds_change = gtk_at_spi_context_bounds_change; diff --git a/gtk/a11y/gtkatspiroot.c b/gtk/a11y/gtkatspiroot.c index c4c09bdae1..e9a174990e 100644 --- a/gtk/a11y/gtkatspiroot.c +++ b/gtk/a11y/gtkatspiroot.c @@ -61,6 +61,7 @@ struct _GtkAtSpiRoot char *desktop_path; gint32 application_id; + guint register_id; GtkAtSpiCache *cache; @@ -83,6 +84,8 @@ gtk_at_spi_root_finalize (GObject *gobject) { GtkAtSpiRoot *self = GTK_AT_SPI_ROOT (gobject); + g_clear_handle_id (&self->register_id, g_source_remove); + g_free (self->bus_address); g_free (self->desktop_name); g_free (self->desktop_path); @@ -499,9 +502,12 @@ on_registration_reply (GObject *gobject, self->toplevels = gtk_window_get_toplevels (); } -static void -gtk_at_spi_root_register (GtkAtSpiRoot *self) +static gboolean +root_register (gpointer data) { + GtkAtSpiRoot *self = data; + const char *unique_name; + /* Register the root element; every application has a single root, so we only * need to do this once. * @@ -524,6 +530,8 @@ gtk_at_spi_root_register (GtkAtSpiRoot *self) self->atspi_version = ATSPI_VERSION; self->root_path = ATSPI_ROOT_PATH; + unique_name = g_dbus_connection_get_unique_name (self->connection); + g_dbus_connection_register_object (self->connection, self->root_path, (GDBusInterfaceInfo *) &atspi_application_interface, @@ -540,7 +548,7 @@ gtk_at_spi_root_register (GtkAtSpiRoot *self) NULL); GTK_NOTE (A11Y, g_message ("Registering (%s, %s) on the a11y bus", - g_dbus_connection_get_unique_name (self->connection), + unique_name, self->root_path)); g_dbus_connection_call (self->connection, @@ -548,15 +556,37 @@ gtk_at_spi_root_register (GtkAtSpiRoot *self) ATSPI_ROOT_PATH, "org.a11y.atspi.Socket", "Embed", - g_variant_new ("((so))", - g_dbus_connection_get_unique_name (self->connection), - self->root_path - ), + g_variant_new ("((so))", unique_name, self->root_path), G_VARIANT_TYPE ("((so))"), G_DBUS_CALL_FLAGS_NONE, -1, NULL, on_registration_reply, self); + + self->register_id = 0; + + return G_SOURCE_REMOVE; +} + +/*< private > + * gtk_at_spi_root_queue_register: + * @self: a #GtkAtSpiRoot + * + * Queues the registration of the root object on the AT-SPI bus. + */ +void +gtk_at_spi_root_queue_register (GtkAtSpiRoot *self) +{ + /* Ignore multiple registration requests while one is already in flight */ + if (self->register_id != 0) + return; + + /* The cache is only available once the registration succeeds */ + if (self->cache != NULL) + return; + + self->register_id = g_idle_add (root_register, self); + g_source_set_name_by_id (self->register_id, "[gtk] ATSPI root registration"); } static void @@ -583,8 +613,6 @@ gtk_at_spi_root_constructed (GObject *gobject) goto out; } - gtk_at_spi_root_register (self); - out: G_OBJECT_CLASS (gtk_at_spi_root_parent_class)->constructed (gobject); } diff --git a/gtk/a11y/gtkatspirootprivate.h b/gtk/a11y/gtkatspirootprivate.h index f5ae272999..7a99e7c254 100644 --- a/gtk/a11y/gtkatspirootprivate.h +++ b/gtk/a11y/gtkatspirootprivate.h @@ -33,6 +33,9 @@ G_DECLARE_FINAL_TYPE (GtkAtSpiRoot, gtk_at_spi_root, GTK, AT_SPI_ROOT, GObject) GtkAtSpiRoot * gtk_at_spi_root_new (const char *bus_address); +void +gtk_at_spi_root_queue_register (GtkAtSpiRoot *self); + GDBusConnection * gtk_at_spi_root_get_connection (GtkAtSpiRoot *self); diff --git a/gtk/gtkatcontext.c b/gtk/gtkatcontext.c index 6e0c54693f..078dc2872d 100644 --- a/gtk/gtkatcontext.c +++ b/gtk/gtkatcontext.c @@ -162,6 +162,16 @@ gtk_at_context_real_child_change (GtkATContext *self, { } +static void +gtk_at_context_real_realize (GtkATContext *self) +{ +} + +static void +gtk_at_context_real_unrealize (GtkATContext *self) +{ +} + static void gtk_at_context_class_init (GtkATContextClass *klass) { @@ -171,6 +181,8 @@ gtk_at_context_class_init (GtkATContextClass *klass) gobject_class->get_property = gtk_at_context_get_property; gobject_class->finalize = gtk_at_context_finalize; + klass->realize = gtk_at_context_real_realize; + klass->unrealize = gtk_at_context_real_unrealize; klass->state_change = gtk_at_context_real_state_change; klass->platform_change = gtk_at_context_real_platform_change; klass->bounds_change = gtk_at_context_real_bounds_change; @@ -511,6 +523,36 @@ gtk_at_context_create (GtkAccessibleRole accessible_role, return res; } +gboolean +gtk_at_context_is_realized (GtkATContext *self) +{ + return self->realized; +} + +void +gtk_at_context_realize (GtkATContext *self) +{ + if (self->realized) + return; + + GTK_NOTE (A11Y, g_message ("Realizing AT context '%s'", G_OBJECT_TYPE_NAME (self))); + GTK_AT_CONTEXT_GET_CLASS (self)->realize (self); + + self->realized = TRUE; +} + +void +gtk_at_context_unrealize (GtkATContext *self) +{ + if (!self->realized) + return; + + GTK_NOTE (A11Y, g_message ("Unrealizing AT context '%s'", G_OBJECT_TYPE_NAME (self))); + GTK_AT_CONTEXT_GET_CLASS (self)->unrealize (self); + + self->realized = FALSE; +} + /*< private > * gtk_at_context_update: * @self: a #GtkATContext @@ -523,6 +565,9 @@ gtk_at_context_update (GtkATContext *self) { g_return_if_fail (GTK_IS_AT_CONTEXT (self)); + if (!self->realized) + return; + /* There's no point in notifying of state changes if there weren't any */ if (self->updated_properties == 0 && self->updated_relations == 0 && @@ -979,12 +1024,18 @@ void gtk_at_context_platform_changed (GtkATContext *self, GtkAccessiblePlatformChange change) { + if (!self->realized) + return; + GTK_AT_CONTEXT_GET_CLASS (self)->platform_change (self, change); } void gtk_at_context_bounds_changed (GtkATContext *self) { + if (!self->realized) + return; + GTK_AT_CONTEXT_GET_CLASS (self)->bounds_change (self); } @@ -993,5 +1044,8 @@ gtk_at_context_child_changed (GtkATContext *self, GtkAccessibleChildChange change, GtkAccessible *child) { + if (!self->realized) + return; + GTK_AT_CONTEXT_GET_CLASS (self)->child_change (self, change, child); } diff --git a/gtk/gtkatcontextprivate.h b/gtk/gtkatcontextprivate.h index 85d897be90..9f161e9f20 100644 --- a/gtk/gtkatcontextprivate.h +++ b/gtk/gtkatcontextprivate.h @@ -116,6 +116,8 @@ struct _GtkATContext GtkAccessiblePropertyChange updated_properties; GtkAccessibleRelationChange updated_relations; GtkAccessiblePlatformChange updated_platform; + + guint realized : 1; }; struct _GtkATContextClass @@ -138,10 +140,17 @@ struct _GtkATContextClass void (* child_change) (GtkATContext *self, GtkAccessibleChildChange changed_child, GtkAccessible *child); + + void (* realize) (GtkATContext *self); + void (* unrealize) (GtkATContext *self); }; GdkDisplay * gtk_at_context_get_display (GtkATContext *self); +void gtk_at_context_realize (GtkATContext *self); +void gtk_at_context_unrealize (GtkATContext *self); +gboolean gtk_at_context_is_realized (GtkATContext *self); + void gtk_at_context_update (GtkATContext *self); void gtk_at_context_set_accessible_state (GtkATContext *self, diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 31c0063022..36c8f95ade 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -2346,6 +2346,16 @@ gtk_widget_root (GtkWidget *widget) if (priv->layout_manager) gtk_layout_manager_set_root (priv->layout_manager, priv->root); + if (priv->at_context != NULL) + { + GtkATContext *root_context = gtk_accessible_get_at_context (GTK_ACCESSIBLE (priv->root)); + + if (root_context) + gtk_at_context_realize (root_context); + + gtk_at_context_realize (priv->at_context); + } + GTK_WIDGET_GET_CLASS (widget)->root (widget); if (!GTK_IS_ROOT (widget)) @@ -2370,6 +2380,9 @@ gtk_widget_unroot (GtkWidget *widget) GTK_WIDGET_GET_CLASS (widget)->unroot (widget); + if (priv->at_context) + gtk_at_context_unrealize (priv->at_context); + if (priv->context) gtk_style_context_set_display (priv->context, gdk_display_get_default ());