[webkit-reviews] review granted: [Bug 45953] Web Inspector: [Extensions API] Expose access to resource bodies : [Attachment 69793] patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Andrey Kosyakov
<caseq at chromium.org>'s request for review:
Bug 45953: Web Inspector: [Extensions API] Expose access to resource bodies
https://bugs.webkit.org/show_bug.cgi?id=45953

Attachment 69793: patch
https://bugs.webkit.org/attachment.cgi?id=69793&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
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.