Bug 45953 - Web Inspector: [Extensions API] Expose access to resource bodies
Summary: Web Inspector: [Extensions API] Expose access to resource bodies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-17 02:48 PDT by Andrey Kosyakov
Modified: 2010-10-19 02:44 PDT (History)
9 users (show)

See Also:


Attachments
patch (30.78 KB, patch)
2010-10-05 08:52 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
patch to land (30.01 KB, patch)
2010-10-06 03:12 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2010-09-17 02:48:26 PDT
Extensions API currently offers resource meta-information in HAR format, but skips the bodies for efficiency reasons. It is suggested to expose a separate webInspectror.resources.getBodies(resourceIdList) method for extensions to access the resource bodies.
Ideally, we would use blobs to return the bodies. However, it appears that blobs are not fully functional yet, so it is suggested to return strings, using base64 for binary resource types.
Comment 1 Andrey Kosyakov 2010-10-05 08:52:47 PDT
Created attachment 69793 [details]
patch
Comment 2 Yury Semikhatsky 2010-10-06 01:33:42 PDT
Comment on attachment 69793 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69793&action=review

> LayoutTests/inspector/extensions-api-expected.txt:63
> +        getBodies : <function>

We should be consistent in naming either body or content everywhere.

> LayoutTests/inspector/extensions-resources.html:98
> +        return (a.response && a.response.content || "").localeCompare(b.response && b.response.content || "");

please embrace the && expressions in bracketsю

> WebCore/inspector/InspectorController.cpp:2095
> +void InspectorController::getResourceContent(unsigned long identifier, const String& encoding, String* content)

I'd rather pass it as a boolean flag. We're not going to support other encodings any time soon.

> WebCore/inspector/front-end/ExtensionServer.js:297
> +        if (typeof message.ids === "number") {

I don't think we should support non-array values for the field called ids.
Comment 3 Andrey Kosyakov 2010-10-06 03:12:23 PDT
Created attachment 69912 [details]
patch to land
Comment 4 Andrey Kosyakov 2010-10-06 03:16:30 PDT
(In reply to comment #2)
> (From update of attachment 69793 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69793&action=review
> 
> > LayoutTests/inspector/extensions-api-expected.txt:63
> > +        getBodies : <function>
> 
> We should be consistent in naming either body or content everywhere.

Done.

> 
> > LayoutTests/inspector/extensions-resources.html:98
> > +        return (a.response && a.response.content || "").localeCompare(b.response && b.response.content || "");
> 
> please embrace the && expressions in bracketsю

Done.
> 
> > WebCore/inspector/InspectorController.cpp:2095
> > +void InspectorController::getResourceContent(unsigned long identifier, const String& encoding, String* content)
> 
> I'd rather pass it as a boolean flag. We're not going to support other encodings any time soon.
> 

Done.

> > WebCore/inspector/front-end/ExtensionServer.js:297
> > +        if (typeof message.ids === "number") {
> 
> I don't think we should support non-array values for the field called ids.

Agreed offline to leave it as is. The idea is that we want to support array of ids to reduce latencies on roundtrips when processing large number of resources, yet uses cases when once requests a single resource id are probably equally frequent, and while it wouldn't be hard for a user to write '[ id ]', it may be a bit counterintuitive.
Comment 5 Andrey Kosyakov 2010-10-08 07:03:53 PDT
Manually committed r69185: http://trac.webkit.org/changeset/69185
Comment 6 Eric Seidel (no email) 2010-10-14 07:19:11 PDT
Comment on attachment 69793 [details]
patch

Cleared Yury Semikhatsky's review+ from obsolete attachment 69793 [details] so that this bug does not appear in http://webkit.org/pending-commit.