[webkit-reviews] review granted: [Bug 134042] Don't kill the UIProcess until all local storage transactions have been committed : [Attachment 233354] patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Roger Fong
<roger_fong at apple.com>'s request for review:
Bug 134042: Don't kill the UIProcess until all local storage transactions have
been committed
https://bugs.webkit.org/show_bug.cgi?id=134042

Attachment 233354: patch
https://bugs.webkit.org/attachment.cgi?id=233354&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=233354&action=review


> Source/WebKit2/UIProcess/Storage/StorageManager.cpp:597
> +	   HashMap<std::pair<RefPtr<IPC::Connection>, uint64_t>,
RefPtr<StorageArea>> storageAreasByConnection = m_storageAreasByConnection;

No need to copy the HashMap here.

> Source/WebKit2/UIProcess/Storage/StorageManager.cpp:598
> +	   for (HashMap<std::pair<RefPtr<IPC::Connection>, uint64_t>,
RefPtr<StorageArea>>::const_iterator it = storageAreasByConnection.begin(), end
= storageAreasByConnection.end(); it != end; ++it) {

This should use a modern for loop.

> Source/WebKit2/UIProcess/Storage/StorageManager.cpp:604
> +	   for (size_t i = 0; i <
connectionAndStorageMapIDPairsToRemove.size(); ++i)
> +	      
m_storageAreasByConnection.remove(connectionAndStorageMapIDPairsToRemove[i]);

This should use a modern for loop, and I think a comment stating that removing
the storage areas causes the changes to be flushed.