Closed
Bug 601470
Opened 14 years ago
Closed 14 years ago
about:memory crash [@ mozilla::imagelib::RasterImage::GetSourceDataSize ] if you load an svg file as an image
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: cork, Assigned: dholbert)
References
Details
(Keywords: crash, crashreportid)
Crash Data
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
If you load an svg file either in an img tag or in css about:memory with mozilla::imagelib::RasterImage::GetSourceDataSize. Crash Reports: bp-85247612-f38f-4f8a-a5a5-7916f2101003 bp-675a88ff-2d01-4cb4-afd2-fa1172101003 bp-4e3879d0-d576-4adf-b301-7914e2101003 bp-4263a542-8323-47d4-8ed5-f003c2101003 bp-df776738-c39f-4576-b526-9c9b32101003 bp-01fd1bfd-9bc9-4f06-86d9-181d32101003 I'm testing this with testcase from bug 600574 https://bugzilla.mozilla.org/attachment.cgi?id=479420
Comment 1•14 years ago
|
||
I tried and failed to reproduce this in FF b6 in my Ubuntu 10.4 VBox.
Assignee | ||
Comment 2•14 years ago
|
||
Confirmed on my desktop at home (running 32-bit Ubuntu 10.04) bp-5af4fb5e-883b-4c7e-a31b-eb95a2101003
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
QA Contact: general → dholbert
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → dholbert
QA Contact: dholbert → general
Assignee | ||
Updated•14 years ago
|
Component: SVG → ImageLib
QA Contact: general → imagelib
Assignee | ||
Updated•14 years ago
|
Assignee | ||
Comment 3•14 years ago
|
||
So, the issue is here: http://hg.mozilla.org/mozilla-central/annotate/dabe4204f1c1/modules/libpr0n/src/imgLoader.cpp#l228 228 RasterImage *image = static_cast<RasterImage*>(req->mImage.get()); 229 if (!image) 230 return PL_DHASH_NEXT; 231 232 if (rtype & RAW_BIT) { 233 arg->value += image->GetSourceDataSize(); 234 } else { 235 arg->value += image->GetDecodedDataSize(); 236 } We need to either (a) not assume that |image| is a RasterImage (b) either... (i) skip the next chunk for vector-type images, or (ii) implement GetSourceDataSize & GetDecodedDataSize for VectorImage (and promote those methods to be declared in the "Image.h" superclass)
Assignee | ||
Comment 4•14 years ago
|
||
> We need to either
Sorry, I meant "We need to..." (do both (a) and part of (b))
Assignee | ||
Updated•14 years ago
|
Keywords: crash,
crashreportid
Summary: about:memory crash with mozilla::imagelib::RasterImage::GetSourceDataSize if you load an svg file as an image → about:memory crash [@ mozilla::imagelib::RasterImage::GetSourceDataSize ] if you load an svg file as an image
Assignee | ||
Comment 5•14 years ago
|
||
This patch: - Promotes GetSourceDataSize & GetDecodedDataSize from RasterImage to Image. - Promotes the (trivial) GetDataSize impl from RasterImage to Image. - Promotes mError from RasterImage to Image, so ^that impl can check mError. - Replaces the silly GetDataSize impl in VectorImage with a (correctly?) silly GetSourceDataSize impl and a XXXTODO silly GetDecodedDataSize impl. (to be fixed in bug 590790) ...and... - Changes the RasterImage* cast that caused this bug's crash to an Image* cast.
Attachment #480564 -
Flags: review?
Assignee | ||
Updated•14 years ago
|
Attachment #480564 -
Flags: review? → review?(joe)
Updated•14 years ago
|
Attachment #480564 -
Flags: review?(joe) → review+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs blocking approval][needs landing]
Comment 6•14 years ago
|
||
This definitely blocks final. However, I'll do you one better and a=b7 this too.
blocking2.0: ? → final+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs blocking approval][needs landing] → [needs landing]
Assignee | ||
Comment 7•14 years ago
|
||
Here's the fix, with a mochitest that just probes all of the entries in Components.interfaces.nsIMemoryReporterManager. (based on aboutMemory.js) Confirmed that this mochitest crashes @ mozilla::imagelib::RasterImage::GetSourceDataSize before-patch, and passes (with no crash) after-patch.
Assignee | ||
Updated•14 years ago
|
Attachment #480761 -
Flags: review?(joe)
Assignee | ||
Updated•14 years ago
|
Attachment #480564 -
Attachment is obsolete: true
Comment 8•14 years ago
|
||
Comment on attachment 480761 [details] [diff] [review] fix with test Add an extra, raster, image, and r=me. (Would prefer to load about:memory in a different tab, but I'm not terribly concerned either way.)
Attachment #480761 -
Flags: review?(joe) → review+
Assignee | ||
Comment 9•14 years ago
|
||
damon.jpg to the rescue!
Attachment #480761 -
Attachment is obsolete: true
Attachment #480766 -
Flags: review+
Assignee | ||
Comment 10•14 years ago
|
||
Landed with a=joe per comment 6: http://hg.mozilla.org/mozilla-central/rev/d262762e4f87
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Reporter | ||
Comment 11•14 years ago
|
||
Verified fixed in Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101007 Firefox/4.0b8pre
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ mozilla::imagelib::RasterImage::GetSourceDataSize ]
You need to log in
before you can comment on or make changes to this bug.
Description
•