inspector: implement V8Inspector timer#14346
inspector: implement V8Inspector timer#14346eugeneo merged 0 commit intonodejs:masterfrom eugeneo:timers
Conversation
src/inspector_agent.cc
Outdated
There was a problem hiding this comment.
nit: I’d align the arguments vertically instead, that seems a bit more readable
src/inspector_agent.cc
Outdated
There was a problem hiding this comment.
Doesn’t just timer_() work as well?
Also, I’d really rather align the arguments (and intializers) vertically for readability here
src/inspector_agent.cc
Outdated
There was a problem hiding this comment.
This is just a question, not a request to change anything: Does V8 have any documention saying that this interval is supposed to be given in seconds? Where would one find that?
There was a problem hiding this comment.
I don't think so. I tried to find it myself... This code originally came from Chrome where it seem to be dated back to WebKit days.
src/inspector_agent.cc
Outdated
There was a problem hiding this comment.
This should uv_close() the timer (and not free the memory until the close callback is called.)
There was a problem hiding this comment.
Oops. Thanks for pointing it out! I fixed it.
|
@bnoordhuis Please take a look. |
|
I'm waiting for arm-fanned CI bot to recover - not feeling confident enough to push this without as little as compiling on all platforms. |
|
Landed as 012206e |
This timer is currently only used by a profiler. Note that the timer invocation will not interrupt JavaScript code. Fixes: #11401 PR-URL: #14346 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This timer is currently only used by a profiler. Note that the timer
invocation will not interrupt JavaScript code.
Fixes: #11401
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
inspector: implemented two client methods.