GNOME.org
 

[nautilus-actions] Review notifications bufferization

commit 9119b5cd6d6e16b71197eedf36b5aa675c2eb5af
Author: Pierre Wieser <pwieser trychlos org>
Date:   Tue Aug 10 00:09:54 2010 +0200

    Review notifications bufferization

 ChangeLog                              |   17 +++++++
 src/core/na-ipivot-consumer.c          |   81 +++++++++++++++++++------------
 src/core/na-ipivot-consumer.h          |    2 +-
 src/core/na-pivot.c                    |   18 ++++---
 src/io-desktop/nadp-desktop-provider.c |   24 ++++------
 src/io-gconf/nagp-gconf-provider.c     |   40 ++++++++++------
 src/nact/nact-main-menubar-file.c      |    8 ++-
 src/nact/nact-main-window.c            |    2 +
 src/plugin-menu/nautilus-actions.c     |    2 +
 9 files changed, 123 insertions(+), 71 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 43faa3c..338059c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2010-08-09 Pierre Wieser <pwieser trychlos org>
 
+	Review notification bufferization.
+
+	* src/core/na-ipivot-consumer.c:
+	* src/core/na-ipivot-consumer.h
+	(na_ipivot_consumer_delay_notify): Removed function.
+	(na_ipivot_consumer_allow_notify): New function.
+
+	* src/nact/nact-main-menubar-file.c (nact_main_menubar_file_save_items):
+	* src/nact/nact-main-window.c (instance_init):
+	* src/plugin-menu/nautilus-actions.c (instance_init):
+	Updated accordingly.
+
+	* src/core/na-pivot.c (on_item_changed_timeout):
+	* src/io-desktop/nadp-desktop-provider.c (on_monitor_timeout):
+	* src/io-gconf/nagp-gconf-provider.c (config_path_changed_trigger_interface):
+	Review the code.
+
 	Fix XML import of localized vs. unlocalized profile label.
 
 	* src/api/na-ifactory-object-data.h:
diff --git a/src/core/na-ipivot-consumer.c b/src/core/na-ipivot-consumer.c
index 63d9c16..04c609d 100644
--- a/src/core/na-ipivot-consumer.c
+++ b/src/core/na-ipivot-consumer.c
@@ -128,34 +128,48 @@ interface_base_finalize( NAIPivotConsumerInterface *klass )
 }
 
 /**
- * na_ipivot_consumer_delay_notify:
+ * na_ipivot_consumer_allow_notify:
  * @instance: the #NAIPivotConsumer instance.
+ * @allow: whether notifications are allowed for this consumer.
+ * @delay: delay in msec before actually allow the notifications;
+ *  only considered when @allow is %TRUE.
  *
- * Informs the #NAIPivotconsumer instance that the next notification
- * message should only be handled if a short delay has expired. This
- * let us to no be polluted by notifications that we create ourselves
- * (e.g. when saving actions).
+ * Set the notification flag for this consumer.
+ *
+ * When @allow is %FALSE, then all notifications from #NAPivot are blocked.
+ *
+ * Allowing notifications with a delay is useful in particular when saving
+ * the items list from NACT. As both I/O providers and NAPivot bufferize
+ * their individual notifications, NAIPivot consumers will only be
+ * triggered themselves after the sum of all burst timeouts.
+ * So, we reauthorize notifications at end of save code, with a delay
+ * which _should_ be greater that this sum...
  */
 void
