[Webkit-unassigned] [Bug 53529] [fileapi] Add support for filesystem: URI handling

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Adam Barth <abarth at webkit.org>  2011-02-01 16:27:24 PST ---
(From update of attachment 80849)
View in context: https://bugs.webkit.org/attachment.cgi?id=80849&action=review

> Source/WebCore/fileapi/FileSystemURL.cpp:66
> +KURL FileSystemURL::getOrigin(const KURL& url)
> +{
> +    ASSERT(url.protocolIs(kFileSystemProtocol));
> +
> +    KURL originUrl(ParsedURLString, url.path());
> +    originUrl.setPath("/");
> +    return originUrl;
> +}

The types on this function don't look right.  An "origin" is a SecurityOrigin, not a KURL...  Also, we've been centralizing all the knowledge about how to compute SecurityOrigins from URLs in the SecurityOrigin class.

> Source/WebCore/fileapi/FileSystemURL.h:53
> +class FileSystemURL {
> +public:
> +    static String createURLString(SecurityOrigin*, AsyncFileSystem::Type, const String& fullPath);
> +    static KURL getOrigin(const KURL&);
> +    static const char* fileSystemProtocol() { return kFileSystemProtocol; }
> +
> +private:
> +    static const char kFileSystemProtocol[];
> +    FileSystemURL() { }
> +};

Generally, we don't create classes that have all static members.  What does a FileSystemURL represent?  How is that different from a KURL?

> Source/WebCore/page/SecurityOrigin.cpp:68
> +// FIXME: Should this be part of SchemeRegistry?

Yes.

> Source/WebCore/page/SecurityOrigin.cpp:69
> +static bool schemeRequiresRequestCapabilityForDisplay(const String& scheme)

We probably don't want to use the term "Capability" here because Capability has a special meaning in security, and this doesn't quite match that meaning.

> Source/WebCore/page/SecurityOrigin.cpp:152
>          return adoptRef(new SecurityOrigin(BlobURL::getOrigin(url), sandboxFlags));

I see that you're copying the pattern here from BlobURL, but that's not a good pattern.  We should fix Blob URL not to use this style either.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.