[Webkit-unassigned] [Bug 52916] Expose "suggested filename" for a resource based on its resource response.

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Brady Eidson <beidson at apple.com>  2011-01-21 13:57:36 PST ---
(In reply to comment #3)
> (From update of attachment 79784 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79784&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.cpp:206
> > +WKStringRef WKBundleFrameCopySuggestedFilenameForResourceURL(WKBundleFrameRef frameRef, WKURLRef urlRef)
> 
> Maybe "...ForSubresourceWithURL" would be more accurate?

I'll go with "ForResourceWithURL", because this would work on main resources, as well.
> 
> Are the "Ref" suffixes really needed on the parameters?

Talk to Sam and Anders.  :)

> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.cpp:208
> > +    return toCopiedAPI(toImpl(frameRef)->suggestedFilenameForResourceURL(WebCore::KURL(WebCore::KURL(), toImpl(urlRef)->string())));
> 
> So we don't get to assume that all strings from WKURLRefs can be used with KURL's ParsedURLStringTag constructor?

Correct, we don't.  Sad, but true.

> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.h:62
> > +WK_EXPORT WKStringRef WKBundleFrameCopySuggestedFilenameForResourceURL(WKBundleFrameRef page, WKURLRef url);
> 
> "page" seems wrong.

lol.  yup.

> > Source/WebKit2/WebProcess/WebPage/WebFrame.cpp:538
> > +    if (DocumentLoader* loader = m_coreFrame->loader()->documentLoader()) {
> > +        if (RefPtr<ArchiveResource> subresource = loader->subresource(url))
> > +            return subresource->response().suggestedFilename();
> > +    }
> 
> I think early returns would be nicer.

Sure.

> 
> > Source/WebKit2/WebProcess/WebPage/WebFrame.h:112
> > +    String suggestedFilenameForResourceURL(const WebCore::KURL& url) const;
> 
> No need for "url" here.

Style bot got me already.

Thanks!

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