Bug 111889 – Web Inspector: Error handling for ScriptPreprocessor

• Component: Web Inspector (Deprecated)
• Status: RESOLVED INVALID

[johnjbarton]

In using the scriptpreprocessor introduced in Bug 80992 and Bug 104384, I found I could succeed with trivial preprocessors but not real JS to JS compilers.

The problem is two-fold: 
1) the environment of the preprocessor is not like any web page: the compiler source must be inside of a function body, not a global scope and the compiler cannot reference console or window.
2) Compile and run time errors are not reported, but result in silently ignoring the preprocessor.

Discovering the first set of issues without the second is pretty much impossible.

[johnjbarton]

Created attachment 192286 [details]
Patch

[johnjbarton]

By adding some LOG_ERROR calls I was able to succeed with my realistic preprocessor (based on traceur JS to JS compiler). 

The next step would plumb the errors back to JS land on the client side, but I don't know how to do that since the errors are asynchronous to the reload() call. For this purpose I think sending console errors back to the debugger would be fine, also something I don't know how to do.

[johnjbarton]

Created attachment 193950 [details]
Patch

[Early Warning System Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17179419

[Early Warning System Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17139606

[Build Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17191462

[Build Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17190644

[kov's GTK+ EWS bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17129670

[EFL EWS Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17139615

[WebKit Review Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17139633

New failing tests:
inspector/debugger/debugger-script-preprocessor.html

[WebKit Review Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17066874

New failing tests:
inspector/debugger/debugger-script-preprocessor.html

[Build Bot]

Comment on attachment 193950 [details]
Patch

Attachment 193950 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17125705

[Pavel Feldman]

Comment on attachment 193950 [details]
Patch

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.

[johnjbarton]

(In reply to comment #13)
> (From update of attachment 193950 [details])
> 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?

Setting tryCatch.setVerbose(true) triggers WebKit/Source/WebCore/bindings/v8/V8Initializer messageHandlerInMainThread, a message listener on v8::V8.  The messageHandlerInMainThread() only works if called on a context related to the main thread window. The preprocessor however is in a utility context and has no 'window' so messageHandlerInMainThread() crashes.  Since this handler is hard wired into the thread, there is no practical way to replace it. 

> 
> > 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)

I don't understand what you mean by Page-aware or Page for that matter. But I can remove the path though document().

> 
> > 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.

Sorry I don't understand "method of a kind"? To be sure, I don't know which conversion from Message to String is appropriate here, I just used one that was already used in the file. What should I use instead?

[johnjbarton]

Created attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

[Early Warning System Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17155737

[Early Warning System Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17286056

[kov's GTK+ EWS bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17190723

[EFL EWS Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17178671

[Build Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17155742

[Build Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17232360

[Build Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17196627

[Build Bot]

Comment on attachment 194082 [details]
Fix 1/2 issues from comment #13; move -expected.txt file to the correct location

Attachment 194082 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17136780

[johnjbarton]

Created attachment 194146 [details]
Add name for preprocessor script so we get line numbers in error messages; fix bindings/js for other platform builds

[johnjbarton]

I worked with this patch today. I was able to use some of the error messages to fix some of my problems. 

The remaining problem is related to how the Inspector works. If the preprocessor returns a results with a syntax error, then the browser's JS compiler never sees the //@ sourceURL. So the compilation is associated with the original source URL but we don't get line numbers (I guess because the compiler sees no name at all). The result is confusing: an error message against the original source which has no error. 

(This case is more common than you might think because it just means the preprocessor returned a string that is not JS).

Somehow the preprocessor output is not registered with the Inspector in this case.

(I also hit a problem not directly related to error processing: Inspector evaluations also pass thru the preprocessor (surprise!). My transcoder did not deal with "with" at top level, so I ended up in the case above.)

[Build Bot]

Comment on attachment 194146 [details]
Add name for preprocessor script so we get line numbers in error messages; fix bindings/js for other platform builds

Attachment 194146 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17216473

[Build Bot]

Comment on attachment 194146 [details]
Add name for preprocessor script so we get line numbers in error messages; fix bindings/js for other platform builds

Attachment 194146 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17258152

[johnjbarton]

Created attachment 194643 [details]
Fix signature for mac builds

[Build Bot]

Comment on attachment 194643 [details]
Fix signature for mac builds

Attachment 194643 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17207680

New failing tests:
inspector/debugger/debugger-script-preprocessor.html

[Build Bot]

Created attachment 194687 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Build Bot]

Comment on attachment 194643 [details]
Fix signature for mac builds

Attachment 194643 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17240440

New failing tests:
svg/batik/filters/feTile.svg
inspector/debugger/debugger-script-preprocessor.html

[Build Bot]

Created attachment 194700 [details]
Archive of layout-test-results from webkit-ews-06 for mac-future

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-future  Platform: Mac OS X 10.8.2

[johnjbarton]

Created attachment 195413 [details]
Try again on mac port expected.txt

[Brian Burg]

Current inspector doesn't have preprocessor.