[webkit-reviews] review denied: [Bug 73099] Web Inspector: Unsafe cross origin access errors should show stack trace in console. : [Attachment 118376] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Vsevolod Vlasov
<vsevik at chromium.org>'s request for review:
Bug 73099: Web Inspector: Unsafe cross origin access errors should show stack
trace in console.
https://bugs.webkit.org/show_bug.cgi?id=73099

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

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


> Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp:41
> +#include "ScriptCallStack.h"

Please mind the alphabetic order

> Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp:118
> +    impl()->setCallStack(createScriptCallStack(exec));

I think createScriptCallStack is a bad name for the method, sorry I missed the
moment it got landed. You should explicitly state that the stack depth depends
on the inspector front-end existence.

> Source/WebCore/bindings/v8/V8Proxy.cpp:166
> +    RefPtr<ScriptCallStack> stackTrace =
createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);

I don't see a corresponding change to JSC. Is this a part of a different
change? You should probably limit the scope of this change to XHR and keep
symmetry here.

> Source/WebCore/xml/XMLHttpRequest.h:128
> +    void setCallStack(PassRefPtr<ScriptCallStack>);

setLastSendCallStack ?

> Source/WebCore/xml/XMLHttpRequest.h:228
> +    RefPtr<ScriptCallStack> m_callStack;

m_lastSendCallStack