Bug 32176 - Test that an image's size is valid before reading it.
Summary: Test that an image's size is valid before reading it.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Langley
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-04 15:32 PST by Adam Langley
Modified: 2010-06-16 20:38 PDT (History)
3 users (show)

See Also:


Attachments
patch (3.40 KB, patch)
2009-12-04 15:33 PST, Adam Langley
darin: review-
Details | Formatted Diff | Diff
patch (12.66 KB, patch)
2009-12-08 17:56 PST, Adam Langley
no flags Details | Formatted Diff | Diff
patch (3.42 KB, patch)
2009-12-08 17:57 PST, Adam Langley
fishd: review+
eric: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Langley 2009-12-04 15:32:00 PST
Previously, an image that failed to load (m_failed == true) would trigger an assertion failure when WebKit tried to get its size.
Comment 1 Adam Langley 2009-12-04 15:33:24 PST
Created attachment 44338 [details]
patch

(contains a binary file, so please don't cq+!)
Comment 2 Darin Adler 2009-12-04 15:45:56 PST
Comment on attachment 44338 [details]
patch

>      // Zero-height images can cause problems for some ports.  If we have an
>      // empty image dimension, just bail.
> -    if (size().isEmpty())
> +    if (isSizeAvailable() && size().isEmpty())
>          return 0;

This changes the logic. If the size is not available, this now continues and creates a native image. But the old code would return 0 if the size was not available. You should reverse the logic.

    if (!isSizeAvailable() || ...
Comment 3 Adam Langley 2009-12-08 17:56:05 PST
Created attachment 44503 [details]
patch

I originally thought that size() would return random memory in the case that it hadn't been set, but now I notice that it's constructed to 0, 0. So I've changed the logic as suggested.
Comment 4 Adam Langley 2009-12-08 17:57:17 PST
Created attachment 44504 [details]
patch

Crap. Wrong patch. Corrected.
Comment 5 Eric Seidel (no email) 2009-12-28 22:40:36 PST
Attachment 44504 [details] was posted by a committer and has review+, assigning to Adam Langley for commit.
Comment 6 Eric Seidel (no email) 2009-12-28 23:19:22 PST
Comment on attachment 44504 [details]
patch

This can't be cq'd because it's missing the binary data.
Comment 7 Adam Langley 2009-12-29 09:49:03 PST
r52102
Comment 8 Eric Seidel (no email) 2010-06-16 20:38:10 PDT
This test seems to be (at least recently) crashing on Gtk.