[webkit-reviews] review denied: [Bug 111889] Web Inspector: Error handling for ScriptPreprocessor : [Attachment 193950] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied johnjbarton
<johnjbarton at chromium.org>'s request for review:
Bug 111889: Web Inspector: Error handling for ScriptPreprocessor
https://bugs.webkit.org/show_bug.cgi?id=111889

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

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


> Source/WebCore/bindings/v8/ScriptDebugServer.cpp:109
> +	   if (tryCatch.HasCaught()) {

Did you try calling SetVerbose on the handler instead of all of this code?

> Source/WebCore/bindings/v8/ScriptDebugServer.cpp:123
> +	       exceptionTarget->document()->reportException("preprocessor
script must evaluate to a function", 0, "", 0);

Please do not reach for the console via document - there is now PageConsole
accessible from page. You might need to be Page-aware (PageScriptDebugServer)

> Source/WebCore/bindings/v8/V8Binding.cpp:88
> +    String exceptionMessage =
toWebCoreStringWithUndefinedOrNullCheck(message->Get());

I don't think we need another method of a kind. Certainly not here.