Closed Bug 593426 Opened 14 years ago Closed 14 years ago

Bump discard timer to something like 1-5 minutes

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: bholley, Assigned: justin.lebar+bug)

References

Details

Attachments

(1 file)

limi+beltzner thought this would be preferable. We should do it pretty late in the cycle, since it'll hamper our ability to detect bugs in discarding. I'm going to propose setting the pref to 1.5 minutes, which means that the actual discarding timeout will range between 1.5 minutes and 3 minutes.
blocking2.0: --- → ?
This doesn't block, but I'd totally take this patch.
blocking2.0: ? → -
(In reply to comment #1) > This doesn't block, but I'd totally take this patch. I think it should. This is the behavior we want, and the only reason we're not doing it now is so that we get better test coverage of our discarding behavior. It's just a 1-line pref patch, and so it won't delay any release. Marking it as a blocker just makes sure that we remember to do it (a useful precaution given that I'm not full-time anymore). Renominating.
blocking2.0: - → ?
Okay, okay.
blocking2.0: ? → final+
Assignee: bobbyholley+bmo → joe
Whiteboard: [1 hour]
blocking2.0: final+ → -
I still think we should do this for ff4. Flipping that pref will have an enormous UX improvement. We specifically set suboptimal prefs to get better testing feedback, and it seems pretty silly to me that we'd ship that way. I haven't really kept up with all of the changes with respect to how we treat blockers, so the best I know to do is to renominate. Let me know if there's something different I should do.
blocking2.0: - → ?
Attached patch Patch v1 (deleted) — Splinter Review
bholley asked me to post a patch for this.
Attachment #499740 - Flags: review?(joe)
This just languished too long for 4.0. We will put it in once we branch.
blocking2.0: ? → .x
Comment on attachment 499740 [details] [diff] [review] Patch v1 Looks fine, but as I mentioned earlier, this can't go in for 4.0.
Attachment #499740 - Flags: review?(joe) → review+
Assignee: joe → justin.lebar+bug
Whiteboard: [1 hour]
What's the potential risk of taking this? I think upping the timer will probably mean fewer image reloads, though admittedly I can't say that I'm feeling ill effects from the current timeout in the nightlies, so if there's any risk, I agree with Joe that there's no strong need to take it.
(In reply to comment #8) > What's the potential risk of taking this? I think upping the timer will > probably mean fewer image reloads, though admittedly I can't say that I'm > feeling ill effects from the current timeout in the nightlies, so if there's > any risk, I agree with Joe that there's no strong need to take it. I don't believe there's any risk other than the standard "it's a patch" risk. We'd just be changing an arbitrary value, so no code paths would change. In the common case it doesn't make much different. However it does make a difference when tabbing back to a page with a lot of large images. If they're just under the sync threshold then things will hang momentarily, and if they're over it things will flicker.
Re-requesting a2.0 based on comments 8 and 9 above.
Attachment #499740 - Flags: approval2.0?
This is pretty noticeable on image-heavy sites, such as [1]. Open it, wait 30s or so, and then switch back. There's a period of a second or two as the images are re-decoded where they're all blank and FF is less responsive. [1] http://www.boston.com/bigpicture/2011/01/protest_spreads_in_the_middle.html
Comment on attachment 499740 [details] [diff] [review] Patch v1 If this doesn't land in the next 24 hours, consider your approval RESCINDED!!!
Attachment #499740 - Flags: approval2.0? → approval2.0+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
No longer depends on: 631733
Depends on: 660577
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: