[Webkit-unassigned] [Bug 145800] NetworkCache: Delete old cache versions

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #254568|review?                     |review+
              Flags|                            |

--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 254568
  --> https://bugs.webkit.org/attachment.cgi?id=254568
patch

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

Looks like this does not build on GTK yet.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp:69
> +        function(String(name), directoryEntryType(dp->d_type));

Doing String(name) here interprets the filename as Latin-1. That’s not right. This needs to be the reverse of WebCore::fileSystemRepresentation. I see some code using String::fromUTF8 for this; openTemporaryFile on the Mac for example. This only works because we are lucky enough to not have any non-ASCII filenames in the directory. Of course, decoding UTF-8 has the inverse problem if there are some file names that we can’t represent as String. I suppose we have to just skip those files. This can’t happen in normal circumstances on OS X, but might happen in other cases on other platforms perhaps.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.h:57
> +        function(String(name));

Same String(name) issue here.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:123
> +        auto partitionPath = WebCore::pathByAppendingComponent(recordsPath, subdirName);
> +        WebCore::deleteEmptyDirectory(partitionPath);

Not sure the local variable makes this easier to read

> Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp:878
> +            String versionString = subdirName.substring(strlen(versionDirectoryPrefix));
> +            bool success;
> +            unsigned directoryVersion = versionString.toUIntStrict(&success);

Should do this with a StringView. No need to copy out a new string just to convert to integer.

-- 
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/20150609/011de586/attachment.html>