[webkit-reviews] review granted: [Bug 202716] Make WebInspector's remote debug EventLoop code into RunLoop : [Attachment 380495] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 202716: Make WebInspector's remote debug EventLoop code into RunLoop
https://bugs.webkit.org/show_bug.cgi?id=202716

Attachment 380495: Patch

https://bugs.webkit.org/attachment.cgi?id=380495&action=review




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 380495
  --> https://bugs.webkit.org/attachment.cgi?id=380495
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380495&action=review

r=me, this looks fine to me, but I'd suggest asking Joe to take a look as he's
more familiar with this code

> Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.cpp:79
> +    return "com.apple.JavaScriptCore.remote-inspector-runloop-mode";

`_s`?

> Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.h:41
> +    static String runLoopMode();

NIT: I think we should move this to `RemoteInspectionTarget` since it's more
"generic" than `JSGlobalObjectScriptDebugServer` (`RemoteInspectionTarget` is
also used when inspecting webpages, which doesn't use
`JSGlobalObjectScriptDebugServer`).

> Source/WTF/wtf/win/RunLoopWin.cpp:125
> +    MSG msg;

NIT: we should call this `message` to match the rest of the file

> Source/WTF/wtf/win/RunLoopWin.cpp:127
> +	   m_ended = true;

This no longer exists.

> Source/WTF/wtf/win/RunLoopWin.cpp:132
> +    TranslateMessage(&msg);
> +    DispatchMessage(&msg);

Should these be `::TranslateMessage` and `::DispatchMessage`?