WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
106162
Pending container size requests can be cleared earlier
https://bugs.webkit.org/show_bug.cgi?id=106162
Summary
Pending container size requests can be cleared earlier
Philip Rogers
Reported
2013-01-04 18:49:34 PST
CachedImage::CreateImage() only clears m_pendingContainerSizeRequests if for images that use container sizes due to a trivial mistake in
http://trac.webkit.org/changeset/137981
Attachments
First pass
(2.16 KB, patch)
2013-01-07 11:18 PST
,
Philip Rogers
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Philip Rogers
Comment 1
2013-01-07 11:18:32 PST
Created
attachment 181535
[details]
First pass
Eric Seidel (no email)
Comment 2
2013-01-07 11:31:07 PST
Comment on
attachment 181535
[details]
First pass Were they never being cleared? Was this effectively a leak?
Eric Seidel (no email)
Comment 3
2013-01-07 11:51:13 PST
I guess my question is... would m_pendingContainerSizeRequests have grown w/o bounds before this? If so, it seems trivial to make a page which would trigger that. In general do we worry about this vector growing without bounds?
Philip Rogers
Comment 4
2013-01-07 12:03:14 PST
(In reply to
comment #2
)
> (From update of
attachment 181535
[details]
) > Were they never being cleared? Was this effectively a leak?
This isn't a leak because they would still be cleared when we destructed the CachedImage, we just keep them around too long. This issue only exists because in
https://bugs.webkit.org/show_bug.cgi?id=105097#c3
you asked for me to clear these requests and I made the change without another round of reviews. This patch is just cleaning up my unreviewed mess :) (In reply to
comment #3
)
> I guess my question is... would m_pendingContainerSizeRequests have grown w/o bounds before this? If so, it seems trivial to make a page which would trigger that. In general do we worry about this vector growing without bounds?
m_pendingContainerSizeRequests should only be used between the time an image is requested and when it is created. Because these requests are stored as a pair of the renderer and the size, this vector could only grow massive if there were an infinite number of renderers all referencing a single image where all the renderers layout before the image loads.
Eric Seidel (no email)
Comment 5
2013-01-07 12:05:24 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > I guess my question is... would m_pendingContainerSizeRequests have grown w/o bounds before this? If so, it seems trivial to make a page which would trigger that. In general do we worry about this vector growing without bounds? > > m_pendingContainerSizeRequests should only be used between the time an image is requested and when it is created. Because these requests are stored as a pair of the renderer and the size, this vector could only grow massive if there were an infinite number of renderers all referencing a single image where all the renderers layout before the image loads.
I see. So if you change the size of a loading-image after initial layout it does not create an additional pending size request? So an "attacker" of this vector would have to create N images in order to create N size change requests?
Philip Rogers
Comment 6
2013-01-07 12:13:15 PST
(In reply to
comment #5
)
> (In reply to
comment #4
) > > (In reply to
comment #3
) > > > I guess my question is... would m_pendingContainerSizeRequests have grown w/o bounds before this? If so, it seems trivial to make a page which would trigger that. In general do we worry about this vector growing without bounds? > > > > m_pendingContainerSizeRequests should only be used between the time an image is requested and when it is created. Because these requests are stored as a pair of the renderer and the size, this vector could only grow massive if there were an infinite number of renderers all referencing a single image where all the renderers layout before the image loads. > > I see. So if you change the size of a loading-image after initial layout it does not create an additional pending size request? So an "attacker" of this vector would have to create N images in order to create N size change requests?
Yeah, you've got it exactly right.
Eric Seidel (no email)
Comment 7
2013-01-07 12:14:05 PST
Comment on
attachment 181535
[details]
First pass OK, so this really is then a perf/memory fix, and not a leak or security fix. Thanks.
WebKit Review Bot
Comment 8
2013-01-07 12:35:14 PST
Comment on
attachment 181535
[details]
First pass Clearing flags on attachment: 181535 Committed
r138976
: <
http://trac.webkit.org/changeset/138976
>
WebKit Review Bot
Comment 9
2013-01-07 12:35:18 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug