ScriptExecutionContext::Task should work well with C++11 lambdas
Created attachment 225977 [details] Patch
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.
Created attachment 226095 [details] WIP
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.
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
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
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
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
Created attachment 226963 [details] WIP Running through EWS.
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.
Created attachment 227092 [details] WIP
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.
Created attachment 227336 [details] WIP patch Retesting through EWSs.
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.
Created attachment 229400 [details] Patch
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.
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.
Created attachment 230241 [details] Patch for landing
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.
Created attachment 230242 [details] Patch for landing
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.
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.
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.
Comment on attachment 230242 [details] Patch for landing Clearing flags on attachment: 230242 Committed r167855: <http://trac.webkit.org/changeset/167855>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 132301
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
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.
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.
Windows EWS is now also building, so I'll land this again in a moment.
Comment on attachment 230357 [details] Patch for landing Clearing flags on attachment: 230357 Committed r167943: <http://trac.webkit.org/changeset/167943>