Bug 105784 - Remove ENABLE_WORKERS
Summary: Remove ENABLE_WORKERS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 108099
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-26 23:40 PST by Laszlo Gombos
Modified: 2013-12-11 02:18 PST (History)
12 users (show)

See Also:


Attachments
1st take (160.77 KB, patch)
2013-03-02 21:59 PST, Laszlo Gombos
eric: review+
Details | Formatted Diff | Diff
updated the patch, submitting only for EWS analysis. (120.70 KB, patch)
2013-10-30 08:29 PDT, Peter Molnar
no flags Details | Formatted Diff | Diff
updated the patch, submitting only for EWS analysis - style fixed (120.70 KB, patch)
2013-10-30 08:38 PDT, Peter Molnar
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (579.05 KB, application/zip)
2013-10-30 10:07 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (625.26 KB, application/zip)
2013-10-30 11:08 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (781.32 KB, application/zip)
2013-10-30 11:20 PDT, Build Bot
no flags Details
updated patch (96.47 KB, patch)
2013-11-19 02:38 PST, Peter Molnar
no flags Details | Formatted Diff | Diff
patch (deleted)
2013-11-19 04:31 PST, Peter Molnar
no flags Details | Formatted Diff | Diff
patch (96.50 KB, patch)
2013-11-19 05:32 PST, Peter Molnar
no flags Details | Formatted Diff | Diff
patch (96.50 KB, patch)
2013-11-19 06:02 PST, Peter Molnar
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
patch (91.50 KB, patch)
2013-11-21 05:58 PST, Peter Molnar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2012-12-26 23:40:22 PST
Remove ENABLE_WORKERS as all major ports have it on by default
Comment 1 Laszlo Gombos 2013-01-26 11:10:26 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 ?
Comment 2 Patrick R. Gansterer 2013-01-28 11:41:09 PST
(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. ;-)
Comment 3 Laszlo Gombos 2013-01-28 12:07:08 PST
> WinCE port compiles with enabled WORKERS too. 

Thanks Patrick. Filed bug 108099 to actually enable it before removing the flag.
Comment 4 Laszlo Gombos 2013-03-02 21:59:36 PST
Created attachment 191125 [details]
1st take
Comment 5 Early Warning System Bot 2013-03-02 23:26:05 PST
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 6 Early Warning System Bot 2013-03-02 23:30:05 PST
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 7 Laszlo Gombos 2013-03-03 06:06:51 PST
Comment on attachment 191125 [details]
1st take

Qt builds need to force a clean build, incremental builds does not seems to work.
Comment 8 Eric Seidel (no email) 2013-03-03 10:06:53 PST
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.
Comment 9 Peter Molnar 2013-10-30 08:29:15 PDT
Created attachment 215506 [details]
updated the patch, submitting only for EWS analysis.
Comment 10 WebKit Commit Bot 2013-10-30 08:31:52 PDT
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.
Comment 11 Peter Molnar 2013-10-30 08:38:22 PDT
Created attachment 215509 [details]
updated the patch, submitting only for EWS analysis - style fixed
Comment 12 Build Bot 2013-10-30 10:06:58 PDT
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
Comment 13 Build Bot 2013-10-30 10:07:01 PDT
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 14 Build Bot 2013-10-30 11:08:35 PDT
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
Comment 15 Build Bot 2013-10-30 11:08:43 PDT
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 16 Build Bot 2013-10-30 11:20:48 PDT
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
Comment 17 Build Bot 2013-10-30 11:20:51 PDT
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
Comment 18 Peter Molnar 2013-11-19 02:38:25 PST
Created attachment 217281 [details]
updated patch

updated patch
Comment 19 Peter Molnar 2013-11-19 04:31:09 PST
Created attachment 217287 [details]
patch

Let's try again the win-ews after fixing eol-style in svn.
Comment 20 Peter Molnar 2013-11-19 05:32:20 PST
Created attachment 217291 [details]
patch

One more try with unix-style line ends.
Comment 21 Peter Molnar 2013-11-19 06:02:24 PST
Created attachment 217293 [details]
patch

As eol-style is now relally fixed in svn, trying win-ews again.
Comment 22 Benjamin Poulain 2013-11-20 22:31:24 PST
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.
Comment 23 Peter Molnar 2013-11-21 05:58:12 PST
Created attachment 217551 [details]
patch
Comment 24 Peter Molnar 2013-11-21 06:12:57 PST
(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?
Comment 25 Peter Molnar 2013-11-21 07:26:23 PST
Also, if it should be removed, I think it should be done as a separate bug/patch.
Comment 26 Joseph Pecoraro 2013-11-21 11:13:43 PST
(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 27 WebKit Commit Bot 2013-11-21 20:59:21 PST
Comment on attachment 217551 [details]
patch

Clearing flags on attachment: 217551

Committed r159679: <http://trac.webkit.org/changeset/159679>
Comment 28 Laszlo Gombos 2013-11-21 23:44:16 PST
Thanks for landing this !