GNOME Bugzilla – Bug 653760
Refresh logs when new stuff happens
Last modified: 2011-07-04 13:07:45 UTC
If we're displaying 'Everything' with 'Some contact' 'Today', and 'Some contact' sends us a message, we should refresh the logs so that message is shown. And likewise for any kind of event.
http://cgit.collabora.com/git/user/pochu/empathy.git/log/?h=refresh-logs I'll squash the last commit before merging.
You should use tp_g_signal_connect_object() when connecting on TpChanel sigs. Atm you suffer this crash - Open the log viewer - Open a chat and send a message - Close the log viewer - Send another message - CRASH Note that you'll have to keep a pointer on the TpAccount and TpChannel to release the ref when the log viewer is destroyed. You forgot to remove some g_print(). When receing a new message, the selection of the 3 columns is briefly unselected and reselected. Can't we avoid that, that's a bit ugly?
New branch at http://cgit.collabora.com/git/user/pochu/empathy.git/log/?h=refresh-logs-653760 First commit is the same as the old branch but ported on top of my log-viewer-gobject branch (mostly s/window/self->priv/, no significant changes). (In reply to comment #2) > You should use tp_g_signal_connect_object() when connecting on TpChanel sigs. > Atm you suffer this crash > - Open the log viewer > - Open a chat and send a message > - Close the log viewer > - Send another message > - CRASH Fixed. > Note that you'll have to keep a pointer on the TpAccount and TpChannel to > release the ref when the log viewer is destroyed. Done. > You forgot to remove some g_print(). Woops, fixed. > When receing a new message, the selection of the 3 columns is briefly > unselected and reselected. Can't we avoid that, that's a bit ugly? That's because of this: if (refresh) { DEBUG ("Refreshing logs after received event"); /* We need to populate the entities in case we didn't have * any previous logs with this contact. */ log_window_who_populate (log_window); log_window_chats_get_messages (log_window, FALSE); } The log_window_who_populate() call wasn't there initially. However I thought of the case were you have selected 'Anyone' selected, and start talking / calling / whatever a contact for whom you have no previous logs. So he isn't currently in the Who treeview. So when we called log_window_chats_get_messages() (which refreshes the logs) his logs weren't picked because he isn't in the Who pane (and we retrieve logs for contacts there as there's no tpl_log_manager_get_all_logs() or similar). So we need to refresh the Who pane first, so that contact gets added, and that also refreshed the When pane. That's why it flickers. I've just noticed that introduces a regression though: If you select 'Some contact', and chat with it, the logs will be refreshed, but then 'Anyone' will be selected (instead of 'Some contact'). Also there's a race as we refresh the logs inmediately but the logger won't have them yet, so you don't get the new contact in the first message anyway. We could refresh the logs in a g_idle_add() after 250ms or so to avoid that. But then there's the other problem and the flickering, so maybe we shouldn't worry about that corner-case too much for now... WDYT?
> The log_window_who_populate() call wasn't there initially. However I thought of > the case were you have selected 'Anyone' selected, and start talking / calling > / whatever a contact for whom you have no previous logs. So he isn't currently > in the Who treeview. So when we called log_window_chats_get_messages() (which > refreshes the logs) his logs weren't picked because he isn't in the Who pane > (and we retrieve logs for contacts there as there's no > tpl_log_manager_get_all_logs() or similar). So we need to refresh the Who pane > first, so that contact gets added, and that also refreshed the When pane. > That's why it flickers. > > I've just noticed that introduces a regression though: If you select 'Some > contact', and chat with it, the logs will be refreshed, but then 'Anyone' will > be selected (instead of 'Some contact'). Also there's a race as we refresh the > logs inmediately but the logger won't have them yet, so you don't get the new > contact in the first message anyway. We could refresh the logs in a > g_idle_add() after 250ms or so to avoid that. But then there's the other > problem and the flickering, so maybe we shouldn't worry about that corner-case > too much for now... WDYT? The events related to the new contacts will still appear in the 'Anyone' section right? If they do, I think that's fine tbh. Speaking of races, don't we have a race when observing chats/calls as well? How can we be sure that the call/msg already raised the logger and so will appear when refreshing the logs?
(In reply to comment #4) > > The log_window_who_populate() call wasn't there initially. However I thought of > > the case were you have selected 'Anyone' selected, and start talking / calling > > / whatever a contact for whom you have no previous logs. So he isn't currently > > in the Who treeview. So when we called log_window_chats_get_messages() (which > > refreshes the logs) his logs weren't picked because he isn't in the Who pane > > (and we retrieve logs for contacts there as there's no > > tpl_log_manager_get_all_logs() or similar). So we need to refresh the Who pane > > first, so that contact gets added, and that also refreshed the When pane. > > That's why it flickers. > > > > I've just noticed that introduces a regression though: If you select 'Some > > contact', and chat with it, the logs will be refreshed, but then 'Anyone' will > > be selected (instead of 'Some contact'). Also there's a race as we refresh the > > logs inmediately but the logger won't have them yet, so you don't get the new > > contact in the first message anyway. We could refresh the logs in a > > g_idle_add() after 250ms or so to avoid that. But then there's the other > > problem and the flickering, so maybe we shouldn't worry about that corner-case > > too much for now... WDYT? > > > The events related to the new contacts will still appear in the 'Anyone' > section right? If they do, I think that's fine tbh. If we remove the log_window_who_populate() call? No, because when you select 'Anyone' we currently do tpl_log_manager_get_events_async() for each contact in the treeview, so if we don't refresh them first, we won't retrieve logs for them. We could probably change it so that if 'Anyone' is selected, we do a tpl_log_manager_get_entities_async() and then call tpl_log_manager_get_events_async() for all those entities instead of for the ones we already have, though ideally TplLogManager would have a ::new-entity signal so we could avoid all these hacks! I've just opened https://bugs.freedesktop.org/show_bug.cgi?id=38945 about it. > Speaking of races, don't we have a race when observing chats/calls as well? > How can we be sure that the call/msg already raised the logger and so will > appear when refreshing the logs?
13:18 < cassidy> pochu, merge your branch for now and improve it once the tp-logger bug has been fixed, I think Merged to master, thanks!