This bug is for making it possible to use v8 in WebKit2, without dealing with WebKitTestRunner for now.
Created attachment 138104 [details] WIP patch to build WK2 with v8 It's just a first step. I wanted to know how many dependencies there are so I just workarounded all usage of JSC (API or internals) everywhere with ifdefs and mock code. The injected bundle is pretty broken with this patch as well as plugins but MiniBrowser seems to works fine basically.
One more thing: in the patch I introduced some wrappers for the JSC API (V8JSCAPIShims). Currently these are just some headers copied over from JavaScriptCore/API. The plan is to provide shims for the most important parts of the API so it will be sufficient for WTR.
Attachment 138104 [details] did not pass style-queue: Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:73: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:79: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:87: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:96: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:108: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSStringRef.h:122: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:343: Extra space between int and version [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:344: Extra space between JSClassAttributes and attributes [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:347: Extra space between JSClassRef and parentClass [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:352: Extra space between JSObjectInitializeCallback and initialize [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:353: Extra space between JSObjectFinalizeCallback and finalize [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:354: Extra space between JSObjectHasPropertyCallback and hasProperty [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:355: Extra space between JSObjectGetPropertyCallback and getProperty [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:356: Extra space between JSObjectSetPropertyCallback and setProperty [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:357: Extra space between JSObjectDeletePropertyCallback and deleteProperty [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:358: Extra space between JSObjectGetPropertyNamesCallback and getPropertyNames [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:359: Extra space between JSObjectCallAsFunctionCallback and callAsFunction [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:360: Extra space between JSObjectCallAsConstructorCallback and callAsConstructor [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:361: Extra space between JSObjectHasInstanceCallback and hasInstance [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:362: Extra space between JSObjectConvertToTypeCallback and convertToType [whitespace/declaration] [3] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:381: The parameter name "definition" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:389: The parameter name "jsClass" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:396: The parameter name "jsClass" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:409: The parameter name "jsClass" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:419: The parameter name "callAsFunction" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:430: The parameter name "jsClass" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:430: The parameter name "callAsConstructor" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:501: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:510: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:510: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:519: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:530: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:542: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:542: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:542: The parameter name "attributes" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:553: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:565: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:577: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:577: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:585: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:595: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:604: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:617: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:626: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:638: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:647: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:655: The parameter name "array" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:662: The parameter name "array" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:670: The parameter name "array" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:679: The parameter name "array" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSObjectRef.h:687: The parameter name "accumulator" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:51: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:65: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:74: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:83: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:92: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:101: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:110: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:119: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:129: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:129: The parameter name "jsClass" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:142: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:152: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:163: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:209: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:220: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:231: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:242: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:252: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:262: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:272: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:284: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSValueRef.h:294: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit2/WebProcess/qt/QtBuiltinBundlePage.h:30: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/V8JSCAPIShims/JSContextRef.h:41: The parameter name "group" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSContextRef.h:42: The parameter name "group" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/JSContextRef.h:44:Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/JavaScriptCore.pri',..." exit_code: 1 The parameter name "group" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/V8JSCAPIShims/APICast.h:4: Alphabetical sorting problem. [build/include_order] [4] Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:64: Should have a space between // and comment [whitespace/comments] [4] Source/JavaScriptCore/V8JSCAPIShims/JavaScript.h:6: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 81 in 54 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 138104 [details] WIP patch to build WK2 with v8 Attachment 138104 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12467260
This looks like the kind of project that needs to be announced on webkit-dev. Has it been discussed there already?
(In reply to comment #5) > This looks like the kind of project that needs to be announced on webkit-dev. Has it been discussed there already? You're right, I have just sent about it.
Comment on attachment 138104 [details] WIP patch to build WK2 with v8 View in context: https://bugs.webkit.org/attachment.cgi?id=138104&action=review > Source/JavaScriptCore/V8JSCAPIShims/APICast.h:9 > +#ifndef APICast_h > +#define APICast_h > + > +#include <v8.h> > +#include "JSBase.h" > + > +// Add toV8 converters. > + > +#endif JSC is not the right place to put these kinds of API shims. I don't know what the right place is (maybe in the v8 repository?) but you can't expect JSC developers to maintain an API layer on top of V8.
This project has not been accepted per webkit-dev discussion. Shouldn't the bug be marked WONTFIX?
(In reply to comment #8) > This project has not been accepted per webkit-dev discussion. Shouldn't the bug be marked WONTFIX? Well, it doesn't hurt if you see how it looks like, so I'm going to upload my patch.
Created attachment 145105 [details] Patch
Comment on attachment 145105 [details] Patch Attachment 145105 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12865295
Comment on attachment 145105 [details] Patch Attachment 145105 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12863281
Comment on attachment 145105 [details] Patch Attachment 145105 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12859649 New failing tests: inspector/extensions/extensions-reload.html
Created attachment 145269 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
I thnik the general direction of this patch is good, but we should reduce the so many #if USE directives. Changes like this is nice, since it improves readibilty of the code: - JSC::initializeThreading(); - WTF::initializeMainThread(); + ScriptController::initializeThreading(); However, in other cases the JSC layer has better abstraction of the general JavaScript language. Look at this: + CoreIPC::DataReference dataReference() const + { +#if USE(V8) + if (!m_buffer.isEmpty()) + return m_buffer; + + String wireString = m_serializedScriptValue->toWireString(); + unsigned lengthInBytes = wireString.length(); + const uint8_t* characters; + if (wireString.is8Bit()) + characters = static_cast<const uint8_t*>(wireString.characters8()); + else { + lengthInBytes *= 2; + characters = reinterpret_cast<const uint8_t*>(wireString.characters16()); + } + m_buffer = Vector<uint8_t>(lengthInBytes); + memcpy(m_buffer.data(), characters, lengthInBytes); + return m_buffer; +#else + return m_serializedScriptValue->data(); +#endif + } I think we should provide the same functionality with v8, so we should keep this as a single line "return m_serializedScriptValue->data();" without any #ifs.
Comment on attachment 145105 [details] Patch You didn't clear the review flag as suggested, so r-'ing and WONTFIXing per complete lack of buy-in in webkit-dev.
This is really important for Qt, since a single engine for a phone is the preferred approach for maintainability reasons. We also think WebKit2 is also an excellent API, and it would truly benefit from multiple engines. So we keep this discussion open, and try to get more supporters as you requested.
(In reply to comment #17) > This is really important for Qt, since a single engine for a phone is the preferred approach for maintainability reasons. It's not really about maintainability but rather memory usage.
Hello. I've a problem after Tools/Scripts/build-webkit --qt --v8 In file included from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/JSBase.h:70:0, from /home/anton/qtwebkit-v8/Source/JavaScriptCore/API/JSObjectRef.h:30, from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/ClassWrapper.h:30, from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/ClassWrapper.cpp:28: /home/anton/qtwebkit-v8/Source/WebKit/qt/v8/ForwardingHeaders/v8.h:1:24: fatal error: private/v8.h: No such file or directory compilation terminated. make[2]: *** [obj/release/JavaScriptCore/ClassWrapper.o] Error 1 make[2]: Leaving directory `/home/anton/qtwebkit-v8/WebKitBuild/Release/Source/WebCore/bindings/V8JSCAPIShim' make[1]: *** [sub-Source-WebCore-bindings-V8JSCAPIShim-V8JSCAPIShim-pro-make_default-ordered] Error 2 make[1]: Leaving directory `/home/anton/qtwebkit-v8/WebKitBuild/Release' make: *** [incremental] Error 2 Fedora 17, qt5 Can anybody help? Thanks.
(In reply to comment #19) > Hello. I've a problem after Tools/Scripts/build-webkit --qt --v8 > > In file included from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/JSBase.h:70:0, > from /home/anton/qtwebkit-v8/Source/JavaScriptCore/API/JSObjectRef.h:30, > from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/ClassWrapper.h:30, > from /home/anton/qtwebkit-v8/Source/WebCore/bindings/V8JSCAPIShim/JavaScriptCore/ClassWrapper.cpp:28: > /home/anton/qtwebkit-v8/Source/WebKit/qt/v8/ForwardingHeaders/v8.h:1:24: fatal error: private/v8.h: No such file or directory > compilation terminated. > make[2]: *** [obj/release/JavaScriptCore/ClassWrapper.o] Error 1 > make[2]: Leaving directory `/home/anton/qtwebkit-v8/WebKitBuild/Release/Source/WebCore/bindings/V8JSCAPIShim' > make[1]: *** [sub-Source-WebCore-bindings-V8JSCAPIShim-V8JSCAPIShim-pro-make_default-ordered] Error 2 > make[1]: Leaving directory `/home/anton/qtwebkit-v8/WebKitBuild/Release' > make: *** [incremental] Error 2 > > Fedora 17, qt5 > > Can anybody help? Thanks. This work has been abandoned. Why do you try to use it? Please answer to me in private, we should not continue the discussion here.