Using the default capture-by-value should be avoided as it can produce various negative side effects, e.g. capturing the this pointer when a member variable is used inside the lambda, and thus failing to make the lambda object self-contained. It also requires the author to explicitly list the variables she intends to use inside the lambda, providing one additional checkpoint at ensuring that the correct variables are used.
Created attachment 239563 [details] Patch
Attachment 239563 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:227: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:333: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:375: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/bindings/js/JSDOMGlobalObjectTask.cpp:86: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h:78: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 239563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239563&action=review Outlined a few problems that could have been avoided, most likely of my doing. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:422 > - m_loaderProxy.postTaskToLoader([=](ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, urlCopy, protocolCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->connect(url, protocol); > + peer->connect(urlCopy, protocolCopy); Case in point. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:439 > - m_loaderProxy.postTaskToLoader([=](ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, messageCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->send(message); > + peer->send(messageCopy); Ditto. > Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp:558 > - m_loaderProxy.postTaskToLoader([=] (ScriptExecutionContext& context) { > + m_loaderProxy.postTaskToLoader([peer, reasonCopy] (ScriptExecutionContext& context) { > ASSERT(isMainThread()); > ASSERT_UNUSED(context, context.isDocument()); > ASSERT(peer); > > - peer->fail(reason); > + peer->fail(reasonCopy); Ditto. > Source/WebCore/dom/StringCallback.cpp:43 > RefPtr<StringCallback> protector(this); > - context->postTask([=] (ScriptExecutionContext&) { > - this->handleEvent(data); > + context->postTask([protector, data] (ScriptExecutionContext&) { > + protector->handleEvent(data); Ditto.
Comment on attachment 239563 [details] Patch I like this better. I remember being a little puzzled by the [=] syntax earlier, since I have always used the exhaustive list of what to capture.
Comment on attachment 239563 [details] Patch Clearing flags on attachment: 239563 Committed r174581: <http://trac.webkit.org/changeset/174581>
All reviewed patches have been landed. Closing bug.