Bug 29537

Summary: Web Inspector: Expose InspectorResource fields
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch timothy: review+

Description Pavel Feldman 2009-09-21 02:59:06 PDT
Currently InspectorResource has all of its members private. Expose them via the API for potential processing in C++ (as agreed with Timothy long ago).
Comment 1 Pavel Feldman 2009-09-21 03:04:41 PDT
Created attachment 39845 [details]
patch
Comment 2 Timothy Hatcher 2009-09-21 14:46:41 PDT
Comment on attachment 39845 [details]
patch


> -
> -    if (!buffer)
> -        return String();
> -
> -    TextEncoding encoding(textEncodingName);
> -    if (!encoding.isValid())
> -        encoding = WindowsLatin1Encoding();
> -    return encoding.decode(buffer->data(), buffer->size());
>  }

I think an explicit return 0; would be good here, I think it happens anyway because PassRefPtr is uesed, but I had to think about where there was no final return here.
Comment 3 Pavel Feldman 2009-09-21 14:56:05 PDT
> I think an explicit return 0; would be good here, I think it happens anyway
> because PassRefPtr is uesed, but I had to think about where there was no final
> return here.

Not sure what you are talking about (patch might look confusing). All branches have explicit returns. Here is the resulting snippet:

PassRefPtr<SharedBuffer> InspectorResource::resourceData(String* textEncodingName) const {
    if (m_requestURL == m_loader->requestURL()) {
        *textEncodingName = m_frame->document()->inputEncoding();
        return m_loader->mainResourceData();
    } else {
        CachedResource* cachedResource = m_frame->document()->docLoader()->cachedResource(requestURL());
        if (!cachedResource)
            return 0;

        if (cachedResource->isPurgeable()) {
            // If the resource is purgeable then make it unpurgeable to get
            // get its data. This might fail, in which case we return an
            // empty String.
            // FIXME: should we do something else in the case of a purged
            // resource that informs the user why there is no data in the
            // inspector?
            if (!cachedResource->makePurgeable(false))
                return 0;
        }

        *textEncodingName = cachedResource->encoding();
        return cachedResource->data();
    }
}
Comment 4 Timothy Hatcher 2009-09-21 15:01:27 PDT
(In reply to comment #3)
> > I think an explicit return 0; would be good here, I think it happens anyway
> > because PassRefPtr is uesed, but I had to think about where there was no final
> > return here.
> 
> Not sure what you are talking about (patch might look confusing). All branches
> have explicit returns. Here is the resulting snippet:
> 
> PassRefPtr<SharedBuffer> InspectorResource::resourceData(String*
> textEncodingName) const {
>     if (m_requestURL == m_loader->requestURL()) {
>         *textEncodingName = m_frame->document()->inputEncoding();
>         return m_loader->mainResourceData();
>     } else {
>         CachedResource* cachedResource =
> m_frame->document()->docLoader()->cachedResource(requestURL());
>         if (!cachedResource)
>             return 0;
> 
>         if (cachedResource->isPurgeable()) {
>             // If the resource is purgeable then make it unpurgeable to get
>             // get its data. This might fail, in which case we return an
>             // empty String.
>             // FIXME: should we do something else in the case of a purged
>             // resource that informs the user why there is no data in the
>             // inspector?
>             if (!cachedResource->makePurgeable(false))
>                 return 0;
>         }
> 
>         *textEncodingName = cachedResource->encoding();
>         return cachedResource->data();
>     }
> }

OK, I see. Then that else should be removed. We don't do "else" after a return in WebKit.
Comment 5 Pavel Feldman 2009-09-21 15:07:06 PDT
(In reply to comment #4)

> OK, I see. Then that else should be removed. We don't do "else" after a return
> in WebKit.

Done.
Comment 6 Pavel Feldman 2009-09-21 15:15:46 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorResource.cpp
	M	WebCore/inspector/InspectorResource.h
Committed r48606