Bug 35261

Summary: [Skia] crash when attempting to render certain SVGs
Product: WebKit Reporter: Evan Stade <estade>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, evan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 35594    
Bug Blocks:    
Attachments:
Description Flags
patch
levin: review-
added test
none
better description
levin: review+, commit-queue: commit-queue-
test case fixed
levin: review-
fixes
none
skip on mac none

Attachments
patch (1.30 KB, patch)
2010-02-22 14:49 PST, Evan Stade
levin: review-
added test (3.98 KB, patch)
2010-02-25 14:12 PST, Evan Stade
no flags
better description (4.12 KB, patch)
2010-02-25 16:25 PST, Evan Stade
levin: review+
commit-queue: commit-queue-
test case fixed (4.46 KB, patch)
2010-02-26 14:04 PST, Evan Stade
levin: review-
fixes (4.45 KB, patch)
2010-02-26 15:12 PST, Evan Stade
no flags
skip on mac (deleted)
2010-03-03 17:31 PST, Evan Stade
no flags
Evan Stade
Comment 1 2010-02-22 14:49:28 PST
Evan Martin
Comment 2 2010-02-22 15:01:15 PST
needs a test, or which test it fixes
David Levin
Comment 3 2010-02-22 17:07:10 PST
Comment on attachment 49242 [details] patch What Mr. Martin said.
Evan Stade
Comment 4 2010-02-25 14:12:15 PST
Created attachment 49532 [details] added test
Evan Stade
Comment 5 2010-02-25 16:25:18 PST
Created attachment 49541 [details] better description
WebKit Commit Bot
Comment 6 2010-02-26 08:03:02 PST
Comment on attachment 49541 [details] better description Rejecting patch 49541 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 12232 test cases. svg/custom/tiling-regular-hexagonal-crash.svg -> failed Exiting early after 1 failures. 9862 tests run. 215.82s total testing time 9861 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 8 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/313560
Evan Stade
Comment 7 2010-02-26 14:04:12 PST
Created attachment 49634 [details] test case fixed
David Levin
Comment 8 2010-02-26 14:45:19 PST
Comment on attachment 49634 [details] test case fixed r- for "Nobody" which will make cq fail. > Index: WebCore/ChangeLog > +2010-02-25 Evan Stade <estade@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=35261 > + [skia] crash when attempting to render certain SVGs > + > + This fixes the crash, but the SVG still doesn't render properly. > + > + Test: svg/custom/tiling-regular-hexagonal-crash.svg > + > + * platform/graphics/skia/ImageSkia.cpp: > + (WebCore::BitmapImageSingleFrameSkia::create): don't return NULL when > + the copy fails; instead return a blank bitmap. The caller doesn't > + check for NULL before dereferencing. Better to use "0" instead of "NULL" since the code never uses "NULL". > Index: LayoutTests/ChangeLog > +2010-02-25 Evan Stade <estade@chromium.org> > + > + Reviewed by Nobody (OOPS!). This is going to fail to commit because the script greps for NOBODY, so the line won't get replaced and it will fail to commit.
Evan Stade
Comment 9 2010-02-26 15:12:55 PST
Created attachment 49654 [details] fixes done
WebKit Commit Bot
Comment 10 2010-03-02 10:57:31 PST
Comment on attachment 49654 [details] fixes Rejecting patch 49654 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 12240 test cases. fast/loader/api-test-new-window-data-load-base-url.html -> failed Exiting early after 1 failures. 7656 tests run. 126.24s total testing time 7655 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 3 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/320960
David Levin
Comment 11 2010-03-02 11:10:31 PST
Comment on attachment 49654 [details] fixes Setting cq+ again due to bogus cq test failure.
WebKit Commit Bot
Comment 12 2010-03-02 18:37:20 PST
Comment on attachment 49654 [details] fixes Clearing flags on attachment: 49654 Committed r55447: <http://trac.webkit.org/changeset/55447>
WebKit Commit Bot
Comment 13 2010-03-02 18:37:25 PST
All reviewed patches have been landed. Closing bug.
Tony Chang
Comment 14 2010-03-02 22:20:13 PST
(In reply to comment #12) > (From update of attachment 49654 [details]) > Clearing flags on attachment: 49654 > > Committed r55447: <http://trac.webkit.org/changeset/55447> This appears to be crashing on Leopard Intel Debug (tests). http://build.webkit.org/waterfall?show=Leopard%20Intel%20Debug%20(Tests) Should we roll it out or disable the test?
Tony Chang
Comment 15 2010-03-02 22:46:32 PST
Tony Chang
Comment 16 2010-03-02 22:48:07 PST
I (In reply to comment #15) > Committed r55450: <http://trac.webkit.org/changeset/55450> I rolled out the patch+test to unstick the commit queue.
Tony Chang
Comment 17 2010-03-02 23:00:39 PST
I opened bug 35631 for tracking the crash in CG per olliej's request.
Evan Stade
Comment 18 2010-03-03 12:46:14 PST
I am confused, is this patch currently landed or not? I think it should be landed and it should be marked as crashing in leopard debug, if there is some way to do that. Otherwise, I don't exactly get the logic of fixing a crash by removing the test case that exposes the crash, especially when the side effect is to create another crash.
Evan Stade
Comment 19 2010-03-03 17:31:28 PST
Created attachment 49968 [details] skip on mac
WebKit Commit Bot
Comment 20 2010-03-04 00:34:45 PST
Comment on attachment 49968 [details] skip on mac Clearing flags on attachment: 49968 Committed r55509: <http://trac.webkit.org/changeset/55509>
WebKit Commit Bot
Comment 21 2010-03-04 00:34:50 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.