954917 - Incorrect shutdown leads to the same tweets being fetched twice

Status: RESOLVED FIXED

(Chat Core :: Twitter, defect)

Description

[aleth [:aleth]]

*** Original post on bio 1484 at 2012-06-05 20:03:00 UTC ***

Quite frequently, when logging off, IB does not shut down entirely correctly. This isn't a problem for any protocol but twitter, where it frequently happens that the same tweets are fetched twice (and then appear twice in the logs).
This is annoying and will become more so once bug 954392 (bio 958) lands.

The problem is, I suspect, that unInit is not called in time or the pref is not saved correctly:
http://lxr.instantbird.org/instantbird/source/chat/protocols/twitter/twitter.js#895

Would it not make more sense to move this code (line 895-905) to run immediately whenever a tweet is received?

Comment 1

[Florian Quèze [:florian]]

*** Original post on bio 1484 at 2012-06-05 20:10:43 UTC ***

So we are saving the value of that pref only at shutdown (when the account is uninitialized), but we read it from the prefs service each time we reconnect the account. There's no visible consequence because we cache tweet ids and eliminate duplicates (arrived during the same session) before displaying received tweets, but isn't this wasting bandwidth?

Comment 2

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1484 as attmnt 1563 at 2012-06-05 20:50:00 UTC ***

I don't know the twitter code, so I might have missed something, but this seems to work.

Comment 3

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1484 as attmnt 1564 at 2012-06-05 21:00:00 UTC ***

Removes some duplication.

flo mentioned there may be an issue with set*Pref not being cheap.

Comment 4

[aleth [:aleth]]

Comment on attachment 8353318 [details] [diff] [review]
Patch

*** Original change on bio 1484 attmnt 1563 at 2012-06-05 21:00:55 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 5

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 1484 as attmnt 1565 at 2012-06-05 21:18:00 UTC ***

I asked on #developers abot setCharPref, but so far no reply.

This patch should avoid the issue though.

Comment 6

[aleth [:aleth]]

Comment on attachment 8353319 [details] [diff] [review]
Patch

*** Original change on bio 1484 attmnt 1564 at 2012-06-05 21:18:11 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 7

[Patrick Cloke [:clokep]]

Comment on attachment 8353320 [details] [diff] [review]
Patch

*** Original change on bio 1484 attmnt 1565 at 2012-06-05 21:47:39 UTC ***

You missed a period in this patch, I've fixed it (and the paths though), so I'll upload a new patch.

Comment 8

[Patrick Cloke [:clokep]]

Attachment: Patch with fixes

*** Original post on bio 1484 as attmnt 1566 at 2012-06-05 21:48:00 UTC ***

This is the same patch with the paths fixed and the missing period.

Comment 9

[aleth [:aleth]]

*** Original post on bio 1484 at 2012-06-05 21:51:58 UTC ***

Thanks for catching the . :)

Comment 10

[Patrick Cloke [:clokep]]

*** Original post on bio 1484 at 2012-06-05 23:07:55 UTC ***

Fixed in http://hg.instantbird.org/instantbird/rev/027f06cf53a2