[webkit-reviews] review canceled: [Bug 46624] Web Inspector: implement pausing on event listeners (back-end part) : [Attachment 69343] Patch.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has canceled Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 46624: Web Inspector: implement pausing on event listeners (back-end part)
https://bugs.webkit.org/show_bug.cgi?id=46624

Attachment 69343: Patch.
https://bugs.webkit.org/attachment.cgi?id=69343&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69343&action=review

>> WebCore/inspector/InspectorInstrumentation.cpp:162
>> +	inspectorController->instrumentWillDispatchEvent(event);
> 
> see comments about this method in InspectorController

Could you rename this method so that it would better reflect the nature of the
call?

> WebCore/inspector/InspectorInstrumentation.cpp:180
> +    TimelineAgentsStack& stack = timelineAgentsStack();

Extract method please. Also this is very, very complex. Could we return Page*
from all *::will methods and call ::did with it?

> WebCore/inspector/InspectorInstrumentation.cpp:184
> +	   stack.removeLast();

You should check depth before this call. Also, please add assertions here with
depth expectations.