Bug 158111 – WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Chris Dumez]

WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables. These are often used cross-thread and copying the captured lambda variables in dangerous.

[Brady Eidson]

👍👍👍

[Chris Dumez]

Created attachment 279867 [details]
WIP patch

WIP patch based on very initial brainstorming with Brady and Anders. Let me know what you think.

[Chris Dumez]

Created attachment 279868 [details]
WIP patch

[WebKit Commit Bot]

Attachment 279868 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 16 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279895 [details]
WIP patch

Version without a virtual base. We could easily extend this to support other lambdas than void() by templatizing the class.
Right now it is non-copyable, but we could imagine a version that is copyable using ThreadSafeRefCounted internally.

[WebKit Commit Bot]

Attachment 279895 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 16 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279897 [details]
WIP patch

[WebKit Commit Bot]

Attachment 279897 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/WorkQueue.h:77:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 16 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279899 [details]
WIP patch

Will play with EWS to get it building on all platforms.

[WebKit Commit Bot]

Attachment 279899 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279902 [details]
WIP patch

[WebKit Commit Bot]

Attachment 279902 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279904 [details]
WIP patch

[WebKit Commit Bot]

Attachment 279904 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/win/WorkItemWin.h:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 16 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279905 [details]
Patch

[WebKit Commit Bot]

Attachment 279905 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279906 [details]
Patch

[WebKit Commit Bot]

Attachment 279906 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Oh MSVC does not like this :/

C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/FunctionDispatcher.h(71): error C2065: 'F': undeclared identifier (compiling source file C:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\inspector\agents\InspectorHeapAgent.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\memory(1630): note: see reference to function template instantiation 'WTF::NonCopyableFunction::LambdaWrapper::LambdaWrapper<_Ty,void>(F &&)' being compiled
          with
          [
              _Ty=Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6>,
              F=Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6>
          ] (compiling source file C:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\inspector\agents\InspectorHeapAgent.cpp)
  C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\ForwardingHeaders\wtf/FunctionDispatcher.h(44): note: see reference to function template instantiation 'std::unique_ptr<WTF::NonCopyableFunction::LambdaWrapper,std::default_delete<_Ty>> std::make_unique<WTF::NonCopyableFunction::LambdaWrapper,Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6>>(Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6> &&)' being compiled
          with
          [
              _Ty=WTF::NonCopyableFunction::LambdaWrapper
          ] (compiling source file C:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\inspector\agents\InspectorHeapAgent.cpp)
  C:\cygwin\home\buildbot\WebKit\Source\JavaScriptCore\inspector\agents\InspectorHeapAgent.cpp(297): note: see reference to function template instantiation 'WTF::NonCopyableFunction::NonCopyableFunction<Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6>,void>(F &&)' being compiled
          with
          [
              F=Inspector::InspectorHeapAgent::didGarbageCollect::<lambda_8879b6e4ab22bfba2f51f56750bddbe6>
          ]

I don't get it.

[Chris Dumez]

Created attachment 279907 [details]
Patch

[Chris Dumez]

Trying to make MSVC happy as it seems confused by the template parameter used inside the lambda function.

[WebKit Commit Bot]

Attachment 279907 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 279907 [details]
Patch

MSVC is finally happy.

[Alexey Proskuryakov]

Comment on attachment 279907 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279907&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
> +        These are often used cross-thread and copying the captured lambda variables can be
> +        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
> +        capture).

I think that the ChangeLog would benefit from a description of what the correct approach is, in addition to this.

[Chris Dumez]

(In reply to comment #24)
> Comment on attachment 279907 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279907&action=review
> 
> > Source/JavaScriptCore/ChangeLog:11
> > +        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
> > +        These are often used cross-thread and copying the captured lambda variables can be
> > +        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
> > +        capture).
> 
> I think that the ChangeLog would benefit from a description of what the
> correct approach is, in addition to this.

Sure, I'll improve the ChangeLog. Basically, the proposal is to use a new WTF::NonCopyableFunction type instead of an std::function for this case to guarantee that the passed-in lambda (and its captured variables) does not get copied.

