Bug 35261 - [Skia] crash when attempting to render certain SVGs
Summary: [Skia] crash when attempting to render certain SVGs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 35594
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-22 14:47 PST by Evan Stade
Modified: 2010-03-04 00:34 PST (History)
4 users (show)

See Also:


Attachments
patch (1.30 KB, patch)
2010-02-22 14:49 PST, Evan Stade
levin: review-
Details | Formatted Diff | Diff
added test (3.98 KB, patch)
2010-02-25 14:12 PST, Evan Stade
no flags Details | Formatted Diff | Diff
better description (4.12 KB, patch)
2010-02-25 16:25 PST, Evan Stade
levin: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
test case fixed (4.46 KB, patch)
2010-02-26 14:04 PST, Evan Stade
levin: review-
Details | Formatted Diff | Diff
fixes (4.45 KB, patch)
2010-02-26 15:12 PST, Evan Stade
no flags Details | Formatted Diff | Diff
skip on mac (deleted)
2010-03-03 17:31 PST, Evan Stade
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Evan Stade 2010-02-22 14:49:28 PST
Created attachment 49242 [details]
patch
Comment 2 Evan Martin 2010-02-22 15:01:15 PST
needs a test, or which test it fixes
Comment 3 David Levin 2010-02-22 17:07:10 PST
Comment on attachment 49242 [details]
patch

What Mr. Martin said.
Comment 4 Evan Stade 2010-02-25 14:12:15 PST
Created attachment 49532 [details]
added test
Comment 5 Evan Stade 2010-02-25 16:25:18 PST
Created attachment 49541 [details]
better description
Comment 6 WebKit Commit Bot 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
Comment 7 Evan Stade 2010-02-26 14:04:12 PST
Created attachment 49634 [details]
test case fixed
Comment 8 David Levin 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.
Comment 9 Evan Stade 2010-02-26 15:12:55 PST
Created attachment 49654 [details]
fixes

done
Comment 10 WebKit Commit Bot 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
Comment 11 David Levin 2010-03-02 11:10:31 PST
Comment on attachment 49654 [details]
fixes

Setting cq+ again due to bogus cq test failure.
Comment 12 WebKit Commit Bot 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>
Comment 13 WebKit Commit Bot 2010-03-02 18:37:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Tony Chang 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?
Comment 15 Tony Chang 2010-03-02 22:46:32 PST
Committed r55450: <http://trac.webkit.org/changeset/55450>
Comment 16 Tony Chang 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.
Comment 17 Tony Chang 2010-03-02 23:00:39 PST
I opened bug 35631 for tracking the crash in CG per olliej's request.
Comment 18 Evan Stade 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.
Comment 19 Evan Stade 2010-03-03 17:31:28 PST
Created attachment 49968 [details]
skip on mac
Comment 20 WebKit Commit Bot 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>
Comment 21 WebKit Commit Bot 2010-03-04 00:34:50 PST
All reviewed patches have been landed.  Closing bug.