-na_ipivot_consumer_delay_notify( NAIPivotConsumer *instance )
+na_ipivot_consumer_allow_notify( NAIPivotConsumer *instance, gboolean allow, guint delay )
 {
-	static const gchar *thisfn = "na_ipivot_consumer_delay_notify";
-	GTimeVal *last_delay;
+	static const gchar *thisfn = "na_ipivot_consumer_allow_notify";
+	GTimeVal *tval;
 
 	g_return_if_fail( NA_IS_IPIVOT_CONSUMER( instance ));
 
 	if( st_initialized && !st_finalized ){
 
-		g_debug( "%s: instance=%p", thisfn, ( void * ) instance );
+		g_debug( "%s: instance=%p, allow=%s, delay=%d", thisfn, ( void * ) instance, allow ? "True":"False", delay );
 
-		last_delay = ( GTimeVal * ) g_object_get_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify" );
+		g_object_set_data( G_OBJECT( instance ), "na-ipivot-consumer-allow-notify", GUINT_TO_POINTER( allow ));
 
-		if( !last_delay ){
-			last_delay = g_new0( GTimeVal, 1 );
-			g_object_set_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify", last_delay );
-		}
+		if( allow ){
+			tval = ( GTimeVal * ) g_object_get_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify" );
 
-		g_get_current_time( last_delay );
+			if( !tval ){
+				tval = g_new0( GTimeVal, 1 );
+				g_object_set_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify", tval );
+			}
+
+			g_get_current_time( tval );
+			g_time_val_add( tval, 1000*delay );
+		}
 	}
 }
 
