Summary: | Web Inspector: Debugger Pause button does not work | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bunhere, commit-queue, dbates, graouts, gyuyoung.kim, joepeck, rakuco, rego, sergio, timothy, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-07-09 17:29:44 PDT
DebuggerManager.prototype.debuggerDidPause has new code that may automatically resume if the call stack it creates is empty. That may be this. (In reply to comment #2) > DebuggerManager.prototype.debuggerDidPause has new code that may automatically resume if the call stack it creates is empty. That may be this. Yes, this is the case. In this case, the call stack is an anonymous script, which does not have a URL, so we don't create a call frame for it. Details of the anonymous script are: js> JSON.stringify(callFramePayload.location) "{"scriptId":"31","lineNumber":0,"columnNumber":0}" js> WebInspector.debuggerManager.scriptForIdentifier("31").displayName "Anonymous Script 1" js> WebInspector.debuggerManager.scriptForIdentifier("31").requestContent(function() { console.log(arguments); }) [Object , "(function() {if(!s.getPPVid)return;var dh=Math.max(Math.max(s.d.body.scrollHeight,s...", false] Which correlates to this code on the page: (s_code_h.js) s.handlePPVevents = new Function("", "if(!s.getPPVid)return;var dh=Math.max(Math.max(s.d.body.scrollHeight,s..."); So, I think we should be able to show this. We should not skip over scripts without a URL. Yes, awesome! Now this shows anonymous scripts in call frames and we can step through them! Huge improvement. So, now we will be able to step into this code into the anonymous function:
Test:
> <script>
> var f = new Function("\nconsole.log(1);\nconsole.log(2);\nconsole.log(3);\n");
> function doit() {
> f();
> }
> </script>
Inspector:
js> doit();
step into and through f()
A problem right now is InjectedScriptSource.js gets its //#sourceURL comment stripped when it gets minified. I have a fix for adding that comment back when we minify.
Created attachment 234679 [details]
[PATCH] Proposed Fix
Comment on attachment 234679 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=234679&action=review > Source/JavaScriptCore/CMakeLists.txt:902 > + COMMAND ${PERL_EXECUTABLE} -e 'print "//# sourceURL=__WebInspectorInjectedScript__\n"' > ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InjectedScriptSource.min.js Is there is a better, shared, place to add this line? I guess the same question applies to the rest of the build logic that is duplicated. Maybe we can hack jsmin.py to not strip sourceURL? Likely not worth it though… > Source/WebInspectorUI/ChangeLog:13 > + Now that we allow anonymous scripts we can step into anonymous We have always allowed anon scripts. I think this was just an oversight with the skipping code. > Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:-373 > - if (!this._callFrames.length) { > - this.resume(); > - return; > - } I wonder how often this legitimately happens still. Comment on attachment 234679 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=234679&action=review >> Source/JavaScriptCore/CMakeLists.txt:902 >> + COMMAND ${PERL_EXECUTABLE} -e 'print "//# sourceURL=__WebInspectorInjectedScript__\n"' > ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InjectedScriptSource.min.js > > Is there is a better, shared, place to add this line? I guess the same question applies to the rest of the build logic that is duplicated. > > Maybe we can hack jsmin.py to not strip sourceURL? Likely not worth it though… Correct, I think the idea fix would be to use a minifier that doesn't strip such comments. But, as a workaround, I felt this was a small enough change that I could just put it here. It only happens for two files. This seems to break the EFL build though, I'll need to figure that out. >> Source/WebInspectorUI/ChangeLog:13 >> + Now that we allow anonymous scripts we can step into anonymous > > We have always allowed anon scripts. I think this was just an oversight with the skipping code. Yep. >> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:-373 >> - } > > I wonder how often this legitimately happens still. If it does legitimately happen, we still have a good UI that we show. Something like "No Call Frames" in the sidebar. Maybe it can happen for eval("console.log(1);\ndebugger;\nconsole.log(2);"). I'll try it. Syntax Warning in cmake code at
/home/gyuyoung/eflews/WebKit/Source/JavaScriptCore/CMakeLists.txt:902:90
Argument not separated from preceding token by whitespace.
This warning is for project developers. Use -Wno-dev to suppress it.
The line is:
> COMMAND ${PERL_EXECUTABLE} -e 'print "//# sourceURL=__WebInspectorInjectedScript__\n"' > ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InjectedScriptSource.min.js
Maybe CMakeLists.txt is using the \n itself instead of letting perl handle it? I'm baffled right now. I just want to append this text to the top of the file, maybe there is a better way. I'll try switching to python.
Created attachment 234711 [details]
[PATCH] For Bots
See if this fixes GTK / EFL. I switched from `perl -e '...'` to `python -c '...'` and got rid of "\n" characters in the CMake file.
Comment on attachment 234711 [details] [PATCH] For Bots View in context: https://bugs.webkit.org/attachment.cgi?id=234711&action=review > Source/JavaScriptCore/CMakeLists.txt:902 > + COMMAND ${PYTHON_EXECUTABLE} -c 'print "//# sourceURL=__WebInspectorInjectedScript__"' > ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InjectedScriptSource.min.js Why can't you use echo here too? (In reply to comment #11) > (From update of attachment 234711 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234711&action=review > > > Source/JavaScriptCore/CMakeLists.txt:902 > > + COMMAND ${PYTHON_EXECUTABLE} -c 'print "//# sourceURL=__WebInspectorInjectedScript__"' > ${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/InjectedScriptSource.min.js > > Why can't you use echo here too? I was unsure if I could in CMake. I'll try. Created attachment 234724 [details]
[PATCH] For Bots 2
Created attachment 234769 [details]
[PATCH] For Bots 3
Bots were red all day yesterday so the patch could not be tested. Rebased and trying again.
Comment on attachment 234769 [details]
[PATCH] For Bots 3
rego tested this patch locally and it worked! I'll land.
FWIW, the first patch 234679 was breaking the build in GTK+ and causing issues in the EWS bots. The last one 234769 builds properly in GTK+. |