Re: [PATCH 03/18] New observer behaviour

[Sergio Villar Senin <svillar igalia com>]

Rob Taylor escribiu:
> New observer behaviour. Observers will get folders_appeared events when
> a tny_folder_store_refresh or tny_folder_store_get_folders occurs and
> the cache is loaded for the first time. They get folders_created events
> when a new folder appears that we didn't know about before. Observers
> are notified about existing folders when they're attached.

Some comments

+ if (folder && !g_hash_table_lookup_extended (priv->known_folders,
folder, NULL, NULL)) {

No need to use the extended version.

- if (!(iter->flags & CAMEL_FOLDER_VIRTUAL) &&
_tny_folder_store_query_passes (query, iter) && priv->account)
+ if (!(iter->flags & CAMEL_FOLDER_VIRTUAL))

Why we are not taking into account the query? And why removing the
priv->account extra check?

+ if (folder && !g_hash_table_lookup_extended (priv->known_folders,
folder, NULL, NULL)) {

Again not need to use _extended

 static void
 tny_camel_folder_store_add_observer_default (TnyFolderStore *self,
TnyFolderStoreObserver *observer)
 {
 	TnyCamelFolderPriv *priv = TNY_CAMEL_FOLDER_GET_PRIVATE (self);
+	TnyFolderStoreChange *change = tny_folder_store_change_new (self);

 	g_assert (TNY_IS_FOLDER_STORE_OBSERVER (observer));

@@ -6100,6 +6206,10 @@ tny_camel_folder_store_add_observer_default
(TnyFolderStore *self, TnyFolderStor
 	}
 	g_static_rec_mutex_unlock (priv->obs_lock);

+	g_hash_table_foreach (priv->known_folders, build_appeared_change, change);
+	notify_folder_store_observers_about_in_idle (self, change,
TNY_FOLDER_PRIV_GET_SESSION (priv));
+	g_object_unref (change);
+

I don't understand this part, why do we notify about appeared folders
each time an observer is added ?


+	if (iter) {
+		TnyFolderStoreChange *change = NULL;

-			if (was_new && folder != NULL)
-				_tny_camel_folder_set_folder_info (self, folder, iter);
+		while (iter) {
+			/* Also take a look at camel-maildir-store.c:525 */
+			if (!(iter->flags & CAMEL_FOLDER_VIRTUAL)) {

Again in the store_account this time we're completely ignoring the query
parameter.

Br