Make it possible to configure QtWebKit to use the V8 JavaScript engine instead of JavaScriptCore. At first, this would be useful for performance benchmarking and quality assurance.
Qt bug tracker entry: http://bugreports.qt.nokia.com/browse/QTBUG-12502
Created attachment 66261 [details] V8 binding changes for QT platform (Symbian and Maemo)
Attachment 66261 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/bindings/v8/V8NPUtils.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/bindings/v8/V8NPObject.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/bindings/v8/V8Proxy.h:43: Alphabetical sorting problem. [build/include_order] [4] WebCore/bindings/v8/NPV8Object.cpp:29: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/bindings/v8/NPV8Object.cpp:122: _NPN_CreateNoScriptObject is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] WebCore/bindings/v8/custom/V8HTMLPlugInElementCustom.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/bindings/v8/npruntime.cpp:29: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebCore/bindings/v8/V8Helpers.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 8 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66262 [details] V8 related changes for QT platform: npruntime_internal.h Remove X11 artifacts
Created attachment 66263 [details] V8 related changes for QT platform: wtf/DateMath.* (Symbian and Maemo) Store cachedUTCOffset and getDSTOffset as hidden properties of global object
Created attachment 66265 [details] V8 related changes for QT platform: eliminate ambiguous choice of CString.h and Animation.h for Symbian
Attachment 66265 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/platform/text/PlatformString.h:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66266 [details] V8 related changes for QT platform: Plugin changes
Created attachment 66267 [details] V8 related changes for QT platform: project file modifications
Attachment 66263 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 JavaScriptCore/wtf/DateMath.cpp:965: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66268 [details] V8 related changes for QT platform: Qt API implementation modifications
Attachment 66266 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/plugins/PluginView.h:45: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66270 [details] V8 related changes for QT platform: DumpRenderTreeSupportQt and FrameLoaderClientQt
Attachment 66270 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:41: Alphabetical sorting problem. [build/include_order] [4] WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:42: Alphabetical sorting problem. [build/include_order] [4] WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:324: One line control clauses should not use braces. [whitespace/braces] [4] WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:340: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 4 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66275 [details] V8 related changes for QT platform: DumpRenderTreeSupportQt and FrameLoaderClientQt
Attachment 66275 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:25: You should add a blank line after implementation file's own header. [build/include_order] [4] WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:42: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 66262 [details] did not build on qt: Build output: http://queues.webkit.org/results/3944035
Created attachment 66289 [details] V8 related changes for QT platform: Plugin changes
Created attachment 66291 [details] V8 related changes for QT platform: npruntime_internal.h Remove X11 artifacts
Created attachment 66292 [details] V8 related changes for QT platform: eliminate ambiguous choice of CString.h and Animation.h for Symbian
Created attachment 66293 [details] V8 related changes for QT platform: wtf/DateMath.* (Symbian and Maemo)
Attachment 66291 [details] did not build on qt: Build output: http://queues.webkit.org/results/3969036
Hi Vlad, interesting work! Three things before we can start proper reviews: 1. Some of your patches replace the entire contents of files with what seems to be mostly the same data. Could this be due to newline format mismatch perhaps? Needs to be fixed. 2. ChangeLog entries are missing. You can use WebKitTools/Scripts/prepare-ChangeLog to generate boilerplate entries. See https://trac.webkit.org/wiki/QtWebKitContrib for more information. 3. (Admittedly of lesser importance) There's both "#if USE(JSC) ... #elif USE(V8) ..." and "#if USE(JSC) ... #else ...". I think we should stick with the first variant.
Attachment 66261 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3949027
First check in and relatevly big... :) Trying to fix those things now (In reply to comment #23) > Hi Vlad, interesting work! > > Three things before we can start proper reviews: > > 1. Some of your patches replace the entire contents of files with what seems to be mostly the same data. Could this be due to newline format mismatch perhaps? Needs to be fixed. > > 2. ChangeLog entries are missing. You can use WebKitTools/Scripts/prepare-ChangeLog to generate boilerplate entries. See https://trac.webkit.org/wiki/QtWebKitContrib for more information. > > 3. (Admittedly of lesser importance) There's both "#if USE(JSC) ... #elif USE(V8) ..." and "#if USE(JSC) ... #else ...". I think we should stick with the first variant.
Attachment 66291 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3927034
Attachment 66293 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3902036
Created attachment 66319 [details] V8 related changes for QT platform: webkit pr* files
Created attachment 66320 [details] V8 related changes for QT platform: webcore pr* files
Created attachment 66321 [details] V8 related changes for QT platform: javascript pr* files. Use wtf only
Created attachment 66322 [details] V8 related changes for QT platform: config.h
Created attachment 66323 [details] V8 related changes for QT platform: datemath mods
Created attachment 66324 [details] V8 related changes for QT platform: v8 bindings
Created attachment 66325 [details] V8 related changes for QT platform: npruntime_internal.h Remove X11 artifacts
Created attachment 66326 [details] V8 related changes for QT platform: eliminate ambiguous choice of CString.h and Animation.h for Symbian
Created attachment 66327 [details] V8 related changes for QT platform: Plugin changes
Created attachment 66328 [details] V8 related changes for QT platform: Qt API implementation modifications
Created attachment 66329 [details] V8 related changes for QT platform: QObject injection implementation
Created attachment 66330 [details] V8 related changes for QT platform: webcoresupport mods
OMG. So many patches attached to one bug. Please consider using a master bug with a bunch of blocking bugs.
Attachment 66329 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 Last 3072 characters of output: t/v8/qt_instancev8.cpp:182: Missing space before ( in foreach( [whitespace/parens] [5] WebCore/bridge/qt/v8/qt_instancev8.cpp:307: Missing space before ( in for( [whitespace/parens] [5] WebCore/bridge/qt/v8/qt_instancev8.cpp:344: Missing spaces around = [whitespace/operators] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:348: One line control clauses should not use braces. [whitespace/braces] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:352: Missing space before ( in foreach( [whitespace/parens] [5] WebCore/bridge/qt/v8/qt_instancev8.cpp:353: Should have a space between // and comment [whitespace/comments] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:357: Missing spaces around = [whitespace/operators] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:360: Should have a space between // and comment [whitespace/comments] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:380: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/bridge/qt/v8/qt_instancev8.cpp:427: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:440: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:498: Missing spaces around = [whitespace/operators] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:510: Extra space before ) [whitespace/parens] [2] WebCore/bridge/qt/v8/qt_instancev8.cpp:519: Missing space before ( in foreach( [whitespace/parens] [5] WebCore/bridge/qt/v8/qt_instancev8.cpp:534: Declaration has space between type name and * in QObject *child [whitespace/declaration] [3] WebCore/bridge/qt/v8/qt_instancev8.cpp:542: Extra space before ) [whitespace/parens] [2] WebCore/bridge/qt/v8/qt_instancev8.cpp:549: Missing spaces around = [whitespace/operators] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:557: Should have a space between // and comment [whitespace/comments] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:583: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:586: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] WebCore/bridge/qt/v8/qt_instancev8.cpp:596: Declaration has space between type name and * in QObject *child [whitespace/declaration] [3] WebCore/bridge/qt/v8/qt_instancev8.cpp:629: Declaration has space between type name and * in QObject *child [whitespace/declaration] [3] WebCore/bridge/qt/v8/qt_instancev8.cpp:578: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 195 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 66330 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:77: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 66325 [details] did not build on qt: Build output: http://queues.webkit.org/results/3915043
Attachment 66323 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3967042
Attachment 66325 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3949036
(In reply to comment #40) > OMG. So many patches attached to one bug. Please consider using a master bug with a bunch of blocking bugs. Second this. Furthermore, separate patches need individual ChangeLogs explaining what's being changed and why - right now it's rather difficult to review these changes :-) Here's an example of a very helpful ChangeLog entry: http://trac.webkit.org/changeset/66334
Created attachment 66375 [details] V8 related changes for QT platform: Plugin changes
Created attachment 66378 [details] V8 related changes for QT platform: wtf/DateMath.* (Symbian and Maemo)
Please close this bug (as invalid), create a new master bug, create one bug for each individual change and set them as blocker for the master bug. This is how we use bugzilla for changes that are closely related to each other.
Attachment 66375 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3924041
Apperently I cannot set it to IGNORED myself.Henry can you do it? (In reply to comment #49) > Please close this bug (as invalid), create a new master bug, create one bug for each individual change and set them as blocker for the master bug. This is how we use bugzilla for changes that are closely related to each other.
Setting this bug invalid on the grounds of the previous comments.
(In reply to comment #51) > Apperently I cannot set it to IGNORED myself.Henry can you do it? Now you can, too, Vlad :)
Comment on attachment 66319 [details] V8 related changes for QT platform: webkit pr* files Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66320 [details] V8 related changes for QT platform: webcore pr* files Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66321 [details] V8 related changes for QT platform: javascript pr* files. Use wtf only Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66322 [details] V8 related changes for QT platform: config.h Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66324 [details] V8 related changes for QT platform: v8 bindings Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66326 [details] V8 related changes for QT platform: eliminate ambiguous choice of CString.h and Animation.h for Symbian Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66328 [details] V8 related changes for QT platform: Qt API implementation modifications Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66329 [details] V8 related changes for QT platform: QObject injection implementation Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66330 [details] V8 related changes for QT platform: webcoresupport mods Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66375 [details] V8 related changes for QT platform: Plugin changes Clearing review flags as these patches have been moved to separate bugs.
Comment on attachment 66378 [details] V8 related changes for QT platform: wtf/DateMath.* (Symbian and Maemo) Clearing review flags as these patches have been moved to separate bugs.