129795 – ScriptExecutionContext::Task should work well with C++11 lambdas

RESOLVED FIXED 129795

ScriptExecutionContext::Task should work well with C++11 lambdas

https://bugs.webkit.org/show_bug.cgi?id=129795

Summary ScriptExecutionContext::Task should work well with C++11 lambdas

Zan Dobersek

Reported 2014-03-06 03:39:52 PST

ScriptExecutionContext::Task should work well with C++11 lambdas

Attachments
Patch (113.90 KB, patch) 2014-03-06 03:57 PST, Zan Dobersek	no flags	Details Formatted Diff Diff
WIP (113.52 KB, patch) 2014-03-07 01:27 PST, Zan Dobersek	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (796.85 KB, application/zip) 2014-03-07 04:22 PST, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (815.84 KB, application/zip) 2014-03-08 15:05 PST, Build Bot	no flags	Details
WIP (110.67 KB, patch) 2014-03-17 14:12 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
WIP (110.78 KB, patch) 2014-03-18 12:55 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
WIP patch (110.82 KB, patch) 2014-03-20 14:42 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
Patch (117.20 KB, patch) 2014-04-15 13:51 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
Patch for landing (117.10 KB, patch) 2014-04-26 06:08 PDT, Zan Dobersek	zan: commit-queue-	Details Formatted Diff Diff
Patch for landing (117.07 KB, patch) 2014-04-26 06:12 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
Patch for landing (117.91 KB, patch) 2014-04-28 23:50 PDT, Zan Dobersek	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Zan Dobersek

Comment 1 2014-03-06 03:57:51 PST

Created attachment 225977 [details] Patch

WebKit Commit Bot

Comment 2 2014-03-06 08:31:00 PST

Attachment 225977 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:139: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:156: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 24 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 3 2014-03-07 01:27:24 PST

Created attachment 226095 [details] WIP

WebKit Commit Bot

Comment 4 2014-03-07 01:29:00 PST

Attachment 226095 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:139: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:156: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 25 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 5 2014-03-07 04:22:29 PST

Comment on attachment 226095 [details] WIP Attachment 226095 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4548094985240576 New failing tests: http/tests/workers/shared-worker-importScripts.html http/tests/websocket/tests/hybi/workers/close-code-and-reason.html http/tests/websocket/tests/hybi/workers/close.html http/tests/websocket/tests/hybi/workers/no-subprotocol.html http/tests/websocket/tests/hybi/workers/multiple-subprotocols.html http/tests/websocket/tests/hybi/workers/send-arraybufferview.html http/tests/websocket/tests/hybi/workers/shared-worker-simple.html http/tests/websocket/tests/hybi/workers/worker-handshake-challenge-randomness.html http/tests/websocket/tests/hybi/workers/receive-blob.html

Build Bot

Comment 6 2014-03-07 04:22:30 PST

Created attachment 226114 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Build Bot

Comment 7 2014-03-08 15:05:04 PST

Comment on attachment 226095 [details] WIP Attachment 226095 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5705517536116736 New failing tests: http/tests/websocket/tests/hybi/workers/close-code-and-reason.html http/tests/websocket/tests/hybi/workers/close-in-onmessage-crash.html http/tests/websocket/tests/hybi/workers/close.html http/tests/websocket/tests/hybi/workers/send-arraybuffer.html http/tests/websocket/tests/hybi/workers/no-subprotocol.html http/tests/websocket/tests/hybi/workers/shared-worker-simple.html http/tests/websocket/tests/hybi/workers/close-in-worker.html http/tests/websocket/tests/hybi/workers/worker-simple.html http/tests/websocket/tests/hybi/workers/send-arraybufferview.html http/tests/websocket/tests/hybi/workers/receive-arraybuffer.html http/tests/websocket/tests/hybi/workers/send-blob.html http/tests/websocket/tests/hybi/workers/no-onmessage-in-sync-op.html http/tests/websocket/tests/hybi/workers/multiple-subprotocols.html http/tests/websocket/tests/hybi/workers/worker-handshake-challenge-randomness.html http/tests/websocket/tests/hybi/workers/receive-blob.html

