<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - Added missing methods implementation of WebCore::FileStream"
href="https://bugs.webkit.org/show_bug.cgi?id=168025">bug 168025</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #301066 Flags</td>
<td>review+
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Added missing methods implementation of WebCore::FileStream"
href="https://bugs.webkit.org/show_bug.cgi?id=168025#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Added missing methods implementation of WebCore::FileStream"
href="https://bugs.webkit.org/show_bug.cgi?id=168025">bug 168025</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=301066&action=diff" name="attach_301066" title="Patch">attachment 301066</a> <a href="attachment.cgi?id=301066&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=301066&action=review">https://bugs.webkit.org/attachment.cgi?id=301066&action=review</a>
Wow. These functions have been unimplemented for years! At first I assumed that this could be hiding all sorts of interesting bugs, and I was planning to ask you if this fixed anything in particular. But I see that they are unimplemented on all ports. And I see that nothing uses these functions currently. Only NetworkDataTaskBlob and BlobResourceHandle use FileStream or AsyncFileStream, and the functions that are unimplemented in FileStream are only ever called in AsyncFileStream functions that are never themselves called.
Now, FileStream::write is still unimplemented on all ports, and you don't implement it here. So I think we need to consider what we want to do with this class. Implementing FileStream::openForWrite does not make much sense when all writes are guaranteed to fail, right? Also, keeping that function around does not make sense if it's not implemented. It does seem nice to have a more featureful FileStream API, but I'm not sure if the value of having a richer API really outweighs the cost of keeping around unused code. At any rate, we definitely need to add WebCore API tests for this code, if implementing these functions did not fix any existing tests.
Darin, I'm curious what you think we should do here. I'm inclined to suggest just removing the affected functions, including FileStream::write and AsyncFileStream::write, so long as nothing is using them, but since they're easy to implement I guess we could keep them around unused if tests are added?
<span class="quote">> Source/WebCore/ChangeLog:3
> + Added missing methods implementations</span >
This commit message isn't descriptive enough, and it should exactly match the title of the bug anyway. So write:
"Added missing methods implementation of WebCore::FileStream"
<span class="quote">> Source/WebCore/platform/glib/FileSystemGlib.cpp:348
> + return g_seekable_truncate(G_SEEKABLE(g_io_stream_get_input_stream(G_IO_STREAM(handle))), offset, 0, 0);</span >
Use nullptr, not 0.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>