Bug 39594 - [v8] ScriptDebugServer should dispatch renderer messages while JS is paused
Summary: [v8] ScriptDebugServer should dispatch renderer messages while JS is paused
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-24 06:03 PDT by Yury Semikhatsky
Modified: 2010-05-25 05:32 PDT (History)
11 users (show)

See Also:


Attachments
patch (34.20 KB, patch)
2010-05-24 06:31 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (34.21 KB, patch)
2010-05-24 06:42 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff
patch that I'm going to land (34.31 KB, patch)
2010-05-25 03:05 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2010-05-24 06:03:50 PDT
[v8] ScriptDebugServer should dispatch renderer messages while JS is paused.
Comment 1 Yury Semikhatsky 2010-05-24 06:31:57 PDT
Created attachment 56879 [details]
patch
Comment 2 WebKit Review Bot 2010-05-24 06:34:29 PDT
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.
Comment 3 Yury Semikhatsky 2010-05-24 06:42:56 PDT
Created attachment 56881 [details]
patch

Fixed style errors.
Comment 4 Pavel Feldman 2010-05-24 11:33:51 PDT
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?
Comment 5 Yury Semikhatsky 2010-05-25 03:04:36 PDT
(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.
Comment 6 Yury Semikhatsky 2010-05-25 03:05:27 PDT
Created attachment 56999 [details]
patch that I'm going to land
Comment 7 Yury Semikhatsky 2010-05-25 04:46:21 PDT
Committed r60154
Comment 8 WebKit Review Bot 2010-05-25 05:01:52 PDT
http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release
Comment 9 Yury Semikhatsky 2010-05-25 05:32:30 PDT
(In reply to comment #8)
> http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release

Fixed in r60155.