[webkit-reviews] review denied: [Bug 105315] Web Inspector: introduce Page.captureScreenshot : [Attachment 180147] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 105315: Web Inspector: introduce Page.captureScreenshot
https://bugs.webkit.org/show_bug.cgi?id=105315

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

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=180147&action=review


> Source/WebKit/chromium/public/WebDevToolsAgent.h:101
> +    WEBKIT_EXPORT static WebString patchWithBrowserData(const WebString&
message, BrowserDataHint, const WebString& hintData);

This means that the client of this API will always be responsible for
formatting the hintData. Is it always going to be a string? What about raw
cookies, wouldn't we need to serialize them in some protocol-defined format if
so then we should expose a way to set the patch data and and let WebCore
serialize it into a message.

>>> Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:84
>>> +static const char screenshot[] = "screenshot";
>> 
>> Guess we should migrate to "static const char* const" whenever possible (to
make use of the constant memory)
> 
> Will do.

This sounds weird, the array content is already immutable, why making the
pointer itself immutable changes where its content is stored?