Bug 29537 – Web Inspector: Expose InspectorResource fields

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

Currently InspectorResource has all of its members private. Expose them via the API for potential processing in C++ (as agreed with Timothy long ago).

[Pavel Feldman]

Created attachment 39845 [details]
patch

[Timothy Hatcher]

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.

[Pavel Feldman]

> 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();
    }
}

[Timothy Hatcher]

(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.

[Pavel Feldman]

(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.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorResource.cpp
	M	WebCore/inspector/InspectorResource.h
Committed r48606