[Webkit-unassigned] [Bug 43714] Move filehandling into fileLoadTimer callback

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 16 07:50:18 PDT 2010


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





--- Comment #3 from Patrick R. Gansterer <paroga at paroga.com>  2010-08-16 07:50:18 PST ---
(In reply to comment #2)
> (From update of attachment 63879 [details])
> > -void ResourceHandle::fileLoadTimer(Timer<ResourceHandle>* timer)
> > +void ResourceHandle::fileLoadTimer(Timer<ResourceHandle>*)
> >  {
> > +    String path = firstRequest().url().path();
> > +    // Windows does not enjoy a leading slash on paths.
> > +    if (path[0] == '/')
> > +        path = path.substring(1);
> > +
> > +    int queryPos = path.find('?');
> > +    if (queryPos != -1)
> > +        path.remove(queryPos);
> 
> I'm surprised that KURL::path would return a string that contains a query component. That seems like a bad bug. Is that really happening?
I had this problem at least once, so i added this.

> It seems like it would be better to use KURL::fileSystemPath anyway.
Didn't see that function. ;-)

> 
> > +    if (fileHandle == INVALID_HANDLE_VALUE) {
> > +        client()->didFail(this, ResourceError());
> > +        deref();
> > +        return;
> > +    }
> 
> Where is the ref() that this deref() is balancing? I don't see it in this patch. Was it a bug that we weren't dereffing before? We usually include a comment at both the ref() and deref() callsites mentioning where the balancing call is.
The ref() is directly at the start(). At the moment the ResourceHandleWin does a "delete this"!!! I'll add comments.

> 
> > +    int dotPos = path.reverseFind('.');
> > +    int slashPos = path.reverseFind('/');
> > +
> > +    if (slashPos < dotPos && dotPos != -1) {
> > +        String ext = path.substring(dotPos + 1);
> > +        response.setMimeType(MIMETypeRegistry::getMIMETypeForExtension(ext));
> > +    }
> 
> This code seems entirely new and doesn't match the ChangeLog (which claims this patch just moves code).
Ok, I'll update the ChangeLog.

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



More information about the webkit-unassigned mailing list