Modernize lambda usage for all callers of RunLoop::dispatch() (take 2) In https://bugs.webkit.org/show_bug.cgi?id=158265 I'd searched for ".dispatch(" Now I'm searching for ">dispatch(" and making the same types of changes.
Created attachment 280277 [details] Patch
Obviously missed something here.
Comment on attachment 280277 [details] Patch Attachment 280277 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1420330 Number of test failures exceeded the failure limit.
Created attachment 280283 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 280277 [details] Patch Attachment 280277 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1420336 Number of test failures exceeded the failure limit.
Created attachment 280285 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
(In reply to comment #2) > Obviously missed something here. This comment was based on the fact that I saw the tests failing. Exploring locally in a bit.
Comment on attachment 280277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280277&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:619 > + WorkQueue::create("com.apple.WebKit.Cache.delete")->dispatch([path = dumpFilePath()] { Why doesn't this call isolatedCopy()? > Source/WebKit2/UIProcess/Storage/StorageManager.cpp:493 > + m_queue->dispatch([this, protectedThis = Ref<StorageManager>(*this), connection = Ref<IPC::Connection>(*allowedConnection), storageNamespaceID]() mutable { allowedConnection can be null :( > Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:261 > #if ENABLE(VIDEO) Not strictly related to your change but the #ifdefs in this file should be outside the if() scope, not inside. > Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:546 > #if ENABLE(VIDEO) Not strictly related to your change but the #ifdefs in this file should be outside the if() scope, not inside.
*** Bug 158010 has been marked as a duplicate of this bug. ***
Comment on attachment 280277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280277&action=review > Source/WebKit2/UIProcess/Storage/StorageManager.cpp:527 > RefPtr<IPC::Connection> protectedConnection(&connection); I think you meant to drop this line as well.
I Ref<>'ed something that was allowed to be nullptr
(In reply to comment #8) > Comment on attachment 280277 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280277&action=review > > > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:619 > > + WorkQueue::create("com.apple.WebKit.Cache.delete")->dispatch([path = dumpFilePath()] { > > Why doesn't this call isolatedCopy()? I double checked what dumpFilePath() does - It always creates a brand new String, therefore new StringImpl, therefore isolatedCopy not needing. > > > Source/WebKit2/UIProcess/Storage/StorageManager.cpp:493 > > + m_queue->dispatch([this, protectedThis = Ref<StorageManager>(*this), connection = Ref<IPC::Connection>(*allowedConnection), storageNamespaceID]() mutable { > > allowedConnection can be null :( Yup, discovered that. https://bugs.webkit.org/show_bug.cgi?id=158277#c11
Created attachment 280297 [details] Patch
Comment on attachment 280297 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280297&action=review R=me with a comment. > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:619 > + WorkQueue::create("com.apple.WebKit.Cache.delete")->dispatch([path = dumpFilePath()] { I still think it would be less fragile to call isolatedCopy() here. I don't believe this is more efficient either because isolatedCopy() when called on a temporary already takes care of avoiding the copy in cases that are safe. See String isolatedCopy() &&;
(In reply to comment #14) > Comment on attachment 280297 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=280297&action=review > > R=me with a comment. > > > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:619 > > + WorkQueue::create("com.apple.WebKit.Cache.delete")->dispatch([path = dumpFilePath()] { > > I still think it would be less fragile to call isolatedCopy() here. I don't > believe this is more efficient either because isolatedCopy() when called on > a temporary already takes care of avoiding the copy in cases that are safe. > See > String isolatedCopy() &&; Fair enough. I didn't know about that optimization!
http://trac.webkit.org/changeset/201587