Summary: | Remove ENABLE_WORKERS | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Laszlo Gombos <laszlo.gombos> | ||||||||||||||||||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
Severity: | Normal | CC: | abarth, benjamin, buildbot, commit-queue, eric, hausmann, joepeck, ossy, paroga, pmolnar.u-szeged, rniwa, timothy | ||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||
Bug Depends on: | 108099 | ||||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||||
Attachments: |
|
Description
Laszlo Gombos
2012-12-26 23:40:22 PST
Partick, it seems to me that the Wince port is the only port that does not have WORKERS enabled. One of the evidence is that http://trac.webkit.org/changeset/140908 change was not needed for WinCE. Can you expand if/when would it be possible to enable WORKERS ? Is this a WinCE platform limitation or a policy/lack of implementation of the WinCE port ? (In reply to comment #1) > Partick, it seems to me that the Wince port is the only port that does not have WORKERS enabled. One of the evidence is that http://trac.webkit.org/changeset/140908 change was not needed for WinCE. > > Can you expand if/when would it be possible to enable WORKERS ? Is this a WinCE platform limitation or a policy/lack of implementation of the WinCE port ? WinCE port compiles with enabled WORKERS too. So feel free to do with the ENABLE_WORKERS whatever you want to do. ;-) > WinCE port compiles with enabled WORKERS too. Thanks Patrick. Filed bug 108099 to actually enable it before removing the flag. Created attachment 191125 [details]
1st take
Comment on attachment 191125 [details] 1st take Attachment 191125 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/16901032 Comment on attachment 191125 [details] 1st take Attachment 191125 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/16792678 Comment on attachment 191125 [details]
1st take
Qt builds need to force a clean build, incremental builds does not seems to work.
Comment on attachment 191125 [details] 1st take View in context: https://bugs.webkit.org/attachment.cgi?id=191125&action=review LGTM. Please fix Qt before landing. > Source/WebCore/bindings/cpp/WebDOMEventTarget.cpp:112 > +#if 0 I would just remove this #if 0 code while your'e there. It shoudl have been removed long ago. Created attachment 215506 [details]
updated the patch, submitting only for EWS analysis.
Attachment 215506 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/FeatureDefines.h', u'Source/WTF/wtf/nix/FeatureDefinesNix.h', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/Configurations/FeatureDefines.xcconfig', u'Source/WebCore/Modules/indexeddb/IDBFactory.cpp', u'Source/WebCore/Modules/indexeddb/WorkerGlobalScopeIndexedDatabase.cpp', u'Source/WebCore/Modules/indexeddb/WorkerGlobalScopeIndexedDatabase.h', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.h', u'Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp', u'Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h', u'Source/WebCore/UseJSC.cmake', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/bindings/cpp/WebDOMEventTarget.cpp', u'Source/WebCore/bindings/generic/ActiveDOMCallback.cpp', u'Source/WebCore/bindings/js/DOMRequestState.h', u'Source/WebCore/bindings/js/JSDOMGlobalObject.cpp', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/js/JSDedicatedWorkerGlobalScopeCustom.cpp', u'Source/WebCore/bindings/js/JSEventListener.cpp', u'Source/WebCore/bindings/js/JSWorkerCustom.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.h', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp', u'Source/WebCore/bindings/js/ScheduledAction.cpp', u'Source/WebCore/bindings/js/ScheduledAction.h', u'Source/WebCore/bindings/js/ScriptProfiler.cpp', u'Source/WebCore/bindings/js/ScriptProfiler.h', u'Source/WebCore/bindings/js/ScriptState.cpp', u'Source/WebCore/bindings/js/ScriptState.h', u'Source/WebCore/bindings/js/WorkerScriptController.cpp', u'Source/WebCore/bindings/js/WorkerScriptController.h', u'Source/WebCore/bindings/js/WorkerScriptDebugServer.cpp', u'Source/WebCore/bindings/js/WorkerScriptDebugServer.h', u'Source/WebCore/dom/EventTargetFactory.in', u'Source/WebCore/dom/MessagePort.cpp', u'Source/WebCore/dom/ScriptExecutionContext.cpp', u'Source/WebCore/inspector/CodeGeneratorInspector.py', u'Source/WebCore/inspector/InspectorConsoleInstrumentation.h', u'Source/WebCore/inspector/InspectorController.cpp', u'Source/WebCore/inspector/InspectorInstrumentation.cpp', u'Source/WebCore/inspector/InspectorInstrumentation.h', u'Source/WebCore/inspector/InspectorProfilerAgent.cpp', u'Source/WebCore/inspector/InspectorProfilerAgent.h', u'Source/WebCore/inspector/InspectorWorkerAgent.cpp', u'Source/WebCore/inspector/InspectorWorkerAgent.h', u'Source/WebCore/inspector/InspectorWorkerResource.h', u'Source/WebCore/inspector/InstrumentingAgents.cpp', u'Source/WebCore/inspector/InstrumentingAgents.h', u'Source/WebCore/inspector/WorkerConsoleAgent.cpp', u'Source/WebCore/inspector/WorkerConsoleAgent.h', u'Source/WebCore/inspector/WorkerDebuggerAgent.cpp', u'Source/WebCore/inspector/WorkerDebuggerAgent.h', u'Source/WebCore/inspector/WorkerInspectorController.cpp', u'Source/WebCore/inspector/WorkerInspectorController.h', u'Source/WebCore/inspector/WorkerRuntimeAgent.cpp', u'Source/WebCore/inspector/WorkerRuntimeAgent.h', u'Source/WebCore/loader/ThreadableLoader.cpp', u'Source/WebCore/loader/WorkerThreadableLoader.cpp', u'Source/WebCore/loader/WorkerThreadableLoader.h', u'Source/WebCore/loader/cache/MemoryCache.cpp', u'Source/WebCore/page/WorkerNavigator.cpp', u'Source/WebCore/page/WorkerNavigator.h', u'Source/WebCore/page/WorkerNavigator.idl', u'Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/workers/AbstractWorker.cpp', u'Source/WebCore/workers/AbstractWorker.h', u'Source/WebCore/workers/AbstractWorker.idl', u'Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp', u'Source/WebCore/workers/DedicatedWorkerGlobalScope.h', u'Source/WebCore/workers/DedicatedWorkerGlobalScope.idl', u'Source/WebCore/workers/DedicatedWorkerThread.cpp', u'Source/WebCore/workers/DedicatedWorkerThread.h', u'Source/WebCore/workers/Worker.cpp', u'Source/WebCore/workers/Worker.h', u'Source/WebCore/workers/Worker.idl', u'Source/WebCore/workers/WorkerGlobalScope.cpp', u'Source/WebCore/workers/WorkerGlobalScope.h', u'Source/WebCore/workers/WorkerGlobalScope.idl', u'Source/WebCore/workers/WorkerGlobalScopeProxy.h', u'Source/WebCore/workers/WorkerLoaderProxy.h', u'Source/WebCore/workers/WorkerLocation.cpp', u'Source/WebCore/workers/WorkerLocation.h', u'Source/WebCore/workers/WorkerLocation.idl', u'Source/WebCore/workers/WorkerMessagingProxy.cpp', u'Source/WebCore/workers/WorkerMessagingProxy.h', u'Source/WebCore/workers/WorkerObjectProxy.h', u'Source/WebCore/workers/WorkerReportingProxy.h', u'Source/WebCore/workers/WorkerRunLoop.cpp', u'Source/WebCore/workers/WorkerRunLoop.h', u'Source/WebCore/workers/WorkerScriptLoader.cpp', u'Source/WebCore/workers/WorkerScriptLoader.h', u'Source/WebCore/workers/WorkerScriptLoaderClient.h', u'Source/WebCore/workers/WorkerThread.cpp', u'Source/WebCore/workers/WorkerThread.h', u'Source/WebKit/ChangeLog', u'Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Configurations/FeatureDefines.xcconfig', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/WebWorkersPrivate.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Configurations/FeatureDefines.xcconfig', u'Source/autotools/SetupWebKitFeatures.m4', u'Source/cmake/WebKitFeatures.cmake', u'Source/cmakeconfig.h.cmake', u'Tools/ChangeLog', u'Tools/Scripts/webkitperl/FeatureList.pm', u'WebKitLibraries/ChangeLog', u'WebKitLibraries/win/tools/vsprops/FeatureDefines.props', u'WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.props']" exit_code: 1
Source/WebCore/UseJSC.cmake:120: Alphabetical sorting problem. "bindings/js/JSWorkerCustom.cpp" should be before "bindings/js/JSWorkerGlobalScopeCustom.cpp". [list/order] [5]
Total errors found: 1 in 41 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 215509 [details]
updated the patch, submitting only for EWS analysis - style fixed
Comment on attachment 215509 [details] updated the patch, submitting only for EWS analysis - style fixed Attachment 215509 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/17138215 New failing tests: fast/workers/storage/change-version-handle-reuse-worker.html fast/workers/storage/read-and-write-transactions-dont-run-together.html fast/workers/storage/empty-statement-sync.html fast/workers/storage/multiple-transactions-on-different-handles.html http/tests/security/cross-origin-worker-websql.html fast/workers/storage/transaction-in-transaction-sync.html fast/workers/storage/open-database-while-transaction-in-progress.html http/tests/security/cross-origin-worker-websql-allowed.html fast/workers/storage/multiple-transactions.html fast/workers/storage/change-version-sync.html fast/workers/storage/execute-sql-args-sync.html fast/workers/storage/execute-sql-args-worker.html fast/workers/storage/open-database-while-transaction-in-progress-sync.html fast/workers/storage/sql-data-types-sync.html fast/workers/storage/open-database-empty-version-sync.html fast/workers/storage/open-database-set-empty-version-sync.html fast/workers/storage/multiple-transactions-on-different-handles-sync.html fast/workers/storage/test-authorizer-sync.html fast/workers/storage/test-authorizer.html Created attachment 215520 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 215509 [details] updated the patch, submitting only for EWS analysis - style fixed Attachment 215509 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/17128243 New failing tests: fast/workers/storage/change-version-handle-reuse-worker.html fast/workers/storage/read-and-write-transactions-dont-run-together.html fast/workers/storage/empty-statement-sync.html fast/workers/storage/multiple-transactions-on-different-handles.html http/tests/security/cross-origin-worker-websql.html fast/workers/storage/transaction-in-transaction-sync.html fast/workers/storage/open-database-while-transaction-in-progress.html http/tests/security/cross-origin-worker-websql-allowed.html fast/workers/storage/multiple-transactions.html fast/workers/storage/change-version-sync.html fast/workers/storage/execute-sql-args-sync.html fast/workers/storage/execute-sql-args-worker.html fast/workers/storage/open-database-while-transaction-in-progress-sync.html fast/workers/storage/sql-data-types-sync.html fast/workers/storage/open-database-empty-version-sync.html fast/workers/storage/open-database-set-empty-version-sync.html fast/workers/storage/multiple-transactions-on-different-handles-sync.html fast/workers/storage/test-authorizer-sync.html fast/workers/storage/test-authorizer.html Created attachment 215529 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 215509 [details] updated the patch, submitting only for EWS analysis - style fixed Attachment 215509 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/17108216 New failing tests: fast/workers/storage/executesql-accepts-only-one-statement-sync.html fast/workers/storage/transaction-in-transaction-sync.html http/tests/security/cross-origin-worker-websql-allowed.html fast/workers/storage/multiple-transactions.html fast/workers/storage/execute-sql-args-sync.html fast/workers/storage/open-database-creation-callback-sync.html fast/workers/storage/open-database-set-empty-version-sync.html fast/workers/storage/multiple-transactions-on-different-handles-sync.html fast/workers/storage/test-authorizer-sync.html fast/workers/storage/open-database-while-transaction-in-progress-sync.html fast/workers/storage/change-version-handle-reuse-worker.html fast/workers/storage/multiple-transactions-on-different-handles.html fast/workers/storage/execute-sql-args-worker.html fast/workers/storage/test-authorizer.html fast/workers/storage/change-version-sync.html fast/workers/storage/change-version-handle-reuse-sync.html fast/workers/storage/open-database-inputs-sync.html fast/workers/storage/open-database-empty-version-sync.html fast/workers/storage/sql-data-types-sync.html fast/workers/storage/interrupt-database.html fast/workers/storage/read-and-write-transactions-dont-run-together.html fast/workers/storage/empty-statement-sync.html http/tests/security/cross-origin-worker-websql.html fast/workers/storage/sql-exception-codes-sync.html fast/workers/storage/open-database-while-transaction-in-progress.html fast/workers/storage/multiple-databases-garbage-collection.html Created attachment 215535 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 217281 [details]
updated patch
updated patch
Created attachment 217287 [details]
patch
Let's try again the win-ews after fixing eol-style in svn.
Created attachment 217291 [details]
patch
One more try with unix-style line ends.
Created attachment 217293 [details]
patch
As eol-style is now relally fixed in svn, trying win-ews again.
Comment on attachment 217293 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=217293&action=review > Source/WebCore/ChangeLog:9 > + No new tests, covered by existing tests. > + Remove this line, this is not necessary. Please also remove the giant list of file, that is useless. > Source/WebCore/WebCore.exp.in:-2884 > -#if ENABLE(WORKERS) > __ZN7WebCore12WorkerThread17workerThreadCountEv > -#endif You should put this export in the main list now. > Source/WebCore/bindings/generic/ActiveDOMCallback.cpp:-39 > -#if ENABLE(WORKERS) > #include "WorkerGlobalScope.h" > -#endif Please move the include in the #include list. > Source/WebCore/bindings/js/ScriptState.cpp:46 > #include "JSWorkerGlobalScope.h" This include should go in the list now. > Source/WebCore/inspector/CodeGeneratorInspector.py:204 > + ??? > Source/WebCore/inspector/InspectorWorkerAgent.cpp:158 > void InspectorWorkerAgent::canInspectWorkers(ErrorString*, bool* result) > { > -#if ENABLE(WORKERS) > *result = true; > -#else > - *result = false; > -#endif > } Remove this function instead and clean the call sites. > Source/WebCore/loader/WorkerThreadableLoader.h:-35 > - > -#if ENABLE(WORKERS) > - Add a blank line between #define WorkerThreadableLoader_h and the first #include. > Source/WebCore/page/WorkerNavigator.h:-30 > - > -#if ENABLE(WORKERS) > - Blank line here. > Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in:-443 > -#if ENABLE(WORKERS) > ?workerThreadCount@WorkerThread@WebCore@@SAIXZ > -#endif This probably needs to be moved in the main section, sorted alphabetically. Created attachment 217551 [details]
patch
(In reply to comment #22) > (From update of attachment 217293 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217293&action=review > > > Source/WebCore/inspector/InspectorWorkerAgent.cpp:158 > > void InspectorWorkerAgent::canInspectWorkers(ErrorString*, bool* result) > > { > > -#if ENABLE(WORKERS) > > *result = true; > > -#else > > - *result = false; > > -#endif > > } > > Remove this function instead and clean the call sites. > It seems this is used by the Inspector UI, and can't be removed without modifying it. As I'm not familiar with the Inspector, CCing Joe and Timothy. What do you think about this? Is it reasonable removing canInspectWorkers? Also, if it should be removed, I think it should be done as a separate bug/patch. (In reply to comment #24) > (In reply to comment #22) > > (From update of attachment 217293 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=217293&action=review > > > > > Source/WebCore/inspector/InspectorWorkerAgent.cpp:158 > > > void InspectorWorkerAgent::canInspectWorkers(ErrorString*, bool* result) > > > { > > > -#if ENABLE(WORKERS) > > > *result = true; > > > -#else > > > - *result = false; > > > -#endif > > > } > > > > Remove this function instead and clean the call sites. > > > > It seems this is used by the Inspector UI, and can't be removed without modifying it. As I'm not familiar with the Inspector, CCing Joe and Timothy. What do you think about this? Is it reasonable removing canInspectWorkers? No, please do not remove canInspectWorkers at this time. This is part of the protocol (WebCore/inspector/protocol/Worker.json) and we would want to leave it in right now at least for protocol backwards compatibility. We can evaluate later if we should remove it ourselves, but at least for now please leave it in. Thanks! Comment on attachment 217551 [details] patch Clearing flags on attachment: 217551 Committed r159679: <http://trac.webkit.org/changeset/159679> Thanks for landing this ! |