[webkit-reviews] review granted: [Bug 116346] WKKeyValueStorageManagerGetKeyValueStorageOrigins should get origins from the UI process : [Attachment 202144] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 116346: WKKeyValueStorageManagerGetKeyValueStorageOrigins should get
origins from the UI process
https://bugs.webkit.org/show_bug.cgi?id=116346

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

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=202144&action=review


r=me with some itches..

> Source/WebKit2/UIProcess/Storage/LocalStorageDatabaseTracker.h:56
> +    Vector<RefPtr<WebCore::SecurityOrigin>> origins();

This method should be const.

> Source/WebKit2/UIProcess/Storage/StorageManager.cpp:340
> +    // Make sure the encoding is initialized before we start dispatching
things to the queue.
> +    UTF8Encoding();

This looks somewhat arbitrary. Are we sure this is the only encoding that will
ever be needed?

> Source/WebKit2/UIProcess/Storage/StorageManager.cpp:571
> +static void callCallbackFunction(void* context, void
(*callbackFunction)(const Vector<RefPtr<WebCore::SecurityOrigin>>&
securityOrigins, void* context), Vector<RefPtr<WebCore::SecurityOrigin>>*
securityOriginsPtr)
> +{
> +    OwnPtr<Vector<RefPtr<WebCore::SecurityOrigin>>> securityOrigins =
adoptPtr(securityOriginsPtr);

I wish we could pass securityOriginsPtr as a PassOwnPtr instead here.

> Source/WebKit2/UIProcess/WebKeyValueStorageManagerProxy.cpp:93
> +    for (size_t i = 0; i < securityOrigins.size(); ++i)

I prefer to use 'unsigned' to iterate over Vectors in new code, as we're
planning to move away from size_t there eventually.