Closed Bug 592131 Opened 14 years ago Closed 14 years ago

Animated images in inactive tabs cause invalidations in the active tab

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

Product:
Core
Component:
Layout: Images, Video, and HTML Frames
Platform:
All
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- beta5+

People

(Reporter: robarnold, Assigned: roc)

References

(
URL
)

Details

Attachments

(3 files)

stack
(deleted), text/plain
Details
patch v1
(deleted), patch
Details | Diff | Splinter Review
my patch
(deleted), patch
tnikkel
: review+
Details | Diff | Splinter Review
STR: 1. In a background tab, open data:text/html,<img src="http://www.garath.net/Sullla/Smilies/rolleyes.gif"> 2. Check CPU usage for the firefox process Expected: ~0% CPU Actual: Steady rate of CPU usage, page faults and paint requests.
Attached file stack (deleted) —
A stack from Rob Arnold.
So I guess the widget for the background tabs being hidden stopped this from being a problem before. We must have some code to stop animated gifs from animating. We must be falling into a bad situation here.
Blocks: 130078
(In reply to comment #2) > So I guess the widget for the background tabs being hidden stopped this from > being a problem before. We must have some code to stop animated gifs from > animating. We actually don't! Images animate in background tabs, and it's actually a tricky problem. Bug 359608 will soon stop them from animating in the bfcache, but background tabs will continue to animate until we figure out how to fast-forward them.
blocking2.0: --- → beta5+
Ok, so it sounds like this is a big problem. We used to avoid painting all our background tabs with animated images, because the widgets were hidden. but post bug 130078, they're not anymore. So we're repainting all our background tabs, which is really bad.
The performance scales with the number of distinct images (that is, opening more copies of the testcase does not make it worse). On my machine, this is chewing up 80% of a core (Core2 duo) with only a few forum tabs open.
All images in background tabs are unlocked, right? Can we not invalidate animated images for background tabs?
Actually, what we should do is, in nsViewportFrame::InvalidateInternal, check if the docshell is hidden. If it is, drop the invalidation on the floor.
Attached patch patch v1 (deleted) — — Splinter Review
Attached a patch per roc's suggestion, though it's completely untested. I don't have a clean objdir right now, and I have to head out. :(
Don't we want the check inside the if (parent) part?
I think we'd also want to drop invalidates that don't come from frame invalidates by checking mIsActive in PresShell::ShouldIgnoreInvalidation.
Anyways, pushed a patch to try server for this.
Attached patch my patch (deleted) — — Splinter Review
This is what I think it should be.
Assignee: nobody → roc
Attachment #470682 - Flags: review?(tnikkel)
Comment on attachment 470682 [details] [diff] [review] my patch Did you see comment 10? Or do you think we don't need to drop in that case?
We should take that part of your try-server patch, but I think it matters a lot less.
Comment on attachment 470682 [details] [diff] [review] my patch (In reply to comment #14) > We should take that part of your try-server patch, but I think it matters a lot > less. Sure, but if we're doing this anyway, may as well do everything. r=me with comment 10 addressed then.
Attachment #470682 - Flags: review?(tnikkel) → review+
Can we get this pushed to mozilla-central with all due haste?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 592596
Depends on: 594438
Depends on: 597774
Product: Core → Core Graveyard
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: