Bug 37604

Summary: [V8] Web Inspector: implement ScriptDebugServer for v8 engine
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: ajwong, bweinstein, commit-queue, dglazkov, eric, gustavo, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch
pfeldman: review-
patch pfeldman: review+

Description Yury Semikhatsky 2010-04-14 13:54:50 PDT
We need to implement ScriptDebugServer for v8. This would allow to have common debugging protocol which wouldn't depend on the underlying JS engine.
Comment 1 Yury Semikhatsky 2010-04-15 06:37:24 PDT
Created attachment 53431 [details]
patch
Comment 2 WebKit Review Bot 2010-04-15 06:38:14 PDT
Attachment 53431 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
 sorted.  [build/include_order] [4]
WebCore/bindings/v8/ScriptDebugServer.cpp:62:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:126:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:144:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:174:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:219:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:291:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:292:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:293:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:295:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:298:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:299:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:300:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:301:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:302:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:303:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:304:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:305:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.cpp:306:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:161:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/chromium/src/js/DebuggerScript.js:182:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:183:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:184:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:185:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:186:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:187:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/src/js/DebuggerScript.js:188:  Line contains tab character.  [whitespace/tab] [5]
WebCore/bindings/v8/ScriptDebugServer.h:134:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/bindings/v8/ScriptDebugServer.h:135:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/chromium/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
WebKit/chromium/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 34 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Eric Seidel (no email) 2010-04-15 06:39:53 PDT
Attachment 53431 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1683276
Comment 4 Early Warning System Bot 2010-04-15 06:42:04 PDT
Attachment 53431 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/1605558
Comment 5 WebKit Review Bot 2010-04-15 06:57:57 PDT
Attachment 53431 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/1727007
Comment 6 Yury Semikhatsky 2010-04-15 07:06:54 PDT
Created attachment 53434 [details]
patch

Fixed errors found by try bots.
Comment 7 WebKit Review Bot 2010-04-15 07:09:34 PDT
Attachment 53434 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 WebKit Review Bot 2010-04-15 07:13:56 PDT
Attachment 53434 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1584495
Comment 9 Yury Semikhatsky 2010-04-15 07:15:04 PDT
Created attachment 53435 [details]
patch

Removed tab from WebCore/ChangeLog
Comment 10 WebKit Review Bot 2010-04-15 07:21:18 PDT
Attachment 53435 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1595517
Comment 11 Pavel Feldman 2010-04-15 07:25:13 PDT
Comment on attachment 53434 [details]
patch

Looks good except for "if (jsEngine === "v8")" in InjectedScript.
Comment 12 Yury Semikhatsky 2010-04-15 08:15:08 PDT
Created attachment 53439 [details]
patch

Fixed Chromium compilation issue related to StringHash.h import
Comment 13 Pavel Feldman 2010-04-15 10:53:15 PDT
Comment on attachment 53439 [details]
patch

Please explain the InjectedScript fork and the way you are going to resolve it asap (as we discussed offline) in the FIXME. Otherwise looks good.
Comment 14 Yury Semikhatsky 2010-04-15 11:11:06 PDT
Created attachment 53453 [details]
patch

Filed a bug(https://bugs.webkit.org/show_bug.cgi?id=37663) and added FIXME comment with regard to call stack iteration.
Comment 15 Yury Semikhatsky 2010-04-15 11:42:02 PDT
Committed r57660
Comment 16 Albert J. Wong 2010-04-15 13:45:42 PDT
Reverted r57660 for reason:

Broke a large number of inspector layout tests in chromium.

Committed r57670: <http://trac.webkit.org/changeset/57670>
Comment 17 Yury Semikhatsky 2010-04-15 18:10:51 PDT
Committed exactly same patch r57701. The Chromium test failure was related to gyp dependency tracking(see http://code.google.com/p/chromium/issues/detail?id=29695).