[Webkit-unassigned] [Bug 168025] Added missing methods implementation of WebCore::FileStream

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=168025

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #301066|review+                     |review-
              Flags|                            |

--- Comment #4 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 301066
  --> https://bugs.webkit.org/attachment.cgi?id=301066
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301066&action=review

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?

> Source/WebCore/ChangeLog:3
> +        Added missing methods implementations

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"

> 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);

Use nullptr, not 0.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170211/44112994/attachment-0001.html>