Move WebCore storage code to std::bind, std::function
Created attachment 220316 [details] Patch
Attachment 220316 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/storage/StorageAreaSync.cpp', u'Source/WebCore/storage/StorageSyncManager.cpp', u'Source/WebCore/storage/StorageSyncManager.h', u'Source/WebCore/storage/StorageThread.cpp', u'Source/WebCore/storage/StorageThread.h', u'Source/WebCore/storage/StorageTracker.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:48: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:61: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:305: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:44: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 8 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220316 [details] Patch Attachment 220316 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5418391959502848
Comment on attachment 220316 [details] Patch Attachment 220316 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6403554377990144
Comment on attachment 220316 [details] Patch I don't think this is correct. WTF::bind has some tricks to automatically ref/deref objects that are captured and I think that's lost in this conversion.
Created attachment 220324 [details] Patch
Attachment 220324 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/storage/StorageAreaSync.cpp', u'Source/WebCore/storage/StorageSyncManager.cpp', u'Source/WebCore/storage/StorageSyncManager.h', u'Source/WebCore/storage/StorageThread.cpp', u'Source/WebCore/storage/StorageThread.h', u'Source/WebCore/storage/StorageTracker.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:49: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:305: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:45: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 8 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220324 [details] Patch Removing the r? flag, still has the same issue as the previous patch.
Created attachment 220433 [details] Patch
Attachment 220433 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/storage/StorageAreaSync.cpp', u'Source/WebCore/storage/StorageSyncManager.cpp', u'Source/WebCore/storage/StorageSyncManager.h', u'Source/WebCore/storage/StorageThread.cpp', u'Source/WebCore/storage/StorageThread.h', u'Source/WebCore/storage/StorageTracker.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:49: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:305: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:45: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageAreaSync.cpp:115: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 9 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220433&action=review > Source/WebCore/storage/StorageAreaSync.cpp:78 > - m_syncManager->dispatch(bind(&StorageAreaSync::performImport, this)); > + m_syncManager->dispatch(std::bind(&StorageAreaSync::performImport, > + std::bind([](const RefPtr<StorageAreaSync>& storageArea) { return storageArea.get(); }, RefPtr<StorageAreaSync>(this)))); To handle referencing and dereferencing of the StorageAreaSync object on which the member function should be called, a nested bind expression is used that holds onto a RefPtr<StorageAreaSync> object and returns a raw pointer to the StorageAreaSync object when called. If this approach makes sense it can be abstracted into a helper template.
I'd rather we moved away from std::bind to lambdas instead.
Lambdas would work and look better, but capturing the this pointer wouldn't handle the (de)referencing, so a RefPtr<T> variable would have to be initalized and captured by value. C++14 adds support for inlining lambda capture initializations. That's available in Clang 3.4 and GCC 4.9.
Created attachment 220505 [details] Patch
Attachment 220505 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/storage/StorageAreaSync.cpp', u'Source/WebCore/storage/StorageSyncManager.cpp', u'Source/WebCore/storage/StorageSyncManager.h', u'Source/WebCore/storage/StorageThread.cpp', u'Source/WebCore/storage/StorageThread.h', u'Source/WebCore/storage/StorageTracker.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:49: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:175: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:238: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:306: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:315: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:315: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:388: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:485: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:45: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageAreaSync.cpp:115: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 15 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220505 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220505&action=review > Source/WebCore/storage/StorageTracker.cpp:288 > - callOnMainThread(bind(&StorageTracker::deleteOriginWithIdentifier, this, originIdentifier.isolatedCopy())); > + callOnMainThread(std::bind([this](const String& originIdentifier) { deleteOriginWithIdentifier(originIdentifier); }, > + originIdentifier.isolatedCopy())); std::bind() remains in use here and in similar places to avoid unnecessarily copying the isolated copies through lambda capture: String originIdentifierCopy = originIdentifier.isolatedCopy(); callOnMainThread([this, originIdentifierCopy] { deleteOriginWithIdentifier(originIdentifier); }); Again, C++14 lambda improvements will allow just this: callOnMainThread([this, originIdentifierCopy = originIdentifier.copy()] { deleteOriginWithIdentifier(originIdentifier); });
Created attachment 220636 [details] Patch Get rid of the multiple-commands-on-same-line style errors. The errors caused by the space between the return type and the leading parantheses of the wrapped function signature should be resolved in the style checker.
Attachment 220636 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/storage/StorageAreaSync.cpp', u'Source/WebCore/storage/StorageSyncManager.cpp', u'Source/WebCore/storage/StorageSyncManager.h', u'Source/WebCore/storage/StorageThread.cpp', u'Source/WebCore/storage/StorageThread.h', u'Source/WebCore/storage/StorageTracker.cpp', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:49: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:311: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageTracker.cpp:320: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:45: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 9 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 220636 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220636&action=review > Source/WebCore/storage/StorageTracker.cpp:293 > + callOnMainThread(std::bind([this](const String& originIdentifier) { > + deleteOriginWithIdentifier(originIdentifier); > + }, originIdentifier.isolatedCopy())); In cases such as this we now mostly use a pure lambda that captures an isolated copy of the necessary string instead of relying on std::bind(). I'll update this patch to use that approach.
Created attachment 237031 [details] Patch
Attachment 237031 [details] did not pass style-queue: ERROR: Source/WebCore/storage/StorageThread.cpp:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:83: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.cpp:95: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:49: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageThread.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.h:45: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/storage/StorageSyncManager.cpp:74: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 7 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 237031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237031&action=review > Source/WebCore/storage/StorageAreaSync.cpp:75 > + m_syncManager->dispatch([=] { Is there a good reason that in sites like this we have [=] instead of [protector]?
Comment on attachment 237031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237031&action=review >> Source/WebCore/storage/StorageAreaSync.cpp:75 >> + m_syncManager->dispatch([=] { > > Is there a good reason that in sites like this we have [=] instead of [protector]? Not really, explicitly listing the captured variables is generally a better idea since it prevents accidentally capturing unwanted objects. I'll fix that before landing.
Committed r172901: <http://trac.webkit.org/changeset/172901>