[webkit-reviews] review granted: [Bug 48507] [GTK] Implement RunLoop, WorkQueue, Connection classes for WebKit2 : [Attachment 75006] RunLoop WorkQueue Connection class implementation

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Amruth Raj
<amruthraj at motorola.com>'s request for review:
Bug 48507: [GTK] Implement RunLoop, WorkQueue, Connection classes for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=48507

Attachment 75006: RunLoop WorkQueue Connection class implementation
https://bugs.webkit.org/attachment.cgi?id=75006&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75006&action=review

Looks good to me, though there are a few style issues. I'll fix them and land
it myself.

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:41
> +static int readBytesFromSocket(int fd, uint8_t* ptr, size_t length)

This should be "fileDescriptor" instead of "fd".

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:69
> +static bool writeBytesToSocket(int fd, uint8_t* ptr, size_t length)

Same.

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:79
> +    // Keep writing to the socket till the complete message has been written


This is missing the period.

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:91
> +    // write operation failed if complete message is not written

This should have a capital letter and a period.

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:135
> +    // handle any previously unprocessed message

Ditto.

> WebKit2/Platform/CoreIPC/gtk/ConnectionGtk.cpp:144
> +    // prepare to read the next message

Ditto.

> WebKit2/Platform/gtk/WorkQueueGtk.cpp:131
> +void WorkQueue::registerEventSourceHandler(int fd, int condition,
PassOwnPtr<WorkItem> item)

Sorry that I missed this. It should be "fileDescriptor" not "fd".

> WebKit2/Platform/gtk/WorkQueueGtk.cpp:144
> +	      
reinterpret_cast<GSourceFunc>(&WorkQueue::EventSource::performWork), 
> +	       eventSource, 
> +	      
reinterpret_cast<GDestroyNotify>(&WorkQueue::EventSource::deleteEventSource));
> +

The indentation looks off here.

> WebKit2/Platform/gtk/WorkQueueGtk.cpp:165
> +void WorkQueue::unregisterEventSourceHandler(int fd)

This should be fileDesciptor as well.