From f7e228dff4ca3781f686b1fd39e862ed5e5edc79 Mon Sep 17 00:00:00 2001 From: michael Date: Fri, 16 Aug 2002 14:44:25 +0000 Subject: [PATCH] 2002-08-12 Michael Meeks * test/test-simple.c (global_listener_cb): bin bogosity. (test_keylisteners): disable, still doesn't work reliably, certainly not on my system anyway. * atk-bridge/bridge.c (spi_atk_bridge_key_listener): don't leak a reference on the DEC. This round-trip fetching of the DEC per keystroke sucks, it should be cached. * cspi/spi-private.h, * cspi/cspi-lowlevel.h, * cspi/bonobo/cspi-bonobo-listener.[ch], * cspi/bonobo/cspi-bonobo.c: get the copyright notices better - there is still a large amount of work in at-spi falsely attributed solely to Sun. * cspi/spi_main.c (cspi_object_ref): kill bogus hash lookup, just increment the ref. (SPI_freeString): make explicit the fact that we handle NULL strings just fine. (report_leaked_ref): obey coding standards. (cspi_object_hash, cspi_object_equal): kill retval. (cspi_object_release): only release if not on loan. (cspi_object_get_ref): add 'loan' concept, bin 'do_ref'. (cspi_object_borrow, cspi_object_return): impl. * cspi/bonobo/cspi-bonobo-listener.c (cspi_event): use cspi_object_borrow / return. git-svn-id: http://svn.gnome.org/svn/at-spi/trunk@331 e2bd861d-eb25-0410-b326-f6ed22b6b98c --- ChangeLog | 31 +++++ atk-bridge/bridge.c | 1 + cspi/bonobo/cspi-bonobo-listener.c | 6 +- cspi/bonobo/cspi-bonobo-listener.h | 2 +- cspi/bonobo/cspi-bonobo.c | 1 + cspi/cspi-lowlevel.h | 2 +- cspi/spi-private.h | 21 ++-- cspi/spi_main.c | 164 +++++++++++++++------------ cspi/spi_text.c | 3 +- docs/reference/cspi/tmpl/spi_accessible.sgml | 1 + test/test-simple.c | 17 +-- 11 files changed, 149 insertions(+), 100 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5da563c..b7f6f0c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,34 @@ +2002-08-12 Michael Meeks + + * test/test-simple.c (global_listener_cb): bin bogosity. + (test_keylisteners): disable, still doesn't work reliably, + certainly not on my system anyway. + + * atk-bridge/bridge.c (spi_atk_bridge_key_listener): + don't leak a reference on the DEC. This round-trip + fetching of the DEC per keystroke sucks, it should be + cached. + + * cspi/spi-private.h, + * cspi/cspi-lowlevel.h, + * cspi/bonobo/cspi-bonobo-listener.[ch], + * cspi/bonobo/cspi-bonobo.c: get the copyright + notices better - there is still a large amount of + work in at-spi falsely attributed solely to Sun. + + * cspi/spi_main.c (cspi_object_ref): kill bogus + hash lookup, just increment the ref. + (SPI_freeString): make explicit the fact that we + handle NULL strings just fine. + (report_leaked_ref): obey coding standards. + (cspi_object_hash, cspi_object_equal): kill retval. + (cspi_object_release): only release if not on loan. + (cspi_object_get_ref): add 'loan' concept, bin 'do_ref'. + (cspi_object_borrow, cspi_object_return): impl. + + * cspi/bonobo/cspi-bonobo-listener.c (cspi_event): + use cspi_object_borrow / return. + 2002-08-12 Darren Kenny * cspi/bonobo/cspi-bonobo-listener.c: diff --git a/atk-bridge/bridge.c b/atk-bridge/bridge.c index d094d59..cb46eaa 100644 --- a/atk-bridge/bridge.c +++ b/atk-bridge/bridge.c @@ -575,6 +575,7 @@ spi_atk_bridge_key_listener (AtkKeyEventStruct *event, gpointer data) result = Accessibility_DeviceEventController_notifyListenersSync ( controller, &key_event, &ev); + bonobo_object_release_unref (controller, &ev); CORBA_exception_free (&ev); } diff --git a/cspi/bonobo/cspi-bonobo-listener.c b/cspi/bonobo/cspi-bonobo-listener.c index 1167ffd..d113597 100644 --- a/cspi/bonobo/cspi-bonobo-listener.c +++ b/cspi/bonobo/cspi-bonobo-listener.c @@ -2,7 +2,7 @@ * AT-SPI - Assistive Technology Service Provider Interface * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap) * - * Copyright 2001 Sun Microsystems Inc. + * Copyright 2002 Ximian Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public @@ -91,7 +91,7 @@ cspi_event (SpiEventListener *listener, GList *l; CSpiEventListener *clistener = (CSpiEventListener *) listener; AccessibleEvent aevent; - Accessible *source = cspi_object_add (cspi_dup_ref(event->source)); + Accessible *source = cspi_object_borrow (event->source); aevent.type = event->type; aevent.source = source; @@ -106,7 +106,7 @@ cspi_event (SpiEventListener *listener, eh->cb.event (&aevent, eh->user_data); } - cspi_object_unref( source ); + cspi_object_return (source); } static void diff --git a/cspi/bonobo/cspi-bonobo-listener.h b/cspi/bonobo/cspi-bonobo-listener.h index aee946f..58c818b 100644 --- a/cspi/bonobo/cspi-bonobo-listener.h +++ b/cspi/bonobo/cspi-bonobo-listener.h @@ -2,7 +2,7 @@ * AT-SPI - Assistive Technology Service Provider Interface * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap) * - * Copyright 2001 Sun Microsystems Inc. + * Copyright 2002 Ximian, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public diff --git a/cspi/bonobo/cspi-bonobo.c b/cspi/bonobo/cspi-bonobo.c index 0d94057..ff239f9 100644 --- a/cspi/bonobo/cspi-bonobo.c +++ b/cspi/bonobo/cspi-bonobo.c @@ -3,6 +3,7 @@ * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap) * * Copyright 2001 Sun Microsystems Inc. + * 2002 Ximian Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public diff --git a/cspi/cspi-lowlevel.h b/cspi/cspi-lowlevel.h index 9c5a0c4..cda8931 100644 --- a/cspi/cspi-lowlevel.h +++ b/cspi/cspi-lowlevel.h @@ -2,7 +2,7 @@ * AT-SPI - Assistive Technology Service Provider Interface * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap) * - * Copyright 2001 Sun Microsystems Inc. + * Copyright 2002 Ximian, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public diff --git a/cspi/spi-private.h b/cspi/spi-private.h index 7ed7d6e..dae71f8 100644 --- a/cspi/spi-private.h +++ b/cspi/spi-private.h @@ -2,7 +2,9 @@ * AT-SPI - Assistive Technology Service Provider Interface * (Gnome Accessibility Project; http://developer.gnome.org/projects/gap) * - * Copyright 2001 Sun Microsystems Inc. + * Copyright 2002 Ximian, Inc. + * 2002 Sun Microsystems Inc. + * * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Library General Public @@ -33,7 +35,8 @@ struct _Accessible { CORBA_Object objref; /* And some other bits */ - guint ref_count; + guint on_loan : 1; + guint ref_count : 30; }; #define CSPI_OBJREF(a) (((Accessible *)(a))->objref) @@ -41,13 +44,13 @@ struct _Accessible { CORBA_Environment *cspi_ev (void); SPIBoolean cspi_exception (void); Accessibility_Registry cspi_registry (void); -Accessible *cspi_object_add (CORBA_Object corba_object); -Accessible *cspi_object_add_ref (CORBA_Object corba_object, - gboolean do_ref); -void cspi_object_ref (Accessible *accessible); -void cspi_object_unref (Accessible *accessible); -SPIBoolean cspi_accessible_is_a (Accessible *obj, - const char *interface_name); +Accessible *cspi_object_add (CORBA_Object corba_object); +void cspi_object_ref (Accessible *accessible); +void cspi_object_unref (Accessible *accessible); +Accessible *cspi_object_borrow (CORBA_Object corba_object); +void cspi_object_return (Accessible *accessible); +SPIBoolean cspi_accessible_is_a (Accessible *accessible, + const char *interface_name); #define cspi_return_if_fail(val) \ if (!(val)) \ diff --git a/cspi/spi_main.c b/cspi/spi_main.c index 21ab64a..6d0efd1 100644 --- a/cspi/spi_main.c +++ b/cspi/spi_main.c @@ -39,11 +39,8 @@ static guint cspi_object_hash (gconstpointer key) { CORBA_Object object = (CORBA_Object) key; - guint retval; - - retval = CORBA_Object_hash (object, 0, &ev); - return retval; + return CORBA_Object_hash (object, 0, &ev); } static gboolean @@ -51,15 +48,12 @@ cspi_object_equal (gconstpointer a, gconstpointer b) { CORBA_Object objecta = (CORBA_Object) a; CORBA_Object objectb = (CORBA_Object) b; - gboolean retval; - - retval = CORBA_Object_is_equivalent (objecta, objectb, &ev); - return retval; + return CORBA_Object_is_equivalent (objecta, objectb, &ev); } static void -cspi_object_release (gpointer value) +cspi_object_release (gpointer value) { Accessible *a = (Accessible *) value; @@ -67,7 +61,10 @@ cspi_object_release (gpointer value) g_print ("releasing %p => %p\n", a, a->objref); #endif - cspi_release_unref (a->objref); + if (!a->on_loan) + { + cspi_release_unref (a->objref); + } memset (a, 0xaa, sizeof (Accessible)); a->ref_count = -1; @@ -78,18 +75,18 @@ cspi_object_release (gpointer value) } SPIBoolean -cspi_accessible_is_a (Accessible *obj, +cspi_accessible_is_a (Accessible *accessible, const char *interface_name) { SPIBoolean retval; Bonobo_Unknown unknown; - if (obj == NULL) + if (accessible == NULL) { return FALSE; } - unknown = Bonobo_Unknown_queryInterface (CSPI_OBJREF (obj), + unknown = Bonobo_Unknown_queryInterface (CSPI_OBJREF (accessible), interface_name, cspi_ev ()); if (ev._major != CORBA_NO_EXCEPTION) @@ -135,7 +132,9 @@ Accessibility_Registry cspi_registry (void) { if (!cspi_ping (registry)) - registry = cspi_init (); + { + registry = cspi_init (); + } return registry; } @@ -157,38 +156,14 @@ cspi_exception (void) return retval; } -Accessible * -cspi_object_new (CORBA_Object corba_object) -{ - Accessible *ref; - - if (corba_object == CORBA_OBJECT_NIL) - { - ref = NULL; - } - else if (!cspi_check_ev ("pre method check: add")) - { - ref = NULL; - } - else - { - ref = malloc (sizeof (Accessible)); -// ref->objref = CORBA_Object_duplicate (corba_object, cspi_ev ()); - ref->objref = corba_object; - ref->ref_count = 1; - } - - return ref; -} - -Accessible * -cspi_object_add (CORBA_Object corba_object) -{ - return cspi_object_add_ref (corba_object, FALSE); -} - -Accessible * -cspi_object_add_ref (CORBA_Object corba_object, gboolean do_ref) +/* + * This method swallows the corba_object BonoboUnknown + * reference, and returns an Accessible associated with it. + * If the reference is loaned, it means it is only valid + * between a borrow / return pair. + */ +static Accessible * +cspi_object_get_ref (CORBA_Object corba_object, gboolean on_loan) { Accessible *ref; @@ -206,26 +181,30 @@ cspi_object_add_ref (CORBA_Object corba_object, gboolean do_ref) { g_assert (ref->ref_count > 0); ref->ref_count++; - if (!do_ref) - cspi_release_unref (corba_object); + if (!on_loan) + { + if (ref->on_loan) /* Convert to a permanant ref */ + { + ref->on_loan = FALSE; + } + else + { + cspi_release_unref (corba_object); + } + } #ifdef DEBUG_OBJECTS g_print ("returning cached %p => %p\n", ref, ref->objref); #endif } else { - ref = cspi_object_new (corba_object); - + ref = malloc (sizeof (Accessible)); + ref->objref = corba_object; + ref->ref_count = 1; + ref->on_loan = on_loan; #ifdef DEBUG_OBJECTS g_print ("allocated %p => %p\n", ref, corba_object); #endif - if (do_ref) { -#ifdef JAVA_BRIDGE_BUG_IS_FIXED - g_assert (CORBA_Object_is_a (corba_object, - "IDL:Bonobo/Unknown:1.0", cspi_ev ())); -#endif - Bonobo_Unknown_ref (corba_object, cspi_ev ()); - } g_hash_table_insert (cspi_get_live_refs (), ref->objref, ref); } } @@ -233,17 +212,42 @@ cspi_object_add_ref (CORBA_Object corba_object, gboolean do_ref) return ref; } +Accessible * +cspi_object_add (CORBA_Object corba_object) +{ + return cspi_object_get_ref (corba_object, FALSE); +} + +Accessible * +cspi_object_borrow (CORBA_Object corba_object) +{ + return cspi_object_get_ref (corba_object, TRUE); +} + +void +cspi_object_return (Accessible *accessible) +{ + g_return_if_fail (accessible != NULL); + + if (!accessible->on_loan || + accessible->ref_count == 1) + { + cspi_object_unref (accessible); + } + else /* Convert to a permanant ref */ + { + accessible->on_loan = FALSE; + accessible->objref = cspi_dup_ref (accessible->objref); + accessible->ref_count--; + } +} + void cspi_object_ref (Accessible *accessible) { g_return_if_fail (accessible != NULL); - if (g_hash_table_lookup (cspi_get_live_refs (), accessible->objref) == NULL) { - accessible->objref = cspi_dup_ref (accessible->objref); - g_hash_table_insert (cspi_get_live_refs (), accessible->objref, accessible); - } else { - accessible->ref_count++; - } + accessible->ref_count++; } void @@ -372,15 +376,25 @@ SPI_nextEvent (SPIBoolean waitForEvent) static void report_leaked_ref (gpointer key, gpointer val, gpointer user_data) { - Accessible *a = (Accessible *) val; - char *name, *role; - name = Accessible_getName (a); - if (cspi_exception ()) name = NULL; - role = Accessible_getRoleName (a); - if (cspi_exception ()) role = NULL; - fprintf (stderr, "leaked object %s, role %s\n", (name) ? name : "", - (role) ? role : ""); - if (name) SPI_freeString (name); + char *name, *role; + Accessible *a = (Accessible *) val; + + name = Accessible_getName (a); + if (cspi_exception ()) + { + name = NULL; + } + + role = Accessible_getRoleName (a); + if (cspi_exception ()) + { + role = NULL; + } + + fprintf (stderr, "leaked %d references to object %s, role %s\n", + a->ref_count, name ? name : "", role ? role : ""); + + SPI_freeString (name); } @@ -437,11 +451,15 @@ SPI_exit (void) * * Free a character string returned from an at-spi call. Clients of * at-spi should use this function instead of free () or g_free(). + * A NULL string @s will be silently ignored. * This API should not be used to free strings * from other libraries or allocated by the client. **/ void SPI_freeString (char *s) { - CORBA_free (s); + if (s) + { + CORBA_free (s); + } } diff --git a/cspi/spi_text.c b/cspi/spi_text.c index 11fab39..fac8de6 100644 --- a/cspi/spi_text.c +++ b/cspi/spi_text.c @@ -23,8 +23,7 @@ #include static Accessibility_TEXT_BOUNDARY_TYPE -get_accessible_text_boundary_type ( - AccessibleTextBoundaryType type) +get_accessible_text_boundary_type (AccessibleTextBoundaryType type) { switch (type) { diff --git a/docs/reference/cspi/tmpl/spi_accessible.sgml b/docs/reference/cspi/tmpl/spi_accessible.sgml index da975e7..406676c 100644 --- a/docs/reference/cspi/tmpl/spi_accessible.sgml +++ b/docs/reference/cspi/tmpl/spi_accessible.sgml @@ -20,6 +20,7 @@ Accessible Objects @objref: +@on_loan: @ref_count: diff --git a/test/test-simple.c b/test/test-simple.c index 47699bb..802a3d0 100644 --- a/test/test-simple.c +++ b/test/test-simple.c @@ -650,13 +650,6 @@ global_listener_cb (const AccessibleEvent *event, fprintf (stderr, "Fielded focus event ...\n"); - /* - * must ref before doing "direct pointer" identity comparisons, - * e.g. "accessible == a". - * Alternatively, one can use cspi_object_equal (a, b) - */ - Accessible_ref (event->source); - if (!do_poke) { desktop = SPI_getDesktop (0); application = Accessible_getChildAtIndex (desktop, 0); @@ -677,8 +670,6 @@ global_listener_cb (const AccessibleEvent *event, print_tree = TRUE; validate_accessible (event->source, TRUE, TRUE); - - Accessible_unref (event->source); } static SPIBoolean @@ -689,8 +680,10 @@ key_listener_cb (const AccessibleKeystroke *stroke, *s = *stroke; - if (s->type == SPI_KEY_PRESSED) key_press_received = TRUE; - else if (s->type == SPI_KEY_RELEASED) key_release_received = TRUE; + if (s->type == SPI_KEY_PRESSED) + key_press_received = TRUE; + else if (s->type == SPI_KEY_RELEASED) + key_release_received = TRUE; return TRUE; } @@ -699,6 +692,7 @@ key_listener_cb (const AccessibleKeystroke *stroke, static void test_keylisteners (void) { +#ifdef BILL_MAKES_THIS_WORK_RELIABLY int i; AccessibleKeystroke stroke; AccessibleKeystrokeListener *key_listener; @@ -742,6 +736,7 @@ test_keylisteners (void) g_assert (SPI_generateMouseEvent (-50, -50, "rel")); g_assert (SPI_generateMouseEvent (-50, -50, "rel")); g_assert (SPI_generateMouseEvent (-1, -1, "b1c")); +#endif } int -- 2.7.4