Summary: | [v8] ScriptDebugServer should dispatch renderer messages while JS is paused | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yury Semikhatsky <yurys> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Yury Semikhatsky <yurys> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, bweinstein, eric, joepeck, keishi, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Yury Semikhatsky
2010-05-24 06:03:50 PDT
Created attachment 56879 [details]
patch
Attachment 56879 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/chromium/src/WebDevToolsAgentImpl.cpp:136: Place brace on its own line for function definitions. [whitespace/braces] [4]
WebKit/chromium/src/WebDevToolsAgentImpl.cpp:143: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 2 in 13 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 56881 [details]
patch
Fixed style errors.
Comment on attachment 56881 [details]
patch
WebCore/bindings/js/ScriptDebugServer.cpp:343
+ (this->*callback)(copy[i]);
Nit: I generally do not use such a reflection in the native code. Having ::dispatchDidContinue or a single event listener class with an event type would be my preference.
WebCore/bindings/v8/ScriptDebugServer.h:85
+ class ClientMessageLoop {
It might be useful to declare it in a separate header to minimize the amount of imports on the client.
WebKit/chromium/public/WebDevToolsAgentClient.h:59
+ class ClientMessageLoop {
Should we name it differently? Like WebKitClientMessageLoop?
(In reply to comment #4) > WebCore/bindings/v8/ScriptDebugServer.h:85 > + class ClientMessageLoop { > It might be useful to declare it in a separate header to minimize the amount of imports on the client. > ScriptDebugServer.h is included there anyway to set implementation of the loop so I'd rather leave it this way because it emphasizes that the message loop is used from the debugger. > WebKit/chromium/public/WebDevToolsAgentClient.h:59 > + class ClientMessageLoop { > Should we name it differently? Like WebKitClientMessageLoop? Done. Created attachment 56999 [details]
patch that I'm going to land
http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release (In reply to comment #8) > http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release Fixed in r60155. |