Closed Bug 954917 Opened 10 years ago Closed 10 years ago

Incorrect shutdown leads to the same tweets being fetched twice

Categories

(Chat Core :: Twitter, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 3 obsolete files)

*** 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?
*** 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?
Attached patch Patch (obsolete) — Splinter Review
*** 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.
Attachment #8353318 - Flags: review?(clokep)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
*** 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.
Attachment #8353319 - Flags: review?(clokep)
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 ***
Attachment #8353318 - Attachment is obsolete: true
Attachment #8353318 - Flags: review?(clokep)
Attached patch Patch (obsolete) — Splinter Review
*** 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.
Attachment #8353320 - Flags: review?(clokep)
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 ***
Attachment #8353319 - Attachment is obsolete: true
Attachment #8353319 - Flags: review?(clokep)
OS: Linux → All
Hardware: x86 → All
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.
Attachment #8353320 - Attachment is obsolete: true
Attachment #8353320 - Flags: review?(clokep) → review-
Attached patch Patch with fixesSplinter Review
*** 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.
Attachment #8353321 - Flags: review+
*** Original post on bio 1484 at 2012-06-05 21:51:58 UTC ***

Thanks for catching the . :)
Whiteboard: [checkin-needed]
*** Original post on bio 1484 at 2012-06-05 23:07:55 UTC ***

Fixed in http://hg.instantbird.org/instantbird/rev/027f06cf53a2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.