[webkit-reviews] review denied: [Bug 95187] Web Inspector: XHR replay : [Attachment 160952] Patch

[bugzilla-daemon at webkit.org]

Vsevolod Vlasov <vsevik at chromium.org> has denied Pavel Chadnov
<chadnov at google.com>'s request for review:
Bug 95187: Web Inspector: XHR replay
https://bugs.webkit.org/show_bug.cgi?id=95187

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

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160952&action=review


Please add tests.

> Source/WebCore/inspector/Inspector.json:878
> +		   "description": "Replays XHR.",

Please make this description more verbose. It should mention that XHR resent is
identical to the original one and specify the concrete parameters that are
identical in them.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:333
> +void InspectorInstrumentation::DidCreateXhrRequestImpl(InstrumentingAgents*
instrumentingAgents, String method, bool async, PassRefPtr<FormData> formData,
const HTTPHeaderMap& headers, bool includeCredentials, String login, String
password)

didCreateXhrRequestImpl

> Source/WebCore/inspector/InspectorResourceAgent.h:71
> +struct XhrData;

XHRReplayData

> Source/WebCore/inspector/InspectorResourceAgent.h:-108
> -

Please revert unneeded changes.

> Source/WebCore/inspector/front-end/NetworkPanel.js:965
> +	   if (request && request.type.name() === "xhr") {

request.type === WebInspector.resourceTypes.XHR

> Source/WebCore/inspector/front-end/NetworkRequest.js:801
> +	   

Please revert

> Source/WebCore/loader/DocumentThreadableLoader.cpp:403
> +    if (m_preflightRequestIdentifier)

Let's make it look like 
if (m_actualRequest)
  ... 
else if (m_preflightRequestIdentifier)
  ... 
for consistency.

> Source/WebCore/xml/XMLHttpRequest.cpp:690
> +    createRequest(ec);

As discussed we should set Content-Type header here.