%SUBJ% The next step to support Remote Debugging in WebKit.
Created attachment 60838 [details] [patch] initial version. Not for review.
Created attachment 60856 [details] [patch] second iteration
Comment on attachment 60856 [details] [patch] second iteration WebCore/inspector/InspectorDOMAgent.cpp:236 + m_frontend->setDocument(InspectorObject::create()); This should be InspectorValue::null() really. Frontend will support it. WebCore/inspector/InspectorDOMAgent.cpp:289 + m_frontend->childNodeInserted(parentId, prevId, value.release()); I think this only adds complexity. We should keep ownership and rely upon the sync nature of the call. WebCore/inspector/InspectorDOMAgent.cpp:1105 + m_frontend->didGetStyleSheet(callId, InspectorObject::create()); null()
Comment on attachment 60856 [details] [patch] second iteration WebCore/bindings/js/ScriptEventListener.cpp:116 + return ustringToString(jsFunction->toString(mainWorldScriptState(document->frame()))); You should use jsListener->isolatedWorld() instead. WebCore/bindings/v8/ScriptEventListener.cpp:123 + ScriptState* scriptState = mainWorldScriptState(document->frame()); It should be possible to get it from the listener, something like static_cast<V8AbstractEventListener*>(listener)->worldContext() worldContext is protected though.
Created attachment 60908 [details] [patch] third iteration. ScriptEventListener's problem was fixed. null is used instead of empty InspectorObject.
Comment on attachment 60908 [details] [patch] third iteration. Sorry for the late review. Just minor comments, mostly about things that were in the surrounding code, that you might want to incorporate or think about in your next patch. > +++ b/WebCore/inspector/InspectorController.h > @@ -220,6 +220,7 @@ public: > InspectorFrontend* inspectorFrontend() { return m_frontend.get(); } > + InspectorFrontend2* inspectorFrontend2() { return m_frontend2.get(); } I think both of these can be made "const". > diff --git a/WebCore/inspector/InspectorDOMAgent.h b/WebCore/inspector/InspectorDOMAgent.h > #include "ScriptState.h" Is this one still needed? > @@ -57,7 +56,7 @@ namespace WebCore { > class ContainerNode; > ... > class NameNodeMap; > class Page; I think these can all be removed. I think while you are making some these changes it would be nice if headers and forward declarations like this could be quickly checked. It would be a great way to clean things up!
Comment on attachment 60908 [details] [patch] third iteration. Looks like there are still some comments to address.
I think this las landed long ago.