Bug 89583 - BlackBerry port does not sanitize GLSL code with ANGLE
Summary: BlackBerry port does not sanitize GLSL code with ANGLE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-20 11:18 PDT by Joshua Netterfield
Modified: 2012-06-22 13:10 PDT (History)
10 users (show)

See Also:

Attachments
Patch (10.71 KB, patch)
2012-06-20 11:50 PDT, Joshua Netterfield 		no flags Details | Formatted Diff | Diff
Patch (10.71 KB, patch)
2012-06-20 11:57 PDT, Joshua Netterfield 		no flags Details | Formatted Diff | Diff
Patch (10.71 KB, patch)
2012-06-21 12:02 PDT, Joshua Netterfield 		no flags Details | Formatted Diff | Diff
Patch (9.54 KB, patch)
2012-06-21 12:21 PDT, Joshua Netterfield 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Netterfield 2012-06-20 11:18:10 PDT 
The BlackBerry port does not sanitize GLSL code with ANGLE like all other ports.

It should.
Comment 1 Joshua Netterfield 2012-06-20 11:50:04 PDT 
Created attachment 148616 [details]
Patch
Comment 2 Joshua Netterfield 2012-06-20 11:57:32 PDT 
Created attachment 148619 [details]
Patch
Comment 3 Konrad Piascik 2012-06-21 11:36:14 PDT 
Comment on attachment 148619 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148619&action=review

I don't know enough about the affected files to provide a full review but these style issues should be fixed.

> Source/ThirdParty/ANGLE/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

you need to specify what changes were made.  The bug title is not descriptive enough to describe what changes are needed in all these files.

> Source/WebCore/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

ditto

> Source/WebKit/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

ditto

> Source/WebCore/platform/graphics/GraphicsContext3D.h:82
> +#if (PLATFORM(CHROMIUM) && USE(SKIA))

There shouldn't be a need to add these brackets

> ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

ditto
Comment 4 Joshua Netterfield 2012-06-21 12:02:50 PDT 
Created attachment 148856 [details]
Patch
Comment 5 Konrad Piascik 2012-06-21 12:11:46 PDT 
Comment on attachment 148856 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148856&action=review

> Source/WebCore/CMakeLists.txt:-2325
> -        ${THIRDPARTY_DIR}/ANGLE/src/compiler/preprocessor/atom.c

why did you move these files lower? Alpha order seems to be case insensitive in this list.

> ChangeLog:10
> +        * Source/cmake/OptionsBlackBerry.cmake:

need description of change
Comment 6 Joshua Netterfield 2012-06-21 12:21:44 PDT 
Created attachment 148862 [details]
Patch
Comment 7 Konrad Piascik 2012-06-22 07:33:32 PDT 
The patch looks good to me but I'd rather have someone who's more familiar with WebGL review this to make sure that we're not breaking anything, both on our port and for everyone else.
Comment 8 Rob Buis 2012-06-22 12:28:16 PDT 
Comment on attachment 148862 [details]
Patch

Looks good.
Comment 9 WebKit Review Bot 2012-06-22 13:10:27 PDT 
Comment on attachment 148862 [details]
Patch

Clearing flags on attachment: 148862

Committed r121051: <http://trac.webkit.org/changeset/121051>
Comment 10 WebKit Review Bot 2012-06-22 13:10:34 PDT 
All reviewed patches have been landed.  Closing bug.