Since http://qt.gitorious.org/qt/qt-script-ng is public now, we can use its version of V8 and the provided functionality to build QtWebKit+V8. Since the version shipped is an older version V8, we need a few ifdef's to make it work, but those should be removed soon, since the work on QtScript is ongoing.
Created attachment 86171 [details] proposed patch
After this patch we can restart the QtWebKit+V8 bot again to use QtScript-NG, and revive our DRT with V8.
Comment on attachment 86171 [details] proposed patch Marking the patch obsolete, since the v8 tree in QtScript is going to be updated in the next few weeks.
Created attachment 94057 [details] proposed patch v2 Based on the previous patch of Andras Becsi <abecsi@webkit.org>.
Comment on attachment 94057 [details] proposed patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=94057&action=review Thanks for taking this. The patch looks good to me, just a few nitpicks which can be fixed when you land it. > Source/WebCore/ChangeLog:9 > + Use the provided V8 and functionality of > + https://bugs.webkit.org/show_bug.cgi?id=56649 to build QtWebKit+V8. Here you probably wanted to add the link to the qtscript repo. > Source/WebCore/WebCore.pri:37 > + lessThan(QT_MAJOR_VERSION, 5) { > + warning("To build QtWebKit+V8 you need qtscript-staging's v8 branch. (See: http://qt.gitorious.org/+qt-developers/qt/qtscript-staging)") > + } This could be an error instead of a warning since it will not build without the specified Qt version, and a warning is easily overseen. > Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:54 > +#if USE(V8) > +#include <wtf/UnusedParam.h> > +#endif Did it cause problems in normal builds without the guard? This does not need to be guarded you can just add the include. > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44 > +#if USE(V8) > +#include <wtf/UnusedParam.h> > +#endif Ditto. > Source/WebCore/page/PageSerializer.cpp:59 > +#if USE(V8) > +#include <wtf/text/CString.h> > +#endif Ditto. > Source/WebCore/page/qt/FrameQt.cpp:27 > +#if USE(V8) > +#include "Document.h" > +#endif Ditto > Source/WebCore/storage/StorageEventDispatcher.cpp:33 > +#if USE(V8) > +#include "Document.h" > +#endif Ditto. > Source/WebKit/qt/WebCoreSupport/ChromeClientQt.cpp:37 > +#if USE(V8) > +#include "Document.h" > +#endif Ditto. > Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:70 > +#if USE(V8) > +#include "ScriptSourceCode.h" > +#endif Ditto.
Created attachment 94060 [details] proposed patch v2.1 Fix the mentioned issues.
Comment on attachment 94060 [details] proposed patch v2.1 View in context: https://bugs.webkit.org/attachment.cgi?id=94060&action=review > Source/WebCore/WebCore.pri:37 > + !exists($${V8_LIB_DIR}$${QMAKE_DIR_SEP}libv8.a): error("Cannot build with V8. Needed library $${V8_LIB_DIR}$${QMAKE_DIR_SEP}libv8.a does not exist.") Hmm, are you sure that this check works with shadow-builds of qtscript-staging? (this can be fixed later though) > Source/WebCore/bindings/v8/ScriptController.cpp:313 > +#if !PLATFORM(QT) > +// FIXME: http://code.google.com/p/v8/source/detail?r=7753 > +// https://bugs.webkit.org/show_bug.cgi?id=60384 > + Is this because of Qt's copy of v8 being outdated? > Source/WebKit/qt/Api/qwebframe.cpp:650 > + engine->globalObject().property(QString::fromLatin1("window")).setProperty(name, v); It would be cleaner to use QLatin1String here. </nitpick> :)
Comment on attachment 94060 [details] proposed patch v2.1 View in context: https://bugs.webkit.org/attachment.cgi?id=94060&action=review The patch in general looks good, r=me. I think the FIXME comment should be fixed though before landing (hence cq-). >> Source/WebCore/bindings/v8/ScriptController.cpp:313 >> + > > Is this because of Qt's copy of v8 being outdated? Please replace this comment with a comment explaining why this code is currently disabled for PLATFORM(QT) and a link to a new bug in bugs.webkit.org that tracks fixing this once the v8 copy in Qt is updated.
(In reply to comment #7) > Hmm, are you sure that this check works with shadow-builds of qtscript-staging? (this can be fixed later though) We don't know how the modularized Qt deployment will work thus this solution is meant to be temporary. Currently we need to copy or make a symlink to the source code into the prefix directory since the V8 source code (include/v8.h) and the library are both in qtscript-staging's src directory. As soon as we know enough about the modularized Qt deployment structure we can modify these checks. > Is this because of Qt's copy of v8 being outdated? Yes, I have opened a bug: https://bugs.webkit.org/show_bug.cgi?id=61187
(In reply to comment #8) > (From update of attachment 94060 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94060&action=review > > The patch in general looks good, r=me. I think the FIXME comment should be fixed though before landing (hence cq-). > > >> Source/WebCore/bindings/v8/ScriptController.cpp:313 > >> + > > > > Is this because of Qt's copy of v8 being outdated? > > Please replace this comment with a comment explaining why this code is currently disabled for PLATFORM(QT) > and a link to a new bug in bugs.webkit.org that tracks fixing this once the v8 copy in Qt is updated. Fixed patch landed in: http://trac.webkit.org/changeset/86949 Closing bug.
Comment on attachment 94060 [details] proposed patch v2.1 Clearing flags.