Bug 39594 – [v8] ScriptDebugServer should dispatch renderer messages while JS is paused

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

[v8] ScriptDebugServer should dispatch renderer messages while JS is paused.

[Yury Semikhatsky]

Created attachment 56879 [details]
patch

[WebKit Review Bot]

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.

[Yury Semikhatsky]

Created attachment 56881 [details]
patch

Fixed style errors.

[Pavel Feldman]

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?

[Yury Semikhatsky]

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

[Yury Semikhatsky]

Created attachment 56999 [details]
patch that I'm going to land

[Yury Semikhatsky]

Committed r60154

[WebKit Review Bot]

http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release

[Yury Semikhatsky]

(In reply to comment #8)
> http://trac.webkit.org/changeset/60154 might have broken Chromium Mac Release

Fixed in r60155.