[webkit-reviews] review denied: [Bug 46086] Web Inspector: implement pausing on XHR : [Attachment 68359] Proposed patch.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 46086: Web Inspector: implement pausing on XHR
https://bugs.webkit.org/show_bug.cgi?id=46086

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68359&action=review

> WebCore/bindings/v8/custom/V8XMLHttpRequestCustom.cpp:117
> +   
InspectorController::instrumentWillSendXMLHttpRequest(xmlHttpRequest->scriptExe
cutionContext(), xmlHttpRequest->url());

Please guard this with #ifdef

> WebCore/inspector/Inspector.idl:140
> +	   [handler=Controller] void setNativeBreakpoint(in unsigned int type,
in Object breakpoint, out unsigned int breakpointId);

It is unclear how to specify 'breakpoint' parameter format. Why does it have
url property? Should we merge 'type' into 'breakpoint' instead? Like

breakpoint: { type: "XHR", condition: { url : "foo" } }

> WebCore/inspector/InspectorController.h:492
> +    ASSERT(document);

Counter?