NonCopyableFunction would also allow us to capture move-only types (such as std::unique_ptr) since NonCopyableFunction does not require the lambda to be copyable (unlike std::function).

[Chris Dumez]

Created attachment 279913 [details]
Patch

[Chris Dumez]

The new version improves the WTF ChangeLog.

[WebKit Commit Bot]

Attachment 279913 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 279913 [details]
Patch

Attachment 279913 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1387868

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 279916 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Chris Dumez]

The EWS failure is unrelated I believe. It has been happening for the last week on several bugs. Failures look like:
Errors look like:
--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_delete-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/imported/w3c/web-platform-tests/IndexedDB/idbobjectstore_delete-actual.txt
@@ -1,3 +1,5 @@
-
-PASS IDBObjectStore.delete() - delete removes record (inline keys) 
-
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584

[Darin Adler]

Comment on attachment 279913 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279913&action=review

I’m sort of shocked that we have to make this NoncopyableFunction class. Nothing in standard C++ for this?

> Source/WTF/ChangeLog:13
> +        This patch introduces a new NonCopyableFunction type that behaves similarly to

The name should be NoncopyableFunction, since "noncopyable" is a single word, even though it’s not a real English word.

> Source/WTF/wtf/FunctionDispatcher.h:44
> +        : m_lambdaWrapper(std::make_unique<LambdaWrapper>(WTFMove(lambda)))

So much heap allocation. Sigh.

> Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:36
> +    auto* functionPtr = new NonCopyableFunction(WTFMove(function));

No way to do this without heap allocation? Isn’t there some technique that might work using a lambda instead of a block below?

[Chris Dumez]

(In reply to comment #32)
> Comment on attachment 279913 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279913&action=review
> 
> I’m sort of shocked that we have to make this NoncopyableFunction class.
> Nothing in standard C++ for this?

I couldn't find any. I only found this paper that talks about this issue:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4159.pdf

> 
> > Source/WTF/ChangeLog:13
> > +        This patch introduces a new NonCopyableFunction type that behaves similarly to
> 
> The name should be NoncopyableFunction, since "noncopyable" is a single
> word, even though it’s not a real English word.

Ok, will fix.

> 
> > Source/WTF/wtf/FunctionDispatcher.h:44
> > +        : m_lambdaWrapper(std::make_unique<LambdaWrapper>(WTFMove(lambda)))
> 
> So much heap allocation. Sigh.
> 
> > Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:36
> > +    auto* functionPtr = new NonCopyableFunction(WTFMove(function));
> 
> No way to do this without heap allocation? Isn’t there some technique that
> might work using a lambda instead of a block below?

I asked Anders if there was a better ways since I am not familiar with ObjC. He could not think of a better one but I'll investigate further.

[Chris Dumez]

(In reply to comment #32)
> Comment on attachment 279913 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279913&action=review
> 
> I’m sort of shocked that we have to make this NoncopyableFunction class.
> Nothing in standard C++ for this?
> 
> > Source/WTF/ChangeLog:13
> > +        This patch introduces a new NonCopyableFunction type that behaves similarly to
> 
> The name should be NoncopyableFunction, since "noncopyable" is a single
> word, even though it’s not a real English word.
> 
> > Source/WTF/wtf/FunctionDispatcher.h:44
> > +        : m_lambdaWrapper(std::make_unique<LambdaWrapper>(WTFMove(lambda)))
> 
> So much heap allocation. Sigh.

I used to have all the LambdaWrapper members directly on NoncopyableFunction in an earlier version of my patch (https://bugs.webkit.org/attachment.cgi?id=279895&action=review) but Anders suggested I used a std::unique_ptr internally to decrease the size of NoncopyableFunction (which had 3 pointer members).

> 
> > Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:36
> > +    auto* functionPtr = new NonCopyableFunction(WTFMove(function));
> 
> No way to do this without heap allocation? Isn’t there some technique that
> might work using a lambda instead of a block below?

[Chris Dumez]

(In reply to comment #32)
> Comment on attachment 279913 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279913&action=review
> 
> I’m sort of shocked that we have to make this NoncopyableFunction class.
> Nothing in standard C++ for this?
> 
> > Source/WTF/ChangeLog:13
> > +        This patch introduces a new NonCopyableFunction type that behaves similarly to
> 
> The name should be NoncopyableFunction, since "noncopyable" is a single
> word, even though it’s not a real English word.
> 
> > Source/WTF/wtf/FunctionDispatcher.h:44
> > +        : m_lambdaWrapper(std::make_unique<LambdaWrapper>(WTFMove(lambda)))
> 
> So much heap allocation. Sigh.
> 
> > Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:36
> > +    auto* functionPtr = new NonCopyableFunction(WTFMove(function));
> 
> No way to do this without heap allocation? Isn’t there some technique that
> might work using a lambda instead of a block below?

FYI, I have tried using a lambda instead of a block as so:
dispatch_async(m_dispatchQueue, [this, function = WTFMove(function)] {
        function();
        deref();
    });

However, this did not build:
/Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:36:37: error: call to implicitly-deleted copy constructor of 'const (lambda at /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:36:37)'
/Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:36:44: note: copy constructor of '' is implicitly deleted because field '' has a deleted copy constructor
In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:27:
In file included from /Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/WorkQueue.h:33:
/Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/FunctionDispatcher.h:36:26: note: 'NoncopyableFunction' has been explicitly marked deleted here
/Volumes/Data/WebKit/OpenSource/Source/WTF/wtf/cocoa/WorkQueueCocoa.mm:36:37: note: implicit capture of lambda object due to conversion to block pointer here
1 error generated.

It looks like it tries to copy my NoncopyableFunction object when converting the lambda into a block.

[Chris Dumez]

Created attachment 279935 [details]
Patch

[WebKit Commit Bot]

Attachment 279935 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

I renamed to NoncopyableFunction and stopped using a unique_ptr internally so we no longer need to heap allocate. Do you mind taking another look since the NoncopyableFunction class looks a bit different now?

Note that I could not find a way to avoid the heap allocations in WorkQueueCocoa.cpp.

[Anders Carlsson]

I don't think this is better than using a unique_ptr - we're still doing heap allocation of the lambda, so we might as well use a unique_ptr instead of untyped lambdas for deletion + invocation.

[Chris Dumez]

Comment on attachment 279935 [details]
Patch

Ok, I'll work on what Anders is suggesting.

[Chris Dumez]

Created attachment 279973 [details]
Patch

[WebKit Commit Bot]

Attachment 279973 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279974 [details]
Patch

[WebKit Commit Bot]

Attachment 279974 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 279973 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279973&action=review

> Source/JavaScriptCore/runtime/Watchdog.cpp:176
> +            this->m_timerDidFire = true;

I don’t think we need "this->" here.

> Source/JavaScriptCore/runtime/Watchdog.cpp:178
> +        this->deref();

I don’t think we need "this->" here.

> Source/WTF/wtf/FunctionDispatcher.h:34
> +class LambdaWrapperBase {

Should these be called LambdaWrapper or FunctionWrapper?

> Source/WTF/wtf/FunctionDispatcher.h:42
> +template<typename F, class = typename std::enable_if<std::is_rvalue_reference<F&&>::value>::type>

Is it better to use the "F" for this argument rather than a name like FunctionType?

> Source/WTF/wtf/FunctionDispatcher.h:44
> +    WTF_MAKE_NONCOPYABLE(LambdaWrapper);

I’m so sick of this macro. We should probably start writing out the "= delete" stuff instead in the future. We made this macro when it was a lot harder to delete the copy constructor and assignment operator.

> Source/WTF/wtf/FunctionDispatcher.h:59
> +    WTF_MAKE_NONCOPYABLE(NoncopyableFunction);

I don’t think this is needed. I think a class with a unique_ptr member ends up being non-copyable automatically.

> Source/WTF/wtf/FunctionDispatcher.h:63
> +    template<typename F, class = typename std::enable_if<std::is_rvalue_reference<F&&>::value>::type>

I think I would prefer FunctionType.

> Source/WTF/wtf/FunctionDispatcher.h:64
> +    NoncopyableFunction(F&& lambda)

Should this be marked explicit or not?

> Source/WTF/wtf/FunctionDispatcher.h:70
> +    NoncopyableFunction(NoncopyableFunction&&) = default;
> +    NoncopyableFunction& operator=(NoncopyableFunction&&) = default;

If we didn’t do the unnecessary WTF_MAKE_NONCOPYABLE, then I think this would just happen automatically without having to state it.

[Chris Dumez]

Comment on attachment 279974 [details]
Patch

Ok, this latest patch should do what Anders suggested.

[Chris Dumez]

Comment on attachment 279973 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279973&action=review

>> Source/WTF/wtf/FunctionDispatcher.h:34
>> +class LambdaWrapperBase {
> 
> Should these be called LambdaWrapper or FunctionWrapper?

Technically, the idea is to usually wrap a lambda, not an std::function. However, it will happily wrap anything that is callable (so a lambda or an std::function).
I could go with FunctionWrapper but I worry it may be confused as std::function wrapper. Maybe CallableWrapper?

>> Source/WTF/wtf/FunctionDispatcher.h:42
>> +template<typename F, class = typename std::enable_if<std::is_rvalue_reference<F&&>::value>::type>
> 
> Is it better to use the "F" for this argument rather than a name like FunctionType?

Maybe LambdaType or CallableType?

[Darin Adler]

Comment on attachment 279974 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279974&action=review

Most of my comments from the last patch still apply. Sorry, caught you at just the wrong time, I think. Please read those comments.

> Source/WTF/wtf/FunctionDispatcher.h:107
> +using WTF::NoncopyableFunction;
>  using WTF::FunctionDispatcher;

Normally we keep these in alphabetical order.

> Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:29
> +#include <wtf/RefPtr.h>

What creates the need for this include? Doesn’t make sense given the code I’m reading below.

> Source/WTF/wtf/efl/DispatchQueueWorkItemEfl.h:52
> +    static std::unique_ptr<TimerWorkItem> create(PassRefPtr<WorkQueue> workQueue, NoncopyableFunction&& function, std::chrono::nanoseconds delayNanoSeconds)

Amazing discipline, seeing this PassRefPtr and being able to leave it alone!

[Chris Dumez]

Comment on attachment 279973 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279973&action=review

>> Source/WTF/wtf/FunctionDispatcher.h:64
>> +    NoncopyableFunction(F&& lambda)
> 
> Should this be marked explicit or not?

The idea is to behave like a std::function (just not copyable), si I'd say no. The corresponding constructor is not explicit on std::function.

dispatch(NoncopyableFunction([]() {})) would look ugly.

[Chris Dumez]

Created attachment 279979 [details]
Patch

[WebKit Commit Bot]

Attachment 279979 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Thus patch should take all Darin's feedback into consideration. However, I went with the naming:
* F -> CallableType
* LambdaWrapper -> CallableWrapper
* lambda -> callable

Let me know if you don't like this naming.

[Chris Dumez]

Created attachment 279981 [details]
Patch

[WebKit Commit Bot]

Attachment 279981 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 279983 [details]
Patch

[Chris Dumez]

Comment on attachment 279983 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279983&action=review

> Source/WTF/wtf/FunctionDispatcher.h:71
> +    class CallableWrapper final : public CallableWrapperBase {

I moved this class inside NoncopyableFunction as per Anders feedback offline.

[WebKit Commit Bot]

Attachment 279983 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:63:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:64:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:69:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:70:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.h:72:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/NetworkProcess.cpp:424:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:574:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:586:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:605:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:626:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:655:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Storage/StorageManager.cpp:674:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 15 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 279983 [details]
Patch

Clearing flags on attachment: 279983

Committed r201464: <http://trac.webkit.org/changeset/201464>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

Committed r201479: <http://trac.webkit.org/changeset/201479>