Here is IORPCDelegate code in WebDevToolsAgentImpl.cpp: class IORPCDelegate : public DevToolsRPC::Delegate, public Noncopyable { public: IORPCDelegate() { } virtual ~IORPCDelegate() { } virtual void sendRpcMessage(const WebDevToolsMessageData& data) { WebDevToolsAgentClient::sendMessageToFrontendOnIOThread(data); } }; WebDevToolsAgentClient::sendMessageToFrontendOnIOThread is implemented in chromium code base (devtools_agent.cc). This prevents us from building webkit as a dll. We should change sendMessageToFrontendOnIOThread to be a virtual function and access it through WebDevToolsAgentClient object.
Mikhail, could you look into this?
Yes, will do.
Hi Mikhail, Any update on this? I am splitting webkit to be a dll for chromium dev build, this issue needs to be fixed in order to move forward. Thanks! (In reply to comment #2) > Yes, will do.
Victor, I'm sorry for holding you! I already tried once to fix it, but discovered that it's not trivial, as the call path goes all the way through static methods (that's why in the current state of affairs it is natural for the method to be static). Then I was distracted, etc. I will try harder, I promise!
Created attachment 58812 [details] proposed patch Below is the corresponding Chromium change. After applying it, a cleanup change to WebKit will be required. Also, after applying the WebKit patch, Heap snapshots will stop working until Chromium change will be applied. I see no other good way of dealing with it, as sendMessageToFrontendOnIOThread is declared and used in WebKit, but defined in Chromium. Thus, to remove it, we first need to remove its usage (WebKit), then definition (Chromium), then declaration (WebKit again). diff --git a/chrome/renderer/devtools_agent.cc b/chrome/renderer/devtools_agent.cc index 5adfa09..42f32e2 100644 --- a/chrome/renderer/devtools_agent.cc +++ b/chrome/renderer/devtools_agent.cc @@ -57,12 +57,14 @@ DevToolsAgent::DevToolsAgent(int routing_id, RenderView* render_view) : routing_id_(routing_id), render_view_(render_view) { agent_for_routing_id_[routing_id] = this; + DevToolsAgentFilter::SetDevToolsAgent(this); CommandLine* cmd = CommandLine::ForCurrentProcess(); expose_v8_debugger_protocol_ =cmd->HasSwitch(switches::kRemoteShellPort); } DevToolsAgent::~DevToolsAgent() { + DevToolsAgentFilter::SetDevToolsAgent(NULL); agent_for_routing_id_.erase(routing_id_); } @@ -96,6 +98,13 @@ void DevToolsAgent::sendMessageToFrontend( render_view_->Send(m); } + +void DevToolsAgent::sendMessageToFrontendFromIOThread( + const WebKit::WebDevToolsMessageData& data) { + DevToolsAgentFilter::SendRpcMessage(DevToolsMessageData(data)); +} + + int DevToolsAgent::hostIdentifier() { return routing_id_; } @@ -195,9 +204,3 @@ WebDevToolsAgent* DevToolsAgent::GetWebAgent() { return NULL; return web_view->devToolsAgent(); } - -// static -void WebKit::WebDevToolsAgentClient::sendMessageToFrontendOnIOThread( - const WebDevToolsMessageData& data) { - DevToolsAgentFilter::SendRpcMessage(DevToolsMessageData(data)); -} diff --git a/chrome/renderer/devtools_agent.h b/chrome/renderer/devtools_agent.h index 23fa10c..f1262d0 100644 --- a/chrome/renderer/devtools_agent.h +++ b/chrome/renderer/devtools_agent.h @@ -40,6 +40,8 @@ class DevToolsAgent : public WebKit::WebDevToolsAgentClient { // WebDevToolsAgentClient implementation virtual void sendMessageToFrontend( const WebKit::WebDevToolsMessageData& data); + virtual void sendMessageToFrontendFromIOThread( + const WebKit::WebDevToolsMessageData& data); virtual int hostIdentifier(); virtual void forceRepaint(); diff --git a/chrome/renderer/devtools_agent_filter.cc b/chrome/renderer/devtools_agent_filter.cc index 5eb54ed..01c1be0 100644 --- a/chrome/renderer/devtools_agent_filter.cc +++ b/chrome/renderer/devtools_agent_filter.cc @@ -28,6 +28,8 @@ void DevToolsAgentFilter::DispatchMessageLoop() { } // static +DevToolsAgent* DevToolsAgentFilter::agent_ = NULL; +// static IPC::Channel* DevToolsAgentFilter::channel_ = NULL; // static int DevToolsAgentFilter::current_routing_id_ = 0; @@ -65,8 +67,13 @@ void DevToolsAgentFilter::OnDebuggerPauseScript() { } void DevToolsAgentFilter::OnRpcMessage(const DevToolsMessageData& data) { - message_handled_ = WebDevToolsAgent::dispatchMessageFromFrontendOnIOThread( - data.ToWebDevToolsMessageData()); + if (agent_ != NULL) { + message_handled_ = WebDevToolsAgent::dispatchMessageFromFrontendOnIOThread( + agent_, + data.ToWebDevToolsMessageData()); + } else { + message_handled_ = false; + } } // static diff --git a/chrome/renderer/devtools_agent_filter.h b/chrome/renderer/devtools_agent_filter.h index 8d23918..caae82b 100644 --- a/chrome/renderer/devtools_agent_filter.h +++ b/chrome/renderer/devtools_agent_filter.h @@ -10,6 +10,7 @@ #include "ipc/ipc_channel_proxy.h" +class DevToolsAgent; struct DevToolsMessageData; // DevToolsAgentFilter is registered as an IPC filter in order to be able to @@ -25,6 +26,7 @@ class DevToolsAgentFilter : public IPC::ChannelProxy::MessageFilter { virtual ~DevToolsAgentFilter(); static void SendRpcMessage(const DevToolsMessageData& data); + static void SetDevToolsAgent(DevToolsAgent* agent) { agent_ = agent; } private: // IPC::ChannelProxy::MessageFilter override. Called on IO thread. @@ -46,6 +48,7 @@ class DevToolsAgentFilter : public IPC::ChannelProxy::MessageFilter { // from IO thread. static int current_routing_id_; static IPC::Channel* channel_; + static DevToolsAgent* agent_; DISALLOW_COPY_AND_ASSIGN(DevToolsAgentFilter); };
Attachment 58812 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKit/chromium/src/WebDevToolsAgentImpl.cpp:126: Use 0 instead of NULL. [readability/null] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 58859 [details] fixed style error
Created attachment 58868 [details] as we discussed offline Chromium change is now smaller: diff --git a/chrome/renderer/devtools_agent.cc b/chrome/renderer/devtools_agent.cc index 5adfa09..01c66e4 100644 --- a/chrome/renderer/devtools_agent.cc +++ b/chrome/renderer/devtools_agent.cc @@ -195,9 +195,3 @@ WebDevToolsAgent* DevToolsAgent::GetWebAgent() { return NULL; return web_view->devToolsAgent(); } - -// static -void WebKit::WebDevToolsAgentClient::sendMessageToFrontendOnIOThread( - const WebDevToolsMessageData& data) { - DevToolsAgentFilter::SendRpcMessage(DevToolsMessageData(data)); -} diff --git a/chrome/renderer/devtools_agent_filter.cc b/chrome/renderer/devtools_agent_filter.cc index 5eb54ed..4c98cbe 100644 --- a/chrome/renderer/devtools_agent_filter.cc +++ b/chrome/renderer/devtools_agent_filter.cc @@ -64,8 +64,21 @@ void DevToolsAgentFilter::OnDebuggerPauseScript() { WebDevToolsAgent::debuggerPauseScript(); } +namespace { + +class WebDevToolsMessageTransportImpl : public WebKit::WebDevToolsMessageTransport { + public: + void sendMessageToFrontendOnIOThread(const WebKit::WebDevToolsMessageData& data) { + DevToolsAgentFilter::SendRpcMessage(DevToolsMessageData(data)); + } +}; + +} // namespace + void DevToolsAgentFilter::OnRpcMessage(const DevToolsMessageData& data) { + WebDevToolsMessageTransportImpl transport; message_handled_ = WebDevToolsAgent::dispatchMessageFromFrontendOnIOThread( + &transport, data.ToWebDevToolsMessageData()); }
Created attachment 58880 [details] will not break functionality, but requires 4 steps to commit Chromium part: diff --git a/chrome/renderer/devtools_agent_filter.cc b/chrome/renderer/devtools_agent_filter.cc index 5eb54ed..4c98cbe 100644 --- a/chrome/renderer/devtools_agent_filter.cc +++ b/chrome/renderer/devtools_agent_filter.cc @@ -64,8 +64,21 @@ void DevToolsAgentFilter::OnDebuggerPauseScript() { WebDevToolsAgent::debuggerPauseScript(); } +namespace { + +class WebDevToolsMessageTransportImpl : public WebKit::WebDevToolsMessageTransport { + public: + void sendMessageToFrontendOnIOThread(const WebKit::WebDevToolsMessageData& data) { + DevToolsAgentFilter::SendRpcMessage(DevToolsMessageData(data)); + } +}; + +} // namespace + void DevToolsAgentFilter::OnRpcMessage(const DevToolsMessageData& data) { + WebDevToolsMessageTransportImpl transport; message_handled_ = WebDevToolsAgent::dispatchMessageFromFrontendOnIOThread( + &transport, data.ToWebDevToolsMessageData()); }
Comment on attachment 58880 [details] will not break functionality, but requires 4 steps to commit WebKit/chromium/public/WebDevToolsAgentClient.h:74 + class WebDevToolsMessageTransport { one file per class please.
OK, will fix this in the next commit. I wanted to make this class an inner of WebDevToolsAgentClient, but forgot that C++ doesn't allow forward declarations for inner classes (except inside its containing class).
Comment on attachment 58880 [details] will not break functionality, but requires 4 steps to commit WebKit/chromium/public/WebDevToolsAgentClient.h:75 + public: style: wrong indentation
Fixed Yury's style comment. Manually committed http://trac.webkit.org/changeset/61320 2010-06-17 Mikhail Naganov <mnaganov@chromium.org> Reviewed by Pavel Feldman. [Chromium] Prepare to making WebDevToolsAgentClient::sendMessageToFrontendOnIOThread virtual https://bugs.webkit.org/show_bug.cgi?id=39302 * public/WebDevToolsAgent.h: * public/WebDevToolsAgentClient.h: (WebKit::WebDevToolsMessageTransport::~WebDevToolsMessageTransport): * src/WebDevToolsAgentImpl.cpp: (WebKit::): (WebKit::WebDevToolsAgent::dispatchMessageFromFrontendOnIOThread):