Manage ScriptExecutionContext::Task derivatives through std::unique_ptr
Created attachment 216048 [details] Patch
Attachment 216048 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/StdLibExtras.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/quota/StorageErrorCallback.h', u'Source/WebCore/Modules/quota/StorageInfo.cpp', u'Source/WebCore/Modules/webdatabase/Database.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseManager.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseSync.cpp', u'Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h', u'Source/WebCore/Modules/webdatabase/SQLStatement.cpp', u'Source/WebCore/Modules/webdatabase/SQLTransaction.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.h', u'Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp', u'Source/WebCore/bindings/js/JSCallbackData.h', u'Source/WebCore/bindings/js/JSDOMGlobalObjectTask.h', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestCallback.cpp', u'Source/WebCore/dom/CrossThreadTask.h', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/Document.h', u'Source/WebCore/dom/ScriptExecutionContext.cpp', u'Source/WebCore/dom/ScriptExecutionContext.h', u'Source/WebCore/dom/StringCallback.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp', u'Source/WebCore/workers/DefaultSharedWorkerRepository.cpp', u'Source/WebCore/workers/WorkerEventQueue.cpp', u'Source/WebCore/workers/WorkerGlobalScope.cpp', u'Source/WebCore/workers/WorkerGlobalScope.h', u'Source/WebCore/workers/WorkerLoaderProxy.h', u'Source/WebCore/workers/WorkerMessagingProxy.cpp', u'Source/WebCore/workers/WorkerMessagingProxy.h', u'Source/WebCore/workers/WorkerRunLoop.cpp', u'Source/WebCore/workers/WorkerRunLoop.h', u'Source/WebCore/workers/WorkerThread.cpp']" exit_code: 1 Source/WebCore/Modules/webdatabase/SQLStatement.cpp:105: std::unique_ptr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WebCore/Modules/webdatabase/SQLStatement.cpp:106: std::unique_ptr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WebCore/Modules/webdatabase/SQLTransaction.cpp:305: std::unique_ptr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WebCore/Modules/webdatabase/SQLTransaction.cpp:306: std::unique_ptr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WebCore/Modules/webdatabase/SQLTransaction.cpp:307: std::unique_ptr is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 5 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
WTF_MAKE_UNIQUE and WTF_MAKE_UNIQUE_ARGS macros work well under Clang 3.2 and up and g++ 4.7 and up. They don't function under g++ 4.6, which some ports still want to support. No idea about MSVC.
Comment on attachment 216048 [details] Patch Attachment 216048 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/21168027
Comment on attachment 216048 [details] Patch Attachment 216048 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/21228023
Comment on attachment 216048 [details] Patch Attachment 216048 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/21168031
Comment on attachment 216048 [details] Patch Attachment 216048 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/20068107
Comment on attachment 216048 [details] Patch The Mac build failed due to a couple undefined references which extra explicit template instantiations would fix. MSVC doesn't play ball with the std::make_unique friend declarations though, so that won't be possible to use. I'll update the patch with public constructors that std::make_unique won't have problems accessing.
Created attachment 216362 [details] Another try Testing through EWS.
Attachment 216362 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/wtf/Compiler.h', u'Source/WTF/wtf/StdLibExtras.h', u'Source/WebCore/Modules/quota/StorageErrorCallback.h', u'Source/WebCore/Modules/quota/StorageInfo.cpp', u'Source/WebCore/Modules/webdatabase/Database.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseManager.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseSync.cpp', u'Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.h', u'Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp', u'Source/WebCore/bindings/js/JSCallbackData.h', u'Source/WebCore/bindings/js/JSDOMGlobalObjectTask.h', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestCallback.cpp', u'Source/WebCore/dom/CrossThreadTask.h', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/Document.h', u'Source/WebCore/dom/ScriptExecutionContext.cpp', u'Source/WebCore/dom/ScriptExecutionContext.h', u'Source/WebCore/dom/StringCallback.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp', u'Source/WebCore/workers/DefaultSharedWorkerRepository.cpp', u'Source/WebCore/workers/WorkerEventQueue.cpp', u'Source/WebCore/workers/WorkerGlobalScope.cpp', u'Source/WebCore/workers/WorkerGlobalScope.h', u'Source/WebCore/workers/WorkerLoaderProxy.h', u'Source/WebCore/workers/WorkerMessagingProxy.cpp', u'Source/WebCore/workers/WorkerMessagingProxy.h', u'Source/WebCore/workers/WorkerRunLoop.cpp', u'Source/WebCore/workers/WorkerRunLoop.h', u'Source/WebCore/workers/WorkerThread.cpp']" exit_code: 1 Source/WTF/wtf/StdLibExtras.h:454: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:459: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:464: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:469: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:474: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:479: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:484: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:489: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:494: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:499: Missing spaces around && [whitespace/operators] [3] Total errors found: 10 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 216362 [details] Another try Attachment 216362 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/22368424
Comment on attachment 216362 [details] Another try Attachment 216362 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/22538395
Created attachment 216418 [details] Third attempt Fixes the GTK build, dunno why/how the Win build is failing.
Attachment 216418 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/wtf/Compiler.h', u'Source/WTF/wtf/NeverDestroyed.h', u'Source/WTF/wtf/StdLibExtras.h', u'Source/WebCore/Modules/quota/StorageErrorCallback.h', u'Source/WebCore/Modules/quota/StorageInfo.cpp', u'Source/WebCore/Modules/webdatabase/Database.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseManager.cpp', u'Source/WebCore/Modules/webdatabase/DatabaseSync.cpp', u'Source/WebCore/Modules/webdatabase/SQLCallbackWrapper.h', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp', u'Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.h', u'Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp', u'Source/WebCore/bindings/js/JSCallbackData.h', u'Source/WebCore/bindings/js/JSDOMGlobalObjectTask.h', u'Source/WebCore/bindings/js/JSDOMWindowBase.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestCallback.cpp', u'Source/WebCore/dom/CrossThreadTask.h', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/dom/Document.h', u'Source/WebCore/dom/ScriptExecutionContext.cpp', u'Source/WebCore/dom/ScriptExecutionContext.h', u'Source/WebCore/dom/StringCallback.cpp', u'Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp', u'Source/WebCore/workers/DefaultSharedWorkerRepository.cpp', u'Source/WebCore/workers/WorkerEventQueue.cpp', u'Source/WebCore/workers/WorkerGlobalScope.cpp', u'Source/WebCore/workers/WorkerGlobalScope.h', u'Source/WebCore/workers/WorkerLoaderProxy.h', u'Source/WebCore/workers/WorkerMessagingProxy.cpp', u'Source/WebCore/workers/WorkerMessagingProxy.h', u'Source/WebCore/workers/WorkerRunLoop.cpp', u'Source/WebCore/workers/WorkerRunLoop.h', u'Source/WebCore/workers/WorkerThread.cpp']" exit_code: 1 Source/WTF/wtf/StdLibExtras.h:454: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:459: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:464: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:469: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:474: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:479: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:484: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:489: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:494: Missing spaces around && [whitespace/operators] [3] Source/WTF/wtf/StdLibExtras.h:499: Missing spaces around && [whitespace/operators] [3] Total errors found: 10 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 216418 [details] Third attempt Attachment 216418 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/22778508
(In reply to comment #15) > (From update of attachment 216418 [details]) > Attachment 216418 [details] did not pass win-ews (win): > Output: http://webkit-queues.appspot.com/results/22778508 AFAIU MSVC is crashing. Fun. This was pretty much the last attempt at trying to add support for std::make_unique friendships that could keep the constructors private. MSVC already required a dumbed-down approach to start with (even MSVC 2012 doesn't process the decltype() approach), so I'm giving up on this for the moment and I'll be uploading a version of this that is making the relevant constructors public.
Created attachment 216720 [details] Patch
Comment on attachment 216720 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=216720&action=review This looks fine, so I'm gonna say r=me. You'll probably need to fix up some things, like s/OVERRIDE/override/ for it to build. It would also be good to go over these classes and spam final/override as appropriate. > Source/WebCore/Modules/webdatabase/Database.cpp:89 > + DerefContextTask(PassRefPtr<ScriptExecutionContext> context) explicit > Source/WebCore/Modules/webdatabase/Database.cpp:203 > + DeliverPendingCallbackTask(PassRefPtr<SQLTransaction> transaction) explicit > Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:237 > + Vector<std::unique_ptr<ScriptExecutionContext::Task>> tasks = std::move(m_pendingTasks); > + for (auto iter = tasks.begin(); iter != tasks.end(); ++iter) > (*iter)->performTask(0); I'd go all in and write this like so: auto tasks = std::move(m_pendingTasks); for (auto& task : tasks) task.performTask(0); > Source/WebCore/bindings/js/JSCallbackData.h:80 > + DeleteCallbackDataTask(JSCallbackData* data) : m_data(data) { } explicit > Source/WebCore/workers/WorkerMessagingProxy.cpp:139 > WorkerGlobalScopeDestroyedTask(WorkerMessagingProxy* messagingProxy) explicit > Source/WebCore/workers/WorkerMessagingProxy.cpp:212 > NotifyNetworkStateChangeTask(bool isOnLine) explicit
This isn't necessary anymore now that ScriptExecutionContext::Task objects are just lambdas (bug #129795).