RESOLVED FIXED 99102
SVGImage::data() returns NULL 
https://bugs.webkit.org/show_bug.cgi?id=99102
Summary SVGImage::data() returns NULL
Jay Civelli
Reported 2012-10-11 14:55:17 PDT
Calling SVGImage::data() returns NULL. It should return the actual SVG data.
Attachments
First pass (1.83 KB, patch)
2012-10-23 13:45 PDT, Philip Rogers 		eric: review+
DetailsFormatted DiffDiff
Add back null check per Jay's request (1.84 KB, patch)
2012-10-23 17:10 PDT, Philip Rogers 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Updating reviewer (1.84 KB, patch)
2012-10-23 17:30 PDT, Philip Rogers 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jay Civelli
Comment 1 2012-10-12 10:08:46 PDT
One way to repro is to run the Chromium webkit_unit_tests, more specifically WebPageNewSerializeTest,SVGImageDontCrash. webkit_unit_tests --gtest_filter=WebPageNewSerializeTest.SVGImageDontCrash In PageSerializer::addImageToResources() there is a LOG_ERROR that's printed when the SVG image content is missing.
Alexey Proskuryakov
Comment 2 2012-10-12 11:53:07 PDT
Is there any user observable effect?
Jay Civelli
Comment 3 2012-10-12 12:50:53 PDT
The only effect I am aware of is that MHTML generated for a page with SVG images will not include them (as it does not get the data).
Philip Rogers
Comment 4 2012-10-22 09:56:20 PDT
@Jay, were you able to create a repro for this (even if just in c++)?
Jay Civelli
Comment 5 2012-10-22 10:02:50 PDT
@Philip You can repro with the steps I described in comment #1. It's a unit-test that generate MHTML from a simple page with an SVG image.
Philip Rogers
Comment 6 2012-10-22 20:51:51 PDT
It looks like the image returned from imageForRenderer() does not contain the copied data() but image->image()->data() does. I should have a patch up tomorrow for this.
Philip Rogers
Comment 7 2012-10-23 13:45:52 PDT
Created attachment 170227 [details] First pass
Eric Seidel (no email)
Comment 8 2012-10-23 16:16:34 PDT
Comment on attachment 170227 [details] First pass LGTM. Do we know if there is a way to test this with LayoutTests instead of just a Chromium Unit Test?
Jay Civelli
Comment 9 2012-10-23 16:43:28 PDT
LGTM @eric We can't test that from a Layout test as page serialization cannot be triggered from a page.
Philip Rogers
Comment 10 2012-10-23 17:10:43 PDT
Created attachment 170271 [details] Add back null check per Jay's request
WebKit Review Bot
Comment 11 2012-10-23 17:27:55 PDT
Comment on attachment 170271 [details] Add back null check per Jay's request Rejecting attachment 170271 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/14529170
Philip Rogers
Comment 12 2012-10-23 17:30:00 PDT
Created attachment 170276 [details] Updating reviewer
WebKit Review Bot
Comment 13 2012-10-23 20:21:23 PDT
Comment on attachment 170276 [details] Updating reviewer Clearing flags on attachment: 170276 Committed r132295: <http://trac.webkit.org/changeset/132295>
WebKit Review Bot
Comment 14 2012-10-23 20:21:26 PDT
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.