[g-a-devel]at-spi patch ...
- From: Michael Meeks <michael ximian com>
- To: Darren Kenny sun com, Marc Mulcahy <marc mulcahy sun com>
- Cc: accessibility mailing list <gnome-accessibility-devel gnome org>
- Subject: [g-a-devel]at-spi patch ...
- Date: 12 Aug 2002 18:08:44 +0100
So,
May I commit this ?
Brief Synopsis:
The 'on_loan' bitflag is used to denote the fact that we have a live
object reference for the lifetime of an incoming event notification.
This is used to save lots of roundtrips dupping and freeing it [ or
leaking and doing whacked out things on 'ref' as before ]. It cleans and
simplifies the code, and makes it much more efficient.
While we handle the case where an object already on loan is re-loaned
and returned by a re-enterant event notification cleanly, this path is
not efficient - since it should almost never happen. It could be made
more efficient were that a likely case, by adding a 'loan_count' or
somesuch. We also promote loaned references where possible to non-loaned
to improve remote traffic efficiency.
The patch also slightly redresses the copyright issue, and fixes a few
other bugs / bits of brokenness that were lurking around. It would be a
shame IMHO to ship a version with Gnome 2.0.1 that was known broken.
May I commit ?
Michael.
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/at-spi/ChangeLog,v
retrieving revision 1.233
diff -u -p -u -r1.233 ChangeLog
--- ChangeLog 12 Aug 2002 09:49:38 -0000 1.233
+++ ChangeLog 12 Aug 2002 17:06:40 -0000
@@ -1,3 +1,34 @@
+2002-08-12 Michael Meeks <michael ximian com>
+
+ * 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 <darren kenny sun com>
* cspi/bonobo/cspi-bonobo-listener.c:
Index: atk-bridge/bridge.c
===================================================================
RCS file: /cvs/gnome/at-spi/atk-bridge/bridge.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 bridge.c
--- atk-bridge/bridge.c 20 Jun 2002 21:37:58 -0000 1.43
+++ atk-bridge/bridge.c 12 Aug 2002 17:06:40 -0000
@@ -575,6 +575,7 @@ spi_atk_bridge_key_listener (AtkKeyEvent
result = Accessibility_DeviceEventController_notifyListenersSync (
controller, &key_event, &ev);
+ bonobo_object_release_unref (controller, &ev);
CORBA_exception_free (&ev);
}
Index: cspi/cspi-lowlevel.h
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/cspi-lowlevel.h,v
retrieving revision 1.4
diff -u -p -u -r1.4 cspi-lowlevel.h
--- cspi/cspi-lowlevel.h 20 Jun 2002 12:10:34 -0000 1.4
+++ cspi/cspi-lowlevel.h 12 Aug 2002 17:06:40 -0000
@@ -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
Index: cspi/spi-private.h
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/spi-private.h,v
retrieving revision 1.8
diff -u -p -u -r1.8 spi-private.h
--- cspi/spi-private.h 13 Jun 2002 22:31:29 -0000 1.8
+++ cspi/spi-private.h 12 Aug 2002 17:06:40 -0000
@@ -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)) \
Index: cspi/spi_main.c
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/spi_main.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 spi_main.c
--- cspi/spi_main.c 20 Jun 2002 12:10:34 -0000 1.26
+++ cspi/spi_main.c 12 Aug 2002 17:06:40 -0000
@@ -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, gcon
{
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_
{
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_
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);
+ }
}
Index: cspi/spi_text.c
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/spi_text.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 spi_text.c
--- cspi/spi_text.c 13 May 2002 07:48:57 -0000 1.18
+++ cspi/spi_text.c 12 Aug 2002 17:06:40 -0000
@@ -23,8 +23,7 @@
#include <cspi/spi-private.h>
static Accessibility_TEXT_BOUNDARY_TYPE
-get_accessible_text_boundary_type (
- AccessibleTextBoundaryType type)
+get_accessible_text_boundary_type (AccessibleTextBoundaryType type)
{
switch (type)
{
Index: cspi/bonobo/cspi-bonobo-listener.c
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/bonobo/cspi-bonobo-listener.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 cspi-bonobo-listener.c
--- cspi/bonobo/cspi-bonobo-listener.c 12 Aug 2002 09:49:38 -0000 1.14
+++ cspi/bonobo/cspi-bonobo-listener.c 12 Aug 2002 17:06:40 -0000
@@ -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 *listene
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 *listene
eh->cb.event (&aevent, eh->user_data);
}
- cspi_object_unref( source );
+ cspi_object_return (source);
}
static void
Index: cspi/bonobo/cspi-bonobo-listener.h
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/bonobo/cspi-bonobo-listener.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 cspi-bonobo-listener.h
--- cspi/bonobo/cspi-bonobo-listener.h 13 May 2002 07:49:03 -0000 1.6
+++ cspi/bonobo/cspi-bonobo-listener.h 12 Aug 2002 17:06:40 -0000
@@ -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
Index: cspi/bonobo/cspi-bonobo.c
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/bonobo/cspi-bonobo.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 cspi-bonobo.c
--- cspi/bonobo/cspi-bonobo.c 20 Jun 2002 12:10:35 -0000 1.5
+++ cspi/bonobo/cspi-bonobo.c 12 Aug 2002 17:06:40 -0000
@@ -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
Index: docs/reference/cspi/tmpl/spi_accessible.sgml
===================================================================
RCS file: /cvs/gnome/at-spi/docs/reference/cspi/tmpl/spi_accessible.sgml,v
retrieving revision 1.9
diff -u -p -u -r1.9 spi_accessible.sgml
--- docs/reference/cspi/tmpl/spi_accessible.sgml 17 Apr 2002 00:20:54 -0000 1.9
+++ docs/reference/cspi/tmpl/spi_accessible.sgml 12 Aug 2002 17:06:40 -0000
@@ -20,6 +20,7 @@ Accessible Objects
</para>
@objref:
+ on_loan:
@ref_count:
<!-- ##### FUNCTION Accessible_ref ##### -->
Index: test/test-simple.c
===================================================================
RCS file: /cvs/gnome/at-spi/test/test-simple.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 test-simple.c
--- test/test-simple.c 19 Jun 2002 13:47:11 -0000 1.29
+++ test/test-simple.c 12 Aug 2002 17:06:40 -0000
@@ -650,13 +650,6 @@ global_listener_cb (const AccessibleEven
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 AccessibleEven
print_tree = TRUE;
validate_accessible (event->source, TRUE, TRUE);
-
- Accessible_unref (event->source);
}
static SPIBoolean
@@ -689,8 +680,10 @@ key_listener_cb (const AccessibleKeystro
*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 AccessibleKeystro
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
--
mmeeks gnu org <><, Pseudo Engineer, itinerant idiot
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
