WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
123804
Manage ScriptExecutionContext::Task derivatives through std::unique_ptr
https://bugs.webkit.org/show_bug.cgi?id=123804
Summary
Manage ScriptExecutionContext::Task derivatives through std::unique_ptr
Zan Dobersek
Reported
2013-11-05 08:42:11 PST
Manage ScriptExecutionContext::Task derivatives through std::unique_ptr
Attachments
Patch
(88.76 KB, patch)
2013-11-05 09:34 PST
,
Zan Dobersek
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Another try
(77.92 KB, patch)
2013-11-07 23:38 PST
,
Zan Dobersek
gtk-ews
: commit-queue-
Details
Formatted Diff
Diff
Third attempt
(78.41 KB, patch)
2013-11-08 11:30 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(81.34 KB, patch)
2013-11-12 13:56 PST
,
Zan Dobersek
kling
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Zan Dobersek
Comment 1
2013-11-05 09:34:12 PST
Created
attachment 216048
[details]
Patch
WebKit Commit Bot
Comment 2
2013-11-05 09:36:29 PST
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.
Zan Dobersek
Comment 3
2013-11-05 09:36:45 PST
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.
Build Bot
Comment 4
2013-11-05 10:26:07 PST
Comment on
attachment 216048
[details]
Patch
Attachment 216048
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/21168027
Build Bot
Comment 5
2013-11-05 10:31:46 PST
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
Build Bot
Comment 6
2013-11-05 11:12:43 PST
Comment on
attachment 216048
[details]
Patch
Attachment 216048
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/21168031
Build Bot
Comment 7
2013-11-05 12:45:46 PST
Comment on
attachment 216048
[details]
Patch
Attachment 216048
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/20068107
Zan Dobersek
Comment 8
2013-11-05 23:54:37 PST
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.
Zan Dobersek
Comment 9
2013-11-07 23:38:39 PST
Created
attachment 216362
[details]
Another try Testing through EWS.
WebKit Commit Bot
Comment 10
2013-11-07 23:43:33 PST
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.
kov's GTK+ EWS bot
Comment 11
2013-11-07 23:54:25 PST
Comment on
attachment 216362
[details]
Another try
Attachment 216362
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/22368424
Build Bot
Comment 12
2013-11-08 00:33:04 PST
Comment on
attachment 216362
[details]
Another try
Attachment 216362
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/22538395
Zan Dobersek
Comment 13
2013-11-08 11:30:38 PST
Created
attachment 216418
[details]
Third attempt Fixes the GTK build, dunno why/how the Win build is failing.
WebKit Commit Bot
Comment 14
2013-11-08 11:31:41 PST
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.
Build Bot
Comment 15
2013-11-08 12:31:37 PST
Comment on
attachment 216418
[details]
Third attempt
Attachment 216418
[details]
did not pass win-ews (win): Output:
http://webkit-queues.appspot.com/results/22778508
Zan Dobersek
Comment 16
2013-11-08 14:24:24 PST
(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.
Zan Dobersek
Comment 17
2013-11-12 13:56:12 PST
Created
attachment 216720
[details]
Patch
Andreas Kling
Comment 18
2014-02-05 17:35:16 PST
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
Zan Dobersek
Comment 19
2014-08-04 09:24:24 PDT
This isn't necessary anymore now that ScriptExecutionContext::Task objects are just lambdas (
bug #129795
).
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug