Closed Bug 955124 Opened 10 years ago Closed 10 years ago

rewrite Bubbles' timer handling so that timers are used only when actually needed

Categories

(Instantbird Graveyard :: Conversation, defect)

x86
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

Details

Attachments

(1 file)

*** Original post on bio 1696 at 2012-09-13 09:13:00 UTC ***

refreshIntervals is currently wasteful in 2 cases:
- a conversation has been restored from hold, in which case the first interval is most likely not 0s, and starting with timers of 1s is wasteful and annoying (as if the user has scrolled up by less than 10px, each time we update the last message interval we auto-scroll to the bottom). We should give refreshIntervals a parameter indicating what the current time interval is, rather than always assume it starts at 0.
- The last new message is over a day (or is it 25 hours? I don't remember if we sometimes show "1 day and 10 minutes" if the hours value is 0...) old, in which case we should update the last time every hour rather than every minute. I noticed this when playing with nglayout.debug.paint_flashing. I had 7 conversations open on my old macbook that had been offline for almost two days, and Instantbird kept repainting the bottom of the conversations (7 times per minute; once per conversation); which seems a nice way to contribute to draining the battery.
*** Original post on bio 1696 at 2012-12-23 15:15:00 UTC ***

Another wasteful case is if we are still displaying the conversation, but have stopped adding message because already 40ms have been spent adding messages. Our timer really shouldn't be fired in that case.
*** Original post on bio 1696 at 2012-12-23 15:37:07 UTC ***

(In reply to comment #0)

> - The last new message is over a day (or is it 25 hours? I don't remember if we
> sometimes show "1 day and 10 minutes" if the hours value is 0...) old, in which
> case we should update the last time every hour rather than every minute.

I just checked, it's after 24 hours. We don't display minutes if there at least a day.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
*** Original post on bio 1696 as attmnt 2180 at 2012-12-26 22:00:00 UTC ***

This patch does all the things mentioned in comment 0 and comment 1 and also takes advantage of the visibility API (https://developer.mozilla.org/en-US/docs/DOM/Using_the_Page_Visibility_API) to avoid all pointless timers in Bubble.

Requesting review from clokep who was the most recently online in #instantbird, but aleth or Mic can feel free to review if they see the patch before clokep and feel good about it.
Attachment #8353942 - Flags: review?(clokep)
Comment on attachment 8353942 [details] [diff] [review]
Patch

*** Original change on bio 1696 attmnt 2180 at 2012-12-27 15:14:14 UTC ***

This change looks fine.

I have plenty of nits about Footer.html, but they all involved changing code that you weren't touching. I think cleaning up that file should be done separately.
Attachment #8353942 - Flags: review?(clokep) → review+
Summary: Bubbles' refreshIntervals function should be rewritten to make timers fire less often → rewrite Bubbles' timer handling so that timers are used only when actually needed
*** Original post on bio 1696 at 2012-12-27 18:35:55 UTC ***

http://hg.instantbird.org/instantbird/rev/caec2bdbc7f5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.