Notifying the debugger agent of events in the lifecycle of an asynchronous operation should be done by the relevant agent. Currently this is the job of InspectorInstrumenation, which handles timers and rAF. When support is added for XHR.send, postMessage, addEventListener, etc, the layering violation will get worse. Move the AsynchronousCallType enum to its own file, and other code specific to async stacks to the relevant agent.
Created attachment 316157 [details] Patch
Comment on attachment 316157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316157&action=review r- unless everyone else disagrees on the enum vs int signature issue. I wasn't paying attention when this code originally landed. > Source/WebCore/inspector/InspectorAsynchronousCallType.h:28 > +namespace WebCore { Please, use namespace Inspector unless it's really not possible. > Source/WebCore/inspector/InspectorAsynchronousCallType.h:31 > + AnimationFrame = 1, Nit: RequestAnimationFrame > Source/WebCore/inspector/InspectorAsynchronousCallType.h:32 > + Timer, Nit: can we call this DOMTimer to be consistent with the relevant C++ class? > Source/WebCore/inspector/InspectorInstrumentation.cpp:349 > + debuggerAgent->didCancelAsyncCall(static_cast<int>(AsynchronousCallType::Timer), timerId); Nooo stop it :( EDIT: I see that we did this to avoid mentioning Web-specific concepts in JSC / Debugger.json. I have no such aversion. This code should be in Inspector namespace anyway. Passing around ints is worse than some optional enum values having no meaning in JavaScriptCore proper. > Source/WebCore/inspector/PageDebuggerAgent.cpp:160 > + JSC::ExecState* scriptState = document->execState(); The dereference of document needs a null check, or passed to here by reference.
Comment on attachment 316157 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316157&action=review > Source/WebCore/ChangeLog:3 > + Web Inspector: Refactoring: remove async stack trace logic from InspectorInstrumentation I'd prefer "extract" over "remove".
Created attachment 316338 [details] Patch
Comment on attachment 316338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316338&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + Move AsyncCallType enum to InspectoprDebuggerAgent, which manages async Nit: InspectorDebuggerAgent
Created attachment 316389 [details] Patch
Comment on attachment 316389 [details] Patch Clearing flags on attachment: 316389 Committed r219889: <http://trac.webkit.org/changeset/219889>
All reviewed patches have been landed. Closing bug.