[webkit-reviews] review denied: [Bug 73099] Web Inspector: Unsafe cross origin access errors should show stack trace in console. : [Attachment 116627] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Nov 28 20:27:53 PST 2011
Sam Weinig <sam at webkit.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 116627: Patch
https://bugs.webkit.org/attachment.cgi?id=116627&action=review
------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=116627&action=review
> Source/WebCore/bindings/v8/V8Proxy.cpp:172
> + addMessageToConsole(page, str, kSourceID, kLineNumber, stackTrace);
You could release the stackTrace here, since addMessageToConsole takes a
PassRefPtr.
> Source/WebCore/page/DOMWindow.cpp:143
> + PassRefPtr<ScriptCallStack> stackTrace() const { return m_stackTrace; }
This should return a raw pointer, since you are not relinquishing the ref.
> Source/WebCore/page/DOMWindow.cpp:908
> + // Capture stack trace only when inspector front-end is loaded as it may
be time consuming.
> + RefPtr<ScriptCallStack> stackTrace;
> + if (InspectorInstrumentation::hasFrontends())
> + stackTrace =
createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);
In other places, I believe we used settings()->developerExtrasEnabled() for
this check.
> Source/WebCore/page/DOMWindow.cpp:911
> + PostMessageTimer* timer = new PostMessageTimer(this, message,
sourceOrigin, source, channels.release(), target.get(), stackTrace);
This could use stackTrace.release().
> Source/WebCore/page/DOMWindow.cpp:925
> + RefPtr<ScriptCallStack> stackTrace = timer->stackTrace();
There doesn't seem to be a good reason to put this in a RefPtr, since you never
use it.
> Source/WebCore/page/DOMWindow.cpp:1778
> // FIXME: Add arguments so that we can provide a correct source URL and
line number.
> - console()->addMessage(JSMessageSource, LogMessageType,
ErrorMessageLevel, message, 1, String());
> + RefPtr<ScriptCallStack> stackTrace =
createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);
> + console()->addMessage(JSMessageSource, LogMessageType,
ErrorMessageLevel, message, 1, String(), stackTrace);
If we have a stack track, can we also have the URL and line number?
> Source/WebCore/xml/XMLHttpRequest.cpp:483
> + // Capture stack trace only when inspector front-end is loaded as it may
be time consuming.
> + if (InspectorInstrumentation::hasFrontends())
> + m_stackTrace =
createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);
Same comment as about about settings()->developerExtrasEnabled()
More information about the webkit-reviews
mailing list