Bug 42340 – Web Inspector: extract debugger implementation into DebuggerAgent

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

There is a bunch of methods on the InspectorBackend providing debugger facilities. All of them can be easily extracted and placed into there own class DebuggerAgent. That would limit the InspectorController responsibilities.

[Yury Semikhatsky]

Created attachment 63707 [details]
Patch

[Ilya Tikhonovsky]

Comment on attachment 63707 [details]
Patch

WebCore/GNUmakefile.am:1600
 +  	WebCore/inspector/InspectorDebuggerAgent.h \
Incorrect order.

WebCore/WebCore.xcodeproj/project.pbxproj: 
 +  			developmentRegion = English;
is it necessary?

WebCore/inspector/InspectorDebuggerAgent.cpp:44
 +  static String md5Base16(const String& string)
Why do you need your own implementation of md5Base16?

WebCore/inspector/InspectorDebuggerAgent.cpp:70
 +      ScriptDebugServer::shared().addListener(agent.get(), inspectorController->inspectedPage());
I think it would be better to move this code to ctor.

[WebKit Review Bot]

Attachment 63707 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3624570

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 63707 [details])
> WebCore/GNUmakefile.am:1600
>  +      WebCore/inspector/InspectorDebuggerAgent.h \
> Incorrect order.
> 
Fixed.

> WebCore/WebCore.xcodeproj/project.pbxproj: 
>  +              developmentRegion = English;
> is it necessary?
> 
No. Thanks.

> WebCore/inspector/InspectorDebuggerAgent.cpp:44
>  +  static String md5Base16(const String& string)
> Why do you need your own implementation of md5Base16?
> 
It was there before. Just forgot to remove it from InspectorController.cpp

> WebCore/inspector/InspectorDebuggerAgent.cpp:70
>  +      ScriptDebugServer::shared().addListener(agent.get(), inspectorController->inspectedPage());
> I think it would be better to move this code to ctor.
I'd rather keep it in the create method to make sure it's called only when the object is fully constructed. It would work either way I think since the constructor is private.

[Yury Semikhatsky]

Created attachment 63714 [details]
Patch

[Yury Semikhatsky]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/CMakeLists.txt
	M	WebCore/ChangeLog
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/inspector/CodeGeneratorInspector.pm
	M	WebCore/inspector/Inspector.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	A	WebCore/inspector/InspectorDebuggerAgent.cpp
	A	WebCore/inspector/InspectorDebuggerAgent.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	M	WebCore/inspector/front-end/ScriptsPanel.js
Committed r64846

[Andrei Popescu]

Created attachment 63733 [details]
Patch

[Andrei Popescu]

(In reply to comment #7)
> Created an attachment (id=63733) [details]
> Patch

Sorry, passed wrong bug no to webkit-patch :)

[WebKit Review Bot]

http://trac.webkit.org/changeset/64846 might have broken Qt Windows 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/64845
http://trac.webkit.org/changeset/64846