Bug 35261 – [Skia] crash when attempting to render certain SVGs

• Component: Platform
• Status: RESOLVED FIXED

[Evan Stade]

http://code.google.com/p/chromium/issues/detail?id=36231

[Evan Stade]

Created attachment 49242 [details]
patch

[Evan Martin]

needs a test, or which test it fixes

[David Levin]

Comment on attachment 49242 [details]
patch

What Mr. Martin said.

[Evan Stade]

Created attachment 49532 [details]
added test

[Evan Stade]

Created attachment 49541 [details]
better description

[WebKit Commit Bot]

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]

Created attachment 49634 [details]
test case fixed

[David Levin]

Comment on attachment 49634 [details]
test case fixed

r- for "Nobody" which will make cq fail.

> Index: WebCore/ChangeLog
> [phone]  Evan Stade  <[e-mail]>
> +
> +        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
> [phone]  Evan Stade  <[e-mail]>
> +
> +        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]

Created attachment 49654 [details]
fixes

done

[WebKit Commit Bot]

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 on attachment 49654 [details]
fixes

Setting cq+ again due to bogus cq test failure.

[WebKit Commit Bot]

Comment on attachment 49654 [details]
fixes

Clearing flags on attachment: 49654

Committed r55447: <http://trac.webkit.org/changeset/55447>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

(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]

Committed r55450: <http://trac.webkit.org/changeset/55450>

[Tony Chang]

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]

I opened bug 35631 for tracking the crash in CG per olliej's request.

[Evan Stade]

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]

Created attachment 49968 [details]
skip on mac

[WebKit Commit Bot]

Comment on attachment 49968 [details]
skip on mac

Clearing flags on attachment: 49968

Committed r55509: <http://trac.webkit.org/changeset/55509>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.