@@ -175,8 +189,11 @@ na_ipivot_consumer_notify_of_autosave_changed( NAIPivotConsumer *instance, gbool
 
 	if( st_initialized && !st_finalized ){
 
-		if( NA_IPIVOT_CONSUMER_GET_INTERFACE( instance )->on_autosave_changed ){
-			NA_IPIVOT_CONSUMER_GET_INTERFACE( instance )->on_autosave_changed( instance, enabled, period );
+		if( is_notify_allowed( instance )){
+
+			if( NA_IPIVOT_CONSUMER_GET_INTERFACE( instance )->on_autosave_changed ){
+				NA_IPIVOT_CONSUMER_GET_INTERFACE( instance )->on_autosave_changed( instance, enabled, period );
+			}
 		}
 	}
 }
@@ -309,20 +326,22 @@ void na_ipivot_consumer_notify_of_mandatory_prefs_changed( NAIPivotConsumer *ins
 static gboolean
 is_notify_allowed( const NAIPivotConsumer *instance )
 {
-	static const gchar *thisfn = "na_ipivot_consumer_is_notify_allowed";
-	GTimeVal *last_delay;
-	GTimeVal now;
-	gulong ecart;
-
-	last_delay = ( GTimeVal * ) g_object_get_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify" );
-	if( !last_delay ){
-		return( TRUE );
-	}
+	gboolean allowed;
+	GTimeVal *tval, now;
+	glong ecart;
+
+	allowed = ( gboolean ) GPOINTER_TO_UINT( g_object_get_data( G_OBJECT( instance ), "na-ipivot-consumer-allow-notify" ));
 
-	g_get_current_time( &now );
-	ecart = 1000000 * ( now.tv_sec - last_delay->tv_sec );
-	ecart += now.tv_usec - last_delay->tv_usec;
+	if( allowed ){
+		tval = ( GTimeVal * ) g_object_get_data( G_OBJECT( instance ), "na-ipivot-consumer-delay-notify" );
+
+		if( tval ){
+			g_get_current_time( &now );
+			ecart = G_USEC_PER_SEC * ( now.tv_sec - tval->tv_sec );
+			ecart += now.tv_usec - tval->tv_usec;
+			allowed = ( ecart > 0 );
+		}
+	}
 
-	g_debug( "%s: ecart=%ld", thisfn, ecart );
-	return( ecart > 2000000 );
+	return( allowed );
 }
diff --git a/src/core/na-ipivot-consumer.h b/src/core/na-ipivot-consumer.h
index b4af8a2..be36c58 100644
--- a/src/core/na-ipivot-consumer.h
+++ b/src/core/na-ipivot-consumer.h
@@ -129,7 +129,7 @@ typedef struct {
 
 GType na_ipivot_consumer_get_type( void );
 
-void  na_ipivot_consumer_delay_notify( NAIPivotConsumer *instance );
+void  na_ipivot_consumer_allow_notify( NAIPivotConsumer *instance, gboolean allow, guint delay );
 
 void  na_ipivot_consumer_notify_of_autosave_changed        ( NAIPivotConsumer *instance, gboolean enabled, guint period );
 void  na_ipivot_consumer_notify_of_create_root_menu_changed( NAIPivotConsumer *instance, gboolean enabled );
diff --git a/src/core/na-pivot.c b/src/core/na-pivot.c
index 16f5f7b..22a3a79 100644
--- a/src/core/na-pivot.c
+++ b/src/core/na-pivot.c
@@ -91,8 +91,7 @@ enum {
 
 
 static GObjectClass *st_parent_class = NULL;
-static gint          st_timeout_msec = 100;
-static gint          st_timeout_usec = 100000;
+static gint          st_burst_timeout = 200;		/* burst timeout in msec */
 
 static GType         register_type( void );
 static void          class_init( NAPivotClass *klass );
@@ -622,15 +621,14 @@ na_pivot_item_changed_handler( NAIIOProvider *provider, const gchar *id, NAPivot
 
 		if( !pivot->private->event_source_id ){
 			pivot->private->event_source_id =
-				g_timeout_add( st_timeout_msec, ( GSourceFunc ) on_item_changed_timeout, pivot );
+				g_timeout_add( st_burst_timeout, ( GSourceFunc ) on_item_changed_timeout, pivot );
 		}
 	}
 }
 
 /*
  * this timer is set when we receive the first event of a serie
- * we continue to loop until last event is at least one half of a
- * second old
+ * we continue to loop until last event is older that our burst timeout
  *
  * there is no race condition here as we are not multithreaded
  * or .. is there ?
@@ -642,17 +640,21 @@ on_item_changed_timeout( NAPivot *pivot )
 	GTimeVal now;
 	gulong diff;
 	GList *ic;
+	gulong timeout_usec = 1000*st_burst_timeout;
 
 	g_return_val_if_fail( NA_IS_PIVOT( pivot ), FALSE );
 
-	g_debug( "%s: pivot=%p", thisfn, pivot );
-
 	g_get_current_time( &now );
 	diff = time_val_diff( &now, &pivot->private->last_event );
-	if( diff < st_timeout_usec ){
+	if( diff < timeout_usec ){
 		return( TRUE );
 	}
 
+	/* last individual notification is older that the st_burst_timeout
+	 * so triggers the NAIIOProvider interface and destroys this timeout
+	 */
+	g_debug( "%s: triggering NAIPivotConsumer interfaces", thisfn );
+
 	if( pivot->private->automatic_reload ){
 		na_pivot_load_items( pivot );
 	}
diff --git a/src/io-desktop/nadp-desktop-provider.c b/src/io-desktop/nadp-desktop-provider.c
index 0a198b5..6caefd3 100644
--- a/src/io-desktop/nadp-desktop-provider.c
+++ b/src/io-desktop/nadp-desktop-provider.c
@@ -54,8 +54,7 @@ extern NAIExporterFormat nadp_formats[];
 
 static GType         st_module_type = 0;
 static GObjectClass *st_parent_class = NULL;
-static guint         st_timeout_msec = 100;
-static guint         st_timeout_usec = 100000;
+static guint         st_burst_timeout = 100;		/* burst timeout in msec */
 
 static void                     class_init( NadpDesktopProviderClass *klass );
 static void                     instance_init( GTypeInstance *instance, gpointer klass );
@@ -351,21 +350,15 @@ nadp_desktop_provider_add_monitor( NadpDesktopProvider *provider, const gchar *d
 void
 nadp_desktop_provider_on_monitor_event( NadpDesktopProvider *provider )
 {
-	static const gchar *thisfn = "nadp_desktop_provider_on_monitor_event";
-	GTimeVal now;
-
 	g_return_if_fail( NADP_IS_DESKTOP_PROVIDER( provider ));
 
 	if( !provider->private->dispose_has_run ){
 
-		g_get_current_time( &now );
-		g_debug( "%s: time=%ld.%ld", thisfn, now.tv_sec, now.tv_usec );
-
 		g_get_current_time( &provider->private->last_event );
 
 		if( !provider->private->event_source_id ){
 			provider->private->event_source_id =
-				g_timeout_add( st_timeout_msec, ( GSourceFunc ) on_monitor_timeout, provider );
+				g_timeout_add( st_burst_timeout, ( GSourceFunc ) on_monitor_timeout, provider );
 		}
 	}
 }
@@ -395,17 +388,20 @@ on_monitor_timeout( NadpDesktopProvider *provider )
 	static const gchar *thisfn = "nadp_desktop_provider_on_monitor_timeout";
 	GTimeVal now;
 	gulong diff;
-
-	g_debug( "%s", thisfn );
+	gulong timeout_usec = 1000*st_burst_timeout;
 
 	g_get_current_time( &now );
 	diff = time_val_diff( &now, &provider->private->last_event );
-	if( diff < st_timeout_usec ){
-		g_debug( "%s: unexpired timeout: returning True", thisfn );
+	if( diff < timeout_usec ){
 		return( TRUE );
 	}
 
-	g_debug( "%s: expired timeout: advertising IIOProvider, returning False", thisfn );
+	/* last individual notification is older that the st_burst_timeout
+	 * so triggers the NAIIOProvider interface and destroys this timeout
+	 */
+	g_debug( "%s: triggering NAIIOProvider interface for provider=%p (%s)",
+			thisfn, ( void * ) provider, G_OBJECT_TYPE_NAME( provider ));
+
 	na_iio_provider_item_changed( NA_IIO_PROVIDER( provider ));
 	provider->private->event_source_id = 0;
 	return( FALSE );
diff --git a/src/io-gconf/nagp-gconf-provider.c b/src/io-gconf/nagp-gconf-provider.c
index 4022b8f..bf049aa 100644
--- a/src/io-gconf/nagp-gconf-provider.c
+++ b/src/io-gconf/nagp-gconf-provider.c
@@ -52,8 +52,7 @@ struct NagpGConfProviderClassPrivate {
 
 static GType         st_module_type = 0;
 static GObjectClass *st_parent_class = NULL;
-static gint          st_timeout_msec = 100;
-static gint          st_timeout_usec = 100000;
+static gint          st_burst_timeout = 100;		/* burst timeout in msec */
 
 static void     class_init( NagpGConfProviderClass *klass );
 static void     instance_init( GTypeInstance *instance, gpointer klass );
@@ -308,11 +307,18 @@ install_monitors( NagpGConfProvider *provider )
  * - first, GConf underlying subsystem advertises us, through the watch
  *   mechanism, of each and every modification ; this leads us to be
  *   triggered for each new/modified/deleted _entry_
- * - as we would trigger the NAIIOProvider interface only once for each
- *   modified _object_, we install a timer in order to wait for all
- *   entries have been modified, or another object is concerned
- * - as soon as one of the two previous conditions is met, we trigger
- *   the NAIIOProvider interface with the corresponding object id
+ * - as we want trigger the NAIIOProvider interface only once for each
+ *   update operation (i.e. once for each flow of individual notifications),
+ *   then we install a timer in order to wait for all
+ *   entries have been modified
+ * - when a [burst_timeout] reasonable delay has elapsed without having
+ *   received any new individual notification, then we can assume that
+ *   we have reached the end of the flow and that we can now trigger
+ *   the NAIIOProvider interface
+ *
+ * Note that we used to try to send one notification per modified object.
+ * This cannot work as we are not sure at all that we will received
+ * individual notifications themselves grouped by object.
  */
 static void
 config_path_changed_cb( GConfClient *client, guint cnxn_id, GConfEntry *entry, NagpGConfProvider *provider )
@@ -327,7 +333,7 @@ config_path_changed_cb( GConfClient *client, guint cnxn_id, GConfEntry *entry, N
 		if( !provider->private->event_source_id ){
 			provider->private->event_source_id =
 				g_timeout_add(
-						st_timeout_msec,
+						st_burst_timeout,
 						( GSourceFunc ) config_path_changed_trigger_interface,
 						provider );
 		}
@@ -336,29 +342,33 @@ config_path_changed_cb( GConfClient *client, guint cnxn_id, GConfEntry *entry, N
 
 /*
  * this timer is set when we receive the first event of a serie
- * we continue to loop until last event is at least one half of a
- * second old
+ * we continue to loop until last event is older that the st_burst_timeout
+ * delay (in msec)
  * there is no race condition here as we are not multithreaded
  * or .. is there ?
  */
 static gboolean
 config_path_changed_trigger_interface( NagpGConfProvider *provider )
 {
-	/*static const gchar *thisfn = "nagp_gconf_provider_config_path_changed_trigger_interface";*/
+	static const gchar *thisfn = "nagp_gconf_provider_config_path_changed_trigger_interface";
 	GTimeVal now;
 	gulong diff;
-
-	/*g_debug( "%s: provider=%p", thisfn, ( void * ) provider );*/
+	gulong timeout_usec = 1000*st_burst_timeout;
 
 	g_get_current_time( &now );
 	diff = time_val_diff( &now, &provider->private->last_event );
-	if( diff < st_timeout_usec ){
+	if( diff < timeout_usec ){
 		return( TRUE );
 	}
 
+	/* last individual notification is older that the st_burst_timeout
+	 * so triggers the NAIIOProvider interface and destroys this timeout
+	 */
+	g_debug( "%s: triggering NAIIOProvider interface for provider=%p (%s)",
+			thisfn, ( void * ) provider, G_OBJECT_TYPE_NAME( provider ));
+
 	na_iio_provider_item_changed( NA_IIO_PROVIDER( provider ));
 	provider->private->event_source_id = 0;
-
 	return( FALSE );
 }
 
diff --git a/src/nact/nact-main-menubar-file.c b/src/nact/nact-main-menubar-file.c
index 7172419..b695702 100644
--- a/src/nact/nact-main-menubar-file.c
+++ b/src/nact/nact-main-menubar-file.c
@@ -267,6 +267,10 @@ nact_main_menubar_file_save_items( NactMainWindow *window )
 
 	g_debug( "%s: window=%p", thisfn, ( void * ) window );
 
+	/* get ride of notification messages of IOProviders
+	 */
+	na_ipivot_consumer_allow_notify( NA_IPIVOT_CONSUMER( window ), FALSE, 0 );
+
 	/* remove deleted items
 	 * so that new actions with same id do not risk to be deleted later
 	 */
@@ -317,9 +321,9 @@ nact_main_menubar_file_save_items( NactMainWindow *window )
 		g_signal_emit_by_name( window, MAIN_WINDOW_SIGNAL_UPDATE_ACTION_SENSITIVITIES, NULL );
 	}
 
-	/* get ride of notification messages of IOProviders
+	/* restore NAPivot notifications
 	 */
-	na_ipivot_consumer_delay_notify( NA_IPIVOT_CONSUMER( window ));
+	na_ipivot_consumer_allow_notify( NA_IPIVOT_CONSUMER( window ), TRUE, 1000 );
 }
 
 /*
diff --git a/src/nact/nact-main-window.c b/src/nact/nact-main-window.c
index 46c7937..f114d04 100644
--- a/src/nact/nact-main-window.c
+++ b/src/nact/nact-main-window.c
@@ -662,6 +662,8 @@ instance_init( GTypeInstance *instance, gpointer klass )
 			G_CALLBACK( on_tab_updatable_item_updated ));
 
 	self->private->dispose_has_run = FALSE;
+
+	na_ipivot_consumer_allow_notify( NA_IPIVOT_CONSUMER( instance ), TRUE, 0 );
 }
 
 static void
diff --git a/src/plugin-menu/nautilus-actions.c b/src/plugin-menu/nautilus-actions.c
index 3868696..a294261 100644
--- a/src/plugin-menu/nautilus-actions.c
+++ b/src/plugin-menu/nautilus-actions.c
@@ -193,6 +193,8 @@ instance_init( GTypeInstance *instance, gpointer klass )
 	self->private = g_new0( NautilusActionsPrivate, 1 );
 
 	self->private->dispose_has_run = FALSE;
+
+	na_ipivot_consumer_allow_notify( NA_IPIVOT_CONSUMER( instance ), TRUE, 0 );
 }
 
 static void
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]