[webkit-reviews] review canceled: [Bug 52294] Web Inspector: Extract BrowserDebuggerAgent from InspectorController, InspectorDOMAgent and InspectorDebugger agent. : [Attachment 78690] [patch] next iteration.

[bugzilla-daemon at webkit.org]

Ilya Tikhonovsky <loislo at chromium.org> has canceled Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 52294: Web Inspector: Extract BrowserDebuggerAgent from
InspectorController, InspectorDOMAgent and InspectorDebugger agent.
https://bugs.webkit.org/show_bug.cgi?id=52294

Attachment 78690: [patch] next iteration.
https://bugs.webkit.org/attachment.cgi?id=78690&action=review

------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>
(In reply to comment #7)
> (From update of attachment 78690 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=78690&action=review
> 
> > Source/WebCore/inspector/InspectorController.cpp:735
> > +	     restoreStickyBreakpoints();
> 
> Why did this call move? Should it be done only in case there is at least one
of the agents?

done.

> 
> > Source/WebCore/inspector/InspectorController.cpp:1270
> > +	 if (type == eventListenerBreakpointType && m_browserDebuggerAgent) {
> 
> This method should become a part of a debugger agent.

The idea was to extract DOM depended debugging things into separate class.

> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:92
> > +	 if (browserDebuggerAgent->shouldBreakOnNodeInsertion(node, parent,
eventData)) {
> 
> This logic should move into the debugger agent, InspectorInstrumentation
should do nothing but pure dispatching.

done.

> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:118
> >	     eventData->setString("breakpointType", domNativeBreakpointType);
> 
> ditto

done.
> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:145
> >	     eventData->setString("breakpointType", domNativeBreakpointType);
> 
> ditto


done.