Bug 84457 – [Qt] Make it possible to use WebKit2 with v8

• Component: WebKit2
• Status: RESOLVED WONTFIX

[Balazs Kelemen]

This bug is for making it possible to use v8 in WebKit2, without dealing with WebKitTestRunner for now.

[Balazs Kelemen]

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.

[Balazs Kelemen]

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.

[WebKit Review Bot]

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.

[Build Bot]

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

[Alexey Proskuryakov]

This looks like the kind of project that needs to be announced on webkit-dev. Has it been discussed there already?

[Balazs Kelemen]

(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.

[Maciej Stachowiak]

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.

[Alexey Proskuryakov]

This project has not been accepted per webkit-dev discussion. Shouldn't the bug be marked WONTFIX?

[Balazs Kelemen]

(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.

[Balazs Kelemen]

Created attachment 145105 [details]
Patch

[Build Bot]

Comment on attachment 145105 [details]
Patch

Attachment 145105 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12865295

[Early Warning System Bot]

Comment on attachment 145105 [details]
Patch

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

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Zoltan Herczeg]

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.

[Alexey Proskuryakov]

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.

[Zoltan Herczeg]

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.

[Balazs Kelemen]

(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.

[Anton]

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.

[Balazs Kelemen]

(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.