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)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | .x+ |
People
(Reporter: bholley, Assigned: justin.lebar+bug)
References
Details
Attachments
(1 file)
(deleted),
patch
|
joe
:
review+
joe
:
approval2.0+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Reporter | ||
Comment 2•14 years ago
|
||
(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: - → ?
Updated•14 years ago
|
Assignee: bobbyholley+bmo → joe
Updated•14 years ago
|
Whiteboard: [1 hour]
Updated•14 years ago
|
blocking2.0: final+ → -
Reporter | ||
Comment 4•14 years ago
|
||
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: - → ?
Assignee | ||
Comment 5•14 years ago
|
||
bholley asked me to post a patch for this.
Attachment #499740 -
Flags: review?(joe)
Comment 6•14 years ago
|
||
This just languished too long for 4.0. We will put it in once we branch.
blocking2.0: ? → .x
Comment 7•14 years ago
|
||
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+
Updated•14 years ago
|
Assignee: joe → justin.lebar+bug
Whiteboard: [1 hour]
Comment 8•14 years ago
|
||
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.
Reporter | ||
Comment 9•14 years ago
|
||
(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.
Assignee | ||
Comment 10•14 years ago
|
||
Re-requesting a2.0 based on comments 8 and 9 above.
Assignee | ||
Updated•14 years ago
|
Attachment #499740 -
Flags: approval2.0?
Assignee | ||
Comment 11•14 years ago
|
||
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 12•14 years ago
|
||
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+
Assignee | ||
Comment 13•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•