Bug 30018

Summary:

WebGL crashes with recent CanvasArray change

Product:

WebKit

Reporter:

Kenneth Russell <kbr>

Component:

WebGL

Assignee:

Kenneth Russell <kbr>

Status:

RESOLVED FIXED

Severity:

Blocker

CC:

cmarrin, dglazkov, kbr, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Replacement patch eliminating stray tab	oliver: review-
Patch including test case	oliver: review-
Replacement patch with layouttest expected results	oliver: review+

Kenneth Russell

Reported 2009-10-02 08:34:11 PDT

https://bugs.webkit.org/show_bug.cgi?id=29906 introduced a change to the CanvasArray base class which tests a PassRefPtr after its contents have been transferred to a RefPtr, causing the NULL check to always fail and the base pointer of the CanvasArray to always be NULL. This results in crashes on most if not all WebGL content. Patch is attached.

Attachments
Patch (1.16 KB, patch) 2009-10-02 08:41 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Replacement patch eliminating stray tab (1.17 KB, patch) 2009-10-02 08:43 PDT, Kenneth Russell	oliver: review-	Details Formatted Diff Diff
Patch including test case (7.53 KB, patch) 2009-10-02 09:25 PDT, Kenneth Russell	oliver: review-	Details Formatted Diff Diff
Replacement patch with layouttest expected results (9.42 KB, patch) 2009-10-02 10:44 PDT, Chris Marrin	oliver: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Kenneth Russell

Comment 1 2009-10-02 08:41:11 PDT

Created attachment 40519 [details] Patch

Kenneth Russell

Comment 2 2009-10-02 08:43:26 PDT

Created attachment 40520 [details] Replacement patch eliminating stray tab

Oliver Hunt

Comment 3 2009-10-02 08:52:51 PDT

Comment on attachment 40520 [details] Replacement patch eliminating stray tab Can you add a testcase for this?

Kenneth Russell

Comment 4 2009-10-02 09:25:40 PDT

Created attachment 40526 [details] Patch including test case Added unit test. Verified that test crashes before fix and runs successfully after fix.

Oliver Hunt

Comment 5 2009-10-02 09:52:11 PDT

Comment on attachment 40526 [details] Patch including test case > +<html xmlns="http://www.w3.org/1999/xhtml"> we don't use namespaces -- tests should basically be html5 doctype if you want strict mode. But in this case you're also using the xml namespace which is also incorrect due to the file extension not being .xhtml, and not having an xml doctype :D And you need expected output Cheers, Oliver

Dimitri Glazkov (Google)

Comment 6 2009-10-02 09:59:52 PDT

... and don't forget -- WebKit style in JS also. 4-space, func brace on new line, etc. :)

Chris Marrin

Comment 7 2009-10-02 10:44:04 PDT

Created attachment 40533 [details] Replacement patch with layouttest expected results

Oliver Hunt

Comment 8 2009-10-02 10:45:56 PDT

Comment on attachment 40533 [details] Replacement patch with layouttest expected results remove the superfluous namespace decl from <html> in the testcase

Chris Marrin

Comment 9 2009-10-02 10:50:07 PDT

Sending LayoutTests/ChangeLog Adding LayoutTests/fast/canvas/webgl/array-unit-tests-expected.txt Adding LayoutTests/fast/canvas/webgl/array-unit-tests.html Sending WebCore/ChangeLog Sending WebCore/html/canvas/CanvasArray.cpp Transmitting file data ..... Committed revision 49027.

Note You need to log in before you can comment on or make changes to this bug.