Add support for handling WebGL load policy. This includes adding a letting the UI process know when we are attempting to create a webgl context and providing API to query for the WebGL load policy associated with a host.
Created attachment 221084 [details] Patch
Comment on attachment 221084 [details] Patch Attachment 221084 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4710223524659200
Comment on attachment 221084 [details] Patch Attachment 221084 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5356839575224320
<rdar://problem/15790448>
Created attachment 221092 [details] Patch
Comment on attachment 221092 [details] Patch Attachment 221092 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6281278001250304
Created attachment 221094 [details] Patch
Comment on attachment 221094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221094&action=review > Source/WebCore/html/HTMLCanvasElement.cpp:235 > + return 0; nullptr. > Source/WebCore/html/HTMLCanvasElement.cpp:238 > + return 0; ditto.
Comment on attachment 221094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221094&action=review This looks great! My only suggestion would be to make as many of these 'const' as possible. > Source/WebCore/loader/FrameLoaderClient.h:334 > + virtual WebGLLoadPolicy webGLPolicyForHost(const String&) { return WebGLAsk; } Could this be const? > Source/WebKit2/UIProcess/WebLoaderClient.h:108 > + void webGLLoadPolicy(WebPageProxy*, WebCore::WebGLLoadPolicy&, const String&); Const? > Source/WebKit2/UIProcess/WebPageProxy.cpp:2684 > +void WebPageProxy::webGLContextCreated(const String& pageURLString) Const! > Source/WebKit2/UIProcess/WebPageProxy.cpp:2689 > +void WebPageProxy::webGLPolicyForHost(const String& host, uint32_t& loadPolicy) Could probably be const. > Source/WebKit2/UIProcess/WebUIClient.cpp:211 > +void WebUIClient::webGLContextCreated(WebPageProxy* page, const String& originatingURL) Const > Source/WebKit2/UIProcess/API/C/WKAPICast.h:476 > +inline WebCore::WebGLLoadPolicy toWebGLLoadPolicy(WKWebGLLoadPolicy webGLLoadPolicy) Const. > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1382 > +WebCore::WebGLLoadPolicy WebFrameLoaderClient::webGLPolicyForHost(const String& host) Const as well.
Comment on attachment 221094 [details] Patch Attachment 221094 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5550322348982272
Comment on attachment 221094 [details] Patch Attachment 221094 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6036037650350080
(In reply to comment #9) I'm assuming you mean the methods themselves should be const'ified if they're returning something that isn't a pointer (and hence won't be modified) right? All the parameters are about as const'ified as I can make em already. A lot of them are references that definitely shouldn't be const.
Created attachment 221101 [details] Patch
Comment on attachment 221101 [details] Patch Attachment 221101 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/4586082461024256
Comment on attachment 221101 [details] Patch Attachment 221101 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6522478666973184
(In reply to comment #12) > (In reply to comment #9) > > I'm assuming you mean the methods themselves should be const'ified if they're returning something that isn't a pointer (and hence won't be modified) right? Right. These methods don't seem to modify any internal state, so it would be best to mark them const to maximize the methods where you can call your new interface.
Comment on attachment 221101 [details] Patch Attachment 221101 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6668306094227456
(In reply to comment #17) > (From update of attachment 221101 [details]) > Attachment 221101 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.appspot.com/results/6668306094227456 My local run of build-webkit tells me that everything built swimmingly...not sure what's going on here..
(In reply to comment #18) > (In reply to comment #17) > > (From update of attachment 221101 [details] [details]) > > Attachment 221101 [details] [details] did not pass mac-ews (mac): > > Output: http://webkit-queues.appspot.com/results/6668306094227456 > > My local run of build-webkit tells me that everything built swimmingly...not sure what's going on here.. And now EWS is telling me to get rid of the things that it just told me to add in. Will reland the exact same patch again, because I don't trust EWS.
Created attachment 221172 [details] Patch
Comment on attachment 221172 [details] Patch Attachment 221172 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5621195886034944 New failing tests: http/tests/canvas/webgl/origin-clean-conformance.html http/tests/webgl/1.0.2/texSubImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-allowed.html http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html http/tests/webgl/1.0.2/texImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-blocked-no-crossorigin.html
Created attachment 221184 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
It is unsurprising that these tests are now failing seeing as the we won't immediately load because a webglcontext has now been created and those tests are being run from a server. I will skip these tests in a followup patch and will make the correct fix for them in the near future. All previous review comments have been addressed, can I get an r+?
Comment on attachment 221172 [details] Patch Attachment 221172 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5490028792774656 New failing tests: http/tests/canvas/webgl/origin-clean-conformance.html http/tests/webgl/1.0.2/texSubImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-allowed.html http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html http/tests/webgl/1.0.2/texImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-blocked-no-crossorigin.html
Created attachment 221186 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 221172 [details] Patch Attachment 221172 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5138185071886336 New failing tests: http/tests/canvas/webgl/origin-clean-conformance.html http/tests/webgl/1.0.2/texSubImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-allowed.html http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html http/tests/webgl/1.0.2/texImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-blocked-no-crossorigin.html
Created attachment 221188 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 221172 [details] Patch r=me.
Comment on attachment 221172 [details] Patch Attachment 221172 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6404822467084288 New failing tests: http/tests/canvas/webgl/origin-clean-conformance.html http/tests/webgl/1.0.2/texSubImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-allowed.html http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html http/tests/webgl/1.0.2/texImage2DHTML.html http/tests/security/webgl-remote-read-remote-image-blocked-no-crossorigin.html
Created attachment 221193 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
https://trac.webkit.org/r162000
(In reply to comment #28) > (From update of attachment 221172 [details]) > r=me. This needs review from a WebKit2 maintainer. Please roll this out until that happens.
Re-opened since this is blocked by bug 127009
Reping for review. We'd like to get this in ASAP.
Created attachment 221225 [details] WebKit2 Patch
Hello. I'm not an expert but with this patch MiniBrowser with debug build and EFL port does not support WebGL. Try to run Khronos conformance tests: https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html?version=1.0.3 or this sites http://get.webgl.org/ https://www.khronos.org/registry/webgl/sdk/demos/webkit/SpiritBox.html This page fails to create webgl context because this code is called: if (policy == WebGLAsk) { page->chrome().client().webGLContextCreated(this); return nullptr; } In my opinion we have a bug now.
(In reply to comment #35) > Created an attachment (id=221225) [details] > Patch Please rollback 162036 as this patch breaks WebGL on Win, GTK & EFL.
(In reply to comment #37) > (In reply to comment #35) > > Created an attachment (id=221225) [details] [details] > > Patch > > Please rollback 162036 as this patch breaks WebGL on Win, GTK & EFL. I'm seeing all green on Win. Where do you see the failure?
(In reply to comment #38) > (In reply to comment #37) > > (In reply to comment #35) > > > Created an attachment (id=221225) [details] [details] [details] > > > Patch > > > > Please rollback 162036 as this patch breaks WebGL on Win, GTK & EFL. > > I'm seeing all green on Win. Where do you see the failure? Oh you must mean WinCairo. What kind of failures are you seeing?
(In reply to comment #36) > Hello. I'm not an expert but with this patch MiniBrowser with debug build and EFL port does not support WebGL. Try to run Khronos conformance tests: > https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html?version=1.0.3 > > or this sites > http://get.webgl.org/ > https://www.khronos.org/registry/webgl/sdk/demos/webkit/SpiritBox.html > > This page fails to create webgl context because this code is called: > if (policy == WebGLAsk) { > page->chrome().client().webGLContextCreated(this); > return nullptr; > } > > In my opinion we have a bug now. Ah you're right. I'll need to add some sort of check to disable this check when the client doesn't want it. I'll comment it out for now and put a fix me.
In theory, fixed it here: http://trac.webkit.org/changeset/162078 Let me know if it's still broken
Created attachment 221283 [details] WebKit2 Patch v2
Created attachment 221314 [details] WebKit2 patch v3
(In reply to comment #39) > (In reply to comment #38) > > (In reply to comment #37) > > > (In reply to comment #35) > > > > Created an attachment (id=221225) [details] [details] [details] [details] > > > > Patch > > > > > > Please rollback 162036 as this patch breaks WebGL on Win, GTK & EFL. > > > > I'm seeing all green on Win. Where do you see the failure? > > Oh you must mean WinCairo. What kind of failures are you seeing? I didn't mean build break, but that WebGL was not working. Don't know if its working now.
(In reply to comment #44) > > > > Please rollback 162036 as this patch breaks WebGL on Win, GTK & EFL. > > > > > > I'm seeing all green on Win. Where do you see the failure? > > > > Oh you must mean WinCairo. What kind of failures are you seeing? > > I didn't mean build break, but that WebGL was not working. Don't know if its working now. This is now working for me. I think he was referring to the WebGL load policy defaulting to 'off', with no way to activate it. For example, yesterday's OS X nightly couldn't be used for WebGL content because it rejected every WebGL site. That behavior was fixed by your changes yesterday.
Created attachment 221395 [details] Patch
Comment on attachment 221395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221395&action=review > Source/WebKit2/UIProcess/WebLoaderClient.cpp:337 > +WebCore::WebGLLoadPolicy WebLoaderClient::webGLLoadPolicy(WebPageProxy* page, const String& site) const s/site/url/, I think? > Source/WebKit2/UIProcess/WebPageProxy.cpp:2683 > +void WebPageProxy::webGLPolicyForSite(const String& site, uint32_t& loadPolicy) s/Site/URL/, I think? > Source/WebKit2/UIProcess/WebPageProxy.h:930 > + void webGLPolicyForSite(const String& site, uint32_t& loadPolicy); ditto > Source/WebKit2/UIProcess/WebPageProxy.messages.in:37 > + WebGLPolicyForSite(String host) -> (uint32_t loadPolicy) ditto > Source/WebKit2/UIProcess/API/C/WKAPICast.h:488 > + return WebCore::WebGLAsk; your default in other places seems to be Allow; why Ask here? > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1342 > +WebCore::WebGLLoadPolicy WebFrameLoaderClient::webGLPolicyForSite(const String& site) const ditto > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h:190 > + virtual WebCore::WebGLLoadPolicy webGLPolicyForSite(const String&) const OVERRIDE; ditto > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:603 > +WebCore::WebGLLoadPolicy WebPage::webGLPolicyForSite(WebFrame* frame, const String& url) ditto (and here hilariously the argument is url already) > Source/WebKit2/WebProcess/WebPage/WebPage.h:293 > + WebCore::WebGLLoadPolicy webGLPolicyForSite(WebFrame*, const String&); ditto
Committed: http://trac.webkit.org/changeset/162141 All patches and related patches: http://trac.webkit.org/changeset/162141 http://trac.webkit.org/changeset/162099 http://trac.webkit.org/changeset/162078 http://trac.webkit.org/changeset/162036 http://trac.webkit.org/changeset/162002