Build Bot

Comment 8 2014-03-08 15:05:08 PST

Created attachment 226238 [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

Zan Dobersek

Comment 9 2014-03-17 14:12:42 PDT

Created attachment 226963 [details] WIP Running through EWS.

WebKit Commit Bot

Comment 10 2014-03-18 00:00:18 PDT

Attachment 226963 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:132: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:139: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:156: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 26 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 11 2014-03-18 12:55:26 PDT

Created attachment 227092 [details] WIP

WebKit Commit Bot

Comment 12 2014-03-18 12:57:39 PDT

Attachment 227092 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:131: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:138: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 26 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 13 2014-03-20 14:42:38 PDT

Created attachment 227336 [details] WIP patch Retesting through EWSs.

WebKit Commit Bot

Comment 14 2014-03-20 14:44:37 PDT

Attachment 227336 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:131: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:138: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 26 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 15 2014-04-15 13:51:24 PDT

Created attachment 229400 [details] Patch

WebKit Commit Bot

Comment 16 2014-04-15 13:53:37 PDT

Attachment 229400 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:131: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:138: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 25 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Darin Adler

Comment 17 2014-04-16 10:22:58 PDT

Comment on attachment 229400 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229400&action=review This change is a very good idea. I think we can do even more simplification, but this is a huge step in the right direction. As a future step, I think we can remove the CrossThreadCopier and CallbackTask and createCallback machinery entirely. No reason callers can’t just copy arguments explicitly rather than using this massive machinery to generate the calls to copy. I tried it and the code comes out much more readable. > Source/WebCore/dom/ScriptExecutionContext.h:131 > + template<typename T, typename = typename std::enable_if<!std::is_base_of<Task, T>::value && std::is_convertible<T, std::function<void (ScriptExecutionContext*)>>::value>::type> I don’t think these functions should take a ScriptExecutionContext*. It would be cleaner to have them take no argument at all. Or ScriptExecutionContext&. The few tasks that need a to get at the document or script execution context could just pass it in. Passing it to all tasks makes them all uglier and more complex for no reason. > Source/WebCore/dom/ScriptExecutionContext.h:149 > + Task(Task&& task) > + : m_task(std::move(task.m_task)) > + , m_isCleanupTask(task.m_isCleanupTask) > + { > + } Won’t the compiler generate this automatically? I don’t think we need to write this code. > Source/WebCore/dom/ScriptExecutionContext.h:159 > + virtual void postTask(Task) = 0; // Executes the task on context's thread asynchronously. I think this should take a Task&& rather than a Task. Doesn’t have to be done on this first check-in, but I think it would be more efficient and also clearer in concept. > Source/WebCore/workers/WorkerMessagingProxy.cpp:191 > + Vector<std::unique_ptr<ScriptExecutionContext::Task>> queuedEarlyTasks = std::move(m_queuedEarlyTasks); Seems like we should use auto here instead of writing out that entire type. > Source/WebCore/workers/WorkerRunLoop.h:63 > + void postTask(ScriptExecutionContext::Task); > + void postTaskAndTerminate(ScriptExecutionContext::Task); > + void postTaskForMode(ScriptExecutionContext::Task, const String& mode); Again, I think Task&& is better than Task for these argument types.

Zan Dobersek

Comment 18 2014-04-26 06:08:57 PDT

Created attachment 230241 [details] Patch for landing

WebKit Commit Bot

Comment 19 2014-04-26 06:10:47 PDT

Attachment 230241 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:123: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:130: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:143: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 25 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 20 2014-04-26 06:12:44 PDT

Created attachment 230242 [details] Patch for landing

WebKit Commit Bot

Comment 21 2014-04-26 06:15:31 PDT

Attachment 230242 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:122: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:129: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:142: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 25 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 22 2014-04-26 06:17:52 PDT

Comment on attachment 230242 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=230242&action=review Made some of the changes proposed in comment #17, landing it later. > Source/WebCore/dom/ScriptExecutionContext.h:136 > + Task(Task&&) = default; The Task class now simply has a defaulted move constructor, which is necessary and the simplest way to make the class non-copyable. > Source/WebCore/workers/WorkerMessagingProxy.cpp:191 > + auto queuedEarlyTasks = std::move(m_queuedEarlyTasks); Now uses auto.

Zan Dobersek

Comment 23 2014-04-26 06:25:10 PDT

Comment on attachment 229400 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229400&action=review >> Source/WebCore/dom/ScriptExecutionContext.h:159 >> + virtual void postTask(Task) = 0; // Executes the task on context's thread asynchronously. > > I think this should take a Task&& rather than a Task. Doesn’t have to be done on this first check-in, but I think it would be more efficient and also clearer in concept. There's no explicit need for it -- the Task class is non-copyable, so already either an rvalue has to be passed in or std::move() has to be used. I'll give this additional attention later.

Zan Dobersek

Comment 24 2014-04-27 07:07:31 PDT

Comment on attachment 230242 [details] Patch for landing Clearing flags on attachment: 230242 Committed r167855: <http://trac.webkit.org/changeset/167855>

Zan Dobersek

Comment 25 2014-04-27 07:07:43 PDT

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 26 2014-04-28 13:51:28 PDT

Re-opened since this is blocked by bug 132301

Brent Fulgham

Comment 27 2014-04-28 13:53:06 PDT

Example of the failure messages: 18> OESTextureFloat.cpp 18>c:\projects\webkit\opensource\source\webcore\dom\ScriptExecutionContext.h(136): error C2610: 'WebCore::ScriptExecutionContext::Task::Task(WebCore::ScriptExecutionContext::Task &&)' : is not a special member function which can be defaulted (C:\Projects\WebKit\OpenSource\WebKitBuild\Debug\obj32\WebCore\DerivedSources\SVGElementFactory.cpp) 18> OESTextureFloatLinear.cpp 18> OESTextureHalfFloat.cpp

Zan Dobersek

Comment 28 2014-04-28 23:50:47 PDT

Created attachment 230357 [details] Patch for landing Sorry for the breakage. MSVC doesn't seem to support defaulted move constructors, so this patch just writes that constructor out again. I'll wait for the EWSs to process the patch before landing.

WebKit Commit Bot

Comment 29 2014-04-29 00:05:06 PDT

Attachment 230357 [details] did not pass style-queue: ERROR: Source/WebCore/workers/WorkerRunLoop.h:78: The parameter name "task" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:122: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:129: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/dom/ScriptExecutionContext.h:146: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerThread.cpp:220: Extra space before ) [whitespace/parens] [2] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:96: Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] ERROR: Source/WebCore/Modules/webdatabase/Database.cpp:175: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/workers/WorkerEventQueue.cpp:50: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:335: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:349: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:364: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:380: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:397: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:414: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:433: Extra space before [ [whitespace/braces] [5] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/dom/CrossThreadTask.h:452: Extra space before [ [whitespace/braces] [5] Total errors found: 25 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zan Dobersek

Comment 30 2014-04-29 09:23:33 PDT

Windows EWS is now also building, so I'll land this again in a moment.

Zan Dobersek

Comment 31 2014-04-29 10:42:00 PDT

Comment on attachment 230357 [details] Patch for landing Clearing flags on attachment: 230357 Committed r167943: <http://trac.webkit.org/changeset/167943>

Zan Dobersek

Comment 32 2014-04-29 10:42:18 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component New Bugs

Assignee

Zan Dobersek

Reported

2014-03-06 03:39 PST

Modified

2014-04-29 10:42 PDT History

CC List

4 users Show

URL

Keywords

Depends on

132301

Blocks

Dependencies

tree graph