Bug 105784 – Remove ENABLE_WORKERS

• Component: New Bugs
• Status: RESOLVED FIXED

[Laszlo Gombos]

Remove ENABLE_WORKERS as all major ports have it on by default

[Laszlo Gombos]

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 ?

[Patrick R. Gansterer]

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

[Laszlo Gombos]

> WinCE port compiles with enabled WORKERS too. 

Thanks Patrick. Filed bug 108099 to actually enable it before removing the flag.

[Laszlo Gombos]

Created attachment 191125 [details]
1st take

[Early Warning System Bot]

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

[Early Warning System Bot]

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

[Laszlo Gombos]

Comment on attachment 191125 [details]
1st take

Qt builds need to force a clean build, incremental builds does not seems to work.

[Eric Seidel (no email)]

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.

[Peter Molnar]

Created attachment 215506 [details]
updated the patch, submitting only for EWS analysis.

[WebKit Commit Bot]

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.

[Peter Molnar]

Created attachment 215509 [details]
updated the patch, submitting only for EWS analysis - style fixed

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Peter Molnar]

Created attachment 217281 [details]
updated patch

updated patch

[Peter Molnar]

Created attachment 217287 [details]
patch

Let's try again the win-ews after fixing eol-style in svn.

[Peter Molnar]

Created attachment 217291 [details]
patch

One more try with unix-style line ends.

[Peter Molnar]

Created attachment 217293 [details]
patch

As eol-style is now relally fixed in svn, trying win-ews again.

[Benjamin Poulain]

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.

[Peter Molnar]

Created attachment 217551 [details]
patch

[Peter Molnar]

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

[Peter Molnar]

Also, if it should be removed, I think it should be done as a separate bug/patch.

[Joseph Pecoraro]

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

[WebKit Commit Bot]

Comment on attachment 217551 [details]
patch

Clearing flags on attachment: 217551

Committed r159679: <http://trac.webkit.org/changeset/159679>

[Laszlo Gombos]

Thanks for landing this !