The patch to check if websocket is available at runtime was lost when JSDOMWindow::webSocket moved to JSDOMWindowWebSocketCustom.cpp.
Created attachment 139213 [details] patch
Attachment 139213 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/platform/blackberry/http/tests..." exit_code: 1 Source/WebCore/bindings/js/JSDOMWindowWebSocketCustom.cpp:32: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 139213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=139213&action=review >> Source/WebCore/bindings/js/JSDOMWindowWebSocketCustom.cpp:32 >> +#include "WebSocket.h" > > Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] This is a false positive. Filed bug 85075. For now this will have to be committed by hand.
Comment on attachment 139213 [details] patch Rejecting attachment 139213 [details] from commit-queue. New failing tests: platform/blackberry/http/tests/websocket/tests/enable-disable-setting.html Full output: http://queues.webkit.org/results/12549321
Created attachment 139263 [details] Archive of layout-test-results from ec2-cq-02 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 139213 [details] patch Rejecting attachment 139213 [details] from commit-queue. New failing tests: platform/blackberry/http/tests/websocket/tests/enable-disable-setting.html Full output: http://queues.webkit.org/results/12555379
Created attachment 139298 [details] Archive of layout-test-results from ec2-cq-01 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Is chromium trying to run a layout test added to layouttests/platform/blackberry?
Comment on attachment 139213 [details] patch Attachment 139213 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12552470 New failing tests: platform/blackberry/http/tests/websocket/tests/enable-disable-setting.html
Created attachment 139331 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 168422 [details] Patch
Attachment 168422 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/bindings/js/JSDOMWindowWebSocketCustom.cpp:31: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 168427 [details] Patch
Comment on attachment 168427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168427&action=review > Source/WebCore/bindings/js/JSDOMWindowWebSocketCustom.cpp:57 > JSValue JSDOMWindow::webSocket(ExecState* exec) const > { > if (!settingsForWindowWebSocket(this)) > return jsUndefined(); > +#if PLATFORM(BLACKBERRY) > + // FIXME: returning undefined is not the same things as disabling the > + // WebSocket object completely. But, this is the best we can do without > + // changing the code generation to check RuntimeEnabledFeatures, like V8 > + // does, which is too big a job at the moment. Remove this check when > + // https://bugs.webkit.org/show_bug.cgi?id=52011 is fixed. > + if (!WebSocket::isAvailable()) > + return jsUndefined(); > +#endif > return getDOMConstructor<JSWebSocketConstructor>(exec, this); > } I don't like this #if PLATFORM(). Does returning jsUndefined when !WebSocket::isAvailable() also make sense to other ports?
Created attachment 168448 [details] Patch
Created attachment 168449 [details] Patch
(In reply to comment #14) > I don't like this #if PLATFORM(). Does returning jsUndefined when !WebSocket::isAvailable() also make sense to other ports? Not really - it's kind of a hack. The correct thing to do is to drop "websocket" from the list of properties entirely, which only works under V8. (There's a bug open about that.) If other ports want to take advantage of this they can add themselves to the ifdef, but it would be rude to turn this behaviour on by default.
This landed in r131204, closing.