[webkit-reviews] review denied: [Bug 58443] [GTK] Support the file system API : [Attachment 138384] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied ChangSeok Oh
<kevin.cs.oh at gmail.com>'s request for review:
Bug 58443: [GTK] Support the file system API
https://bugs.webkit.org/show_bug.cgi?id=58443

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

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


This patch seems a bit too big to review at once. Why not try splitting it up?
Since the API is divided into operations it makes sense to implement one per
patch and unskip tests that pass with each iteration.

One thing that I wonder about looking at this initially is the threading
behavior. Instead of actually using the asychronous GIO APIs, the operations
are just posted back to the main thread. That seems wrong -- unless this is
supposed to be called from workers and all operations need to be on the main
thread. Is that the case? If so perhaps it would be better to have a
"filesystem thread" so that these operations don't block the main thread.

> Source/WTF/wtf/gobject/GOwnPtr.cpp:54
> +template <> void freeOwnedGPtr<GOutputStream>(GOutputStream* ptr)
> +{
> +    g_output_stream_close(ptr, 0, 0);
> +}
> +

GOutputStream is a reference counted GObject, so you should not use GOwnPtr to
manage it. g_output_stream_close doesn't free a GOuputStream, only the internal
bits.