WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables. These are often used cross-thread and copying the captured lambda variables in dangerous.
👍👍👍
Created attachment 279867 [details] WIP patch WIP patch based on very initial brainstorming with Brady and Anders. Let me know what you think.
Created attachment 279868 [details] WIP patch
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.
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.
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.
Created attachment 279897 [details] WIP patch
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.
Created attachment 279899 [details] WIP patch Will play with EWS to get it building on all platforms.
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.
Created attachment 279902 [details] WIP patch
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.
Created attachment 279904 [details] WIP patch
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.
Created attachment 279905 [details] Patch
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.
Created attachment 279906 [details] Patch
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.
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.
Created attachment 279907 [details] Patch
Trying to make MSVC happy as it seems confused by the template parameter used inside the lambda function.
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.
Comment on attachment 279907 [details] Patch MSVC is finally happy.
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.
(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).
Created attachment 279913 [details] Patch
The new version improves the WTF ChangeLog.
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.
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.
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
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
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?
(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.
(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?
(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.
Created attachment 279935 [details] Patch
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.
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.
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.
Comment on attachment 279935 [details] Patch Ok, I'll work on what Anders is suggesting.
Created attachment 279973 [details] Patch
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.
Created attachment 279974 [details] Patch
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.
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.
Comment on attachment 279974 [details] Patch Ok, this latest patch should do what Anders suggested.
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?
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!
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.
Created attachment 279979 [details] Patch
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.
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.
Created attachment 279981 [details] Patch
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.
Created attachment 279983 [details] Patch
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.
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.
Comment on attachment 279983 [details] Patch Clearing flags on attachment: 279983 Committed r201464: <http://trac.webkit.org/changeset/201464>
All reviewed patches have been landed. Closing bug.
Committed r201479: <http://trac.webkit.org/changeset/201479>