Bug 70077 – Implement WEBGL_debug_shaders and WEBGL_debug_renderer_info extensions

• Component: WebGL
• Status: RESOLVED FIXED

[Zhenyao Mo]

Specs for two extensions:

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/extensions/WEBGL_debug_shaders/index.html
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/extensions/WEBGL_debug_renderer_info/index.html

[Zhenyao Mo]

Created attachment 111055 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Zhenyao Mo]

Whether the current WebGLRenderingContext is for privileged content or not is not implemented in this patch.

The current patch is tested with 

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/extensions/webgl_debug_renderer_info.html
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/extensions/webgl_debug_shaders.html

for both WebGLRenderingContext::allowPrivilegedExtensions() returns true and false.

[Early Warning System Bot]

Comment on attachment 111055 [details]
Patch

Attachment 111055 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10067607

[Zhenyao Mo]

Created attachment 111066 [details]
Patch

[Early Warning System Bot]

Comment on attachment 111066 [details]
Patch

Attachment 111066 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10052674

[Zhenyao Mo]

Created attachment 111079 [details]
Patch

[Zhenyao Mo]

Created attachment 111080 [details]
Patch

[Zhenyao Mo]

Created attachment 111084 [details]
Patch

[Early Warning System Bot]

Comment on attachment 111084 [details]
Patch

Attachment 111084 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10052699

[Zhenyao Mo]

Created attachment 111095 [details]
Patch

[Early Warning System Bot]

Comment on attachment 111095 [details]
Patch

Attachment 111095 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10066667

[Zhenyao Mo]

Created attachment 111111 [details]
Patch

[Early Warning System Bot]

Comment on attachment 111111 [details]
Patch

Attachment 111111 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10066747

[Zhenyao Mo]

Created attachment 111185 [details]
Patch

[Early Warning System Bot]

Comment on attachment 111185 [details]
Patch

Attachment 111185 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10080502

[Patrick R. Gansterer]

Please create _on_ clean enty to the changelog and add the new files to CMakeList, GNUMakeFile and vcproj too.

[Daniel Bates]

Comment on attachment 111185 [details]
Patch

r- per comment 17.

[Zhenyao Mo]

Created attachment 111265 [details]
Patch

[WebKit Review Bot]

Attachment 111265 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Fisher (:fishd, Google)]

Comment on attachment 111265 [details]
Patch

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

> Source/WebKit/chromium/public/WebGraphicsContext3D.h:370
> +    virtual WebString getTranslatedShaderSourceANGLE(WebGLId shader) { return WebString(); }

WebKit API change LGTM

[Zhenyao Mo]

Created attachment 111268 [details]
Patch

[WebKit Review Bot]

Attachment 111268 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
From git://git.webkit.org/WebKit
   a891ab4..3e87a7a  master     -> origin/master
	M	Tools/ChangeLog
	M	Tools/Scripts/webkitdirs.pm
r97622 = ed5ce88c4a887a8a1849fa2e1fe5f707e3a7c311 (refs/remotes/trunk)
	M	Tools/ChangeLog
	M	Tools/Scripts/run-qtwebkit-tests
r97623 = 3e87a7a4343696b900ed25b42741b4e88dcaee01 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zhenyao Mo]

QT bot is finally green.  Please review.

[Kenneth Russell]

Comment on attachment 111268 [details]
Patch

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

The code looks good overall. Couple of issues that need to be cleaned up.

> Source/WebCore/ChangeLog:49
> [phone]  Mark Hahnenberg  <[e-mail]>

There's a merge problem in the ChangeLog. Please unapply, sync and reapply your changes.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2017
> +    }

This block is duplicated from above and needs to be removed.

> Source/WebCore/html/canvas/WebGLRenderingContext.h:630
> +    // Determine if we are running the privileged code in the browser.

"the privileged" -> "privileged". Also, for clarity, I think you should add ", for example, a Safari or Chrome extension".

> Source/WebKit/chromium/ChangeLog:15
> [phone]  Dmitry Lomov  <[e-mail]>

Another merge problem in the ChangeLog.

[Zhenyao Mo]

Created attachment 111682 [details]
Patch

[Zhenyao Mo]

(In reply to comment #25)
> (From update of attachment 111268 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111268&action=review
> 
> The code looks good overall. Couple of issues that need to be cleaned up.
> 
> > Source/WebCore/ChangeLog:49
> > [phone]  Mark Hahnenberg  <[e-mail]>
> 
> There's a merge problem in the ChangeLog. Please unapply, sync and reapply your changes.

Done.

> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2017
> > +    }
> 
> This block is duplicated from above and needs to be removed.

Done.

> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.h:630
> > +    // Determine if we are running the privileged code in the browser.
> 
> "the privileged" -> "privileged". Also, for clarity, I think you should add ", for example, a Safari or Chrome extension".

Done.

> 
> > Source/WebKit/chromium/ChangeLog:15
> > [phone]  Dmitry Lomov  <[e-mail]>
> 
> Another merge problem in the ChangeLog.

Done.

[Kenneth Russell]

Comment on attachment 111682 [details]
Patch

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

Looks good modulo two tiny nits. r=me

> Source/WebCore/ChangeLog:1
> [phone]  Zhenyao Mo  <[e-mail]>

Please make sure the date gets fixed upon landing.

> Source/WebKit/chromium/ChangeLog:1
> [phone]  Zhenyao Mo  <[e-mail]>

Same here.

[Zhenyao Mo]

Committed r97896: <http://trac.webkit.org/changeset/97896>