DevTools: Split message handling on the agent side into the Renderer and IO handling classes (currently mixed).

All the messages except for the DebuggerCommand should go be dispatched on the Renderer thread by the DevToolsAgent.
DebuggerCommand should be handled by the DevToolsAgentFilter separately (for v8 suspend support).

Review URL: http://codereview.chromium.org/62037

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13153 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 113 insertions, 150 deletions

diff --git a/chrome/renderer/devtools_agent.cc b/chrome/renderer/devtools_agent.cc
index e46217f..633203c 100644
--- a/chrome/renderer/devtools_agent.cc
+++ b/chrome/renderer/devtools_agent.cc
@@ -2,152 +2,63 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// DevToolsAgent belongs to the inspectable renderer and provides Glue's
-// agents with the communication capabilities. All messages from/to Glue's
-// agents infrastructure are flowing through this comminucation agent.
-//
-// DevToolsAgent is registered as an IPC filter in order to be able to
-// dispatch messages while on the IO thread. The reason for that is that while
-// debugging, Render thread is being held by the v8 and hence no messages
-// are being dispatched there. While holding the thread in a tight loop,
-// v8 provides thread-safe Api for controlling debugger. In our case v8's Api
-// is being used from this communication agent on the IO thread.
-
 #include "chrome/renderer/devtools_agent.h"
 
-#include "base/message_loop.h"
 #include "chrome/common/devtools_messages.h"
 #include "chrome/common/render_messages.h"
-// TODO(yurys): remove this macros once plugins available on other platforms
-#if defined(OS_WIN)
-#include "chrome/renderer/plugin_channel_host.h"
-#endif  // OS_WIN
-#include "chrome/renderer/render_process.h"
 #include "chrome/renderer/render_view.h"
 #include "webkit/glue/webdevtoolsagent.h"
 
-DevToolsAgent::DevToolsAgent(int routing_id,
-                             RenderView* view,
-                             MessageLoop* view_loop)
+DevToolsAgent::DevToolsAgent(int routing_id, RenderView* view)
     : routing_id_(routing_id),
-      view_(view),
-      view_loop_(view_loop),
-      io_loop_(NULL) {
+      view_(view) {
 }
 
 DevToolsAgent::~DevToolsAgent() {
 }
 
-// Called on render thread.
-void DevToolsAgent::RenderViewDestroyed() {
-  DCHECK(MessageLoop::current() == view_loop_);
-  view_ = NULL;
-}
-
-void DevToolsAgent::Send(const IPC::Message& tools_client_message) {
-  DCHECK(MessageLoop::current() == view_loop_);
-  if (!view_) {
-    return;
-  }
-
-  IPC::Message* m = new ViewHostMsg_ForwardToDevToolsClient(
-      routing_id_,
-      tools_client_message);
-  view_->Send(m);
-}
-
-// Called on the IO thread.
-void DevToolsAgent::OnFilterAdded(IPC::Channel* channel) {
-  io_loop_ = MessageLoop::current();
-}
-
-// Called on the IO thread.
+// Called on the Renderer thread.
 bool DevToolsAgent::OnMessageReceived(const IPC::Message& message) {
-  DCHECK(MessageLoop::current() == io_loop_);
-
-  if (message.routing_id() != routing_id_)
-    return false;
-
-  // TODO(yurys): only DebuggerCommand message is handled on the IO thread
-  // all other messages could be handled in RenderView::OnMessageReceived. With
-  // that approach we wouldn't have to call view_loop_->PostTask for each of the
-  // messages.
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(DevToolsAgent, message)
     IPC_MESSAGE_HANDLER(DevToolsAgentMsg_Attach, OnAttach)
     IPC_MESSAGE_HANDLER(DevToolsAgentMsg_Detach, OnDetach)
     IPC_MESSAGE_HANDLER(DevToolsAgentMsg_RpcMessage, OnRpcMessage)
-    IPC_MESSAGE_HANDLER(DevToolsAgentMsg_DebuggerCommand,
-                        OnDebuggerCommand)
     IPC_MESSAGE_HANDLER(DevToolsAgentMsg_InspectElement, OnInspectElement)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
-// Called on the IO thread.
-void DevToolsAgent::OnFilterRemoved() {
-  io_loop_ = NULL;
-}
-
-void DevToolsAgent::EvaluateScript(const std::wstring& script) {
-  DCHECK(MessageLoop::current() == view_loop_);
-  // view_ may have been cleared after this method execution was scheduled.
-  if (view_)
-    view_->EvaluateScript(L"", script);
-}
-
-void DevToolsAgent::OnAttach() {
-  view_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &DevToolsAgent::Attach));
-}
-
-void DevToolsAgent::OnDetach() {
-  view_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &DevToolsAgent::Detach));
-}
-
 void DevToolsAgent::SendMessageToClient(const std::string& raw_msg) {
-  Send(DevToolsClientMsg_RpcMessage(raw_msg));
-}
-
-void DevToolsAgent::OnRpcMessage(const std::string& raw_msg) {
-  view_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &DevToolsAgent::DispatchRpcMessage, raw_msg));
-}
-
-void DevToolsAgent::OnInspectElement(int x, int y) {
-  view_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &DevToolsAgent::InspectElement, x, y));
+  IPC::Message* m = new ViewHostMsg_ForwardToDevToolsClient(
+      routing_id_,
+      DevToolsClientMsg_RpcMessage(raw_msg));
+  view_->Send(m);
 }
 
-void DevToolsAgent::Attach() {
+void DevToolsAgent::OnAttach() {
   WebDevToolsAgent* web_agent = GetWebAgent();
   if (web_agent) {
     web_agent->Attach();
   }
 }
 
-void DevToolsAgent::Detach() {
+void DevToolsAgent::OnDetach() {
   WebDevToolsAgent* web_agent = GetWebAgent();
   if (web_agent) {
     web_agent->Detach();
   }
 }
 
-void DevToolsAgent::DispatchRpcMessage(const std::string& raw_msg) {
+void DevToolsAgent::OnRpcMessage(const std::string& raw_msg) {
   WebDevToolsAgent* web_agent = GetWebAgent();
   if (web_agent) {
     web_agent->DispatchMessageFromClient(raw_msg);
   }
 }
 
-void DevToolsAgent::OnDebuggerCommand(const std::string& command) {
-  // Debugger commands are handled on the IO thread.
-  WebDevToolsAgent::ExecuteDebuggerCommand(command);
-}
-
-void DevToolsAgent::InspectElement(int x, int y) {
+void DevToolsAgent::OnInspectElement(int x, int y) {
   WebDevToolsAgent* web_agent = GetWebAgent();
   if (web_agent) {
     web_agent->InspectElement(x, y);
@@ -155,8 +66,6 @@ void DevToolsAgent::InspectElement(int x, int y) {
 }
 
 WebDevToolsAgent* DevToolsAgent::GetWebAgent() {
-  if (!view_)
-    return NULL;
   WebView* web_view = view_->webview();
   if (!web_view)
     return NULL;
diff --git a/chrome/renderer/devtools_agent.h b/chrome/renderer/devtools_agent.h
index 29352a0..7d3c4a1 100644
--- a/chrome/renderer/devtools_agent.h
+++ b/chrome/renderer/devtools_agent.h
@@ -7,66 +7,40 @@
 
 #include <string>
 
-#include "base/basictypes.h"
-#include "base/ref_counted.h"
-#include "base/scoped_ptr.h"
-#include "chrome/common/ipc_channel_proxy.h"
 #include "webkit/glue/webdevtoolsagent_delegate.h"
 
-class MessageLoop;
+namespace IPC {
+class Message;
+}
+
 class RenderView;
 class WebDevToolsAgent;
 
-// Inspected page end of communication channel between the render process of
-// the page being inspected and tools UI renderer process. All messages will
-// go through browser process. On the renderer side of the tools UI there's
-// a corresponding ToolsClient object.
-class DevToolsAgent : public IPC::ChannelProxy::MessageFilter,
-                      public WebDevToolsAgentDelegate {
+// DevToolsAgent belongs to the inspectable RenderView and provides Glue's
+// agents with the communication capabilities. All messages from/to Glue's
+// agents infrastructure are flowing through this comminucation agent.
+// There is a corresponding DevToolsClient object on the client side.
+class DevToolsAgent : public WebDevToolsAgentDelegate {
  public:
-  // DevToolsAgent is a field of the RenderView. The view is supposed to remove
-  // this agent from message filter list on IO thread before dying.
-  DevToolsAgent(int routing_id, RenderView* view, MessageLoop* view_loop);
+  DevToolsAgent(int routing_id, RenderView* view);
   virtual ~DevToolsAgent();
 
+  // IPC message interceptor. Called on the Render thread.
+  virtual bool OnMessageReceived(const IPC::Message& message);
+
   // WebDevToolsAgentDelegate implementation
   virtual void SendMessageToClient(const std::string& raw_msg);
 
-  // DevToolsAgent is created by RenderView which is supposed to call this
-  // method from its destructor.
-  void RenderViewDestroyed();
-
  private:
-  // Sends message to DevToolsClient. Should be called on the render thread.
-  void Send(const IPC::Message& tools_client_message);
-
-  // IPC::ChannelProxy::MessageFilter overrides. Called on IO thread.
-  virtual void OnFilterAdded(IPC::Channel* channel);
-  virtual bool OnMessageReceived(const IPC::Message& message);
-  virtual void OnFilterRemoved();
-
-  void Attach();
-  void Detach();
-  void DispatchRpcMessage(const std::string& raw_msg);
-  void InspectElement(int x, int y);
   WebDevToolsAgent* GetWebAgent();
 
-  // Evaluate javascript URL in the renderer
-  void EvaluateScript(const std::wstring& script);
-
-  // All these OnXXX methods will be executed in IO thread so that we can
-  // handle debug messages even when v8 is stopped.
   void OnAttach();
   void OnDetach();
   void OnRpcMessage(const std::string& raw_msg);
-  void OnDebuggerCommand(const std::string& command);
   void OnInspectElement(int x, int y);
 
   int routing_id_; //  View routing id that we can access from IO thread.
   RenderView* view_;
-  MessageLoop* view_loop_;
-
-  MessageLoop* io_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(DevToolsAgent);
 };
diff --git a/chrome/renderer/devtools_agent_filter.cc b/chrome/renderer/devtools_agent_filter.cc
new file mode 100644
index 0000000..319746f
--- /dev/null
+++ b/chrome/renderer/devtools_agent_filter.cc
@@ -0,0 +1,28 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/renderer/devtools_agent_filter.h"
+
+#include "chrome/common/devtools_messages.h"
+#include "webkit/glue/webdevtoolsagent.h"
+
+DevToolsAgentFilter::DevToolsAgentFilter() {
+}
+
+DevToolsAgentFilter::~DevToolsAgentFilter() {
+}
+
+bool DevToolsAgentFilter::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(DevToolsAgentFilter, message)
+    IPC_MESSAGE_HANDLER(DevToolsAgentMsg_DebuggerCommand,
+                        OnDebuggerCommand)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void DevToolsAgentFilter::OnDebuggerCommand(const std::string& command) {
+  WebDevToolsAgent::ExecuteDebuggerCommand(command);
+}
diff --git a/chrome/renderer/devtools_agent_filter.h b/chrome/renderer/devtools_agent_filter.h
new file mode 100644
index 0000000..edfe221
--- /dev/null
+++ b/chrome/renderer/devtools_agent_filter.h
@@ -0,0 +1,39 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_RENDERER_DEVTOOLS_AGENT_FILTER_H_
+#define CHROME_RENDERER_DEVTOOLS_AGENT_FILTER_H_
+
+#include <string>
+
+#include "chrome/common/ipc_channel_proxy.h"
+
+// DevToolsAgentFilter is registered as an IPC filter in order to be able to
+// dispatch messages while on the IO thread. The reason for that is that while
+// debugging, Render thread is being held by the v8 and hence no messages
+// are being dispatched there. While holding the thread in a tight loop,
+// v8 provides thread-safe Api for controlling debugger. In our case v8's Api
+// is being used from this communication agent on the IO thread.
+class DevToolsAgentFilter : public IPC::ChannelProxy::MessageFilter {
+ public:
+  // DevToolsAgentFilter is a field of the RenderView. The view is supposed
+  // to remove this agent from the message filter list on IO thread before
+  // dying.
+  DevToolsAgentFilter();
+  virtual ~DevToolsAgentFilter();
+
+ private:
+  // IPC::ChannelProxy::MessageFilter override. Called on IO thread.
+  virtual bool OnMessageReceived(const IPC::Message& message);
+
+  // OnDebuggerCommand will be executed in the IO thread so that we can
+  // handle debug messages even when v8 is stopped.
+  void OnDebuggerCommand(const std::string& command);
+
+  int routing_id_;  // View routing id that we can access from IO thread.
+
+  DISALLOW_COPY_AND_ASSIGN(DevToolsAgentFilter);
+};
+
+#endif  // CHROME_RENDERER_DEVTOOLS_AGENT_FILTER_H_
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index ee218a3..5b6b38d 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -30,6 +30,7 @@
 #include "chrome/renderer/about_handler.h"
 #include "chrome/renderer/debug_message_handler.h"
 #include "chrome/renderer/devtools_agent.h"
+#include "chrome/renderer/devtools_agent_filter.h"
 #include "chrome/renderer/devtools_client.h"
 #include "chrome/renderer/extensions/extension_process_bindings.h"
 #include "chrome/renderer/localized_error.h"
@@ -181,6 +182,7 @@ RenderView::RenderView(RenderThreadBase* render_thread)
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       first_default_plugin_(NULL),
       devtools_agent_(NULL),
+      devtools_agent_filter_(NULL),
       devtools_client_(NULL),
       history_back_list_count_(0),
       history_forward_list_count_(0),
@@ -207,9 +209,7 @@ RenderView::~RenderView() {
   }
 
   render_thread_->RemoveFilter(debug_message_handler_);
-
-  devtools_agent_->RenderViewDestroyed();
-  render_thread_->RemoveFilter(devtools_agent_);
+  render_thread_->RemoveFilter(devtools_agent_filter_);
 
 #ifdef CHROME_PERSONALIZATION
   Personalization::CleanupRendererPersonalization(personalization_);
@@ -296,8 +296,8 @@ void RenderView::Init(gfx::NativeViewId parent_hwnd,
     decrement_shared_popup_at_destruction_ = false;
   }
 
-  devtools_agent_ = new DevToolsAgent(routing_id, this,
-      MessageLoop::current());
+  devtools_agent_.reset(new DevToolsAgent(routing_id, this));
+  devtools_agent_filter_ = new DevToolsAgentFilter();
   webwidget_ = WebView::Create(this, webkit_prefs);
 
 #if defined(OS_LINUX)
@@ -335,8 +335,7 @@ void RenderView::Init(gfx::NativeViewId parent_hwnd,
 
   debug_message_handler_ = new DebugMessageHandler(this);
   render_thread_->AddFilter(debug_message_handler_);
-
-  render_thread_->AddFilter(devtools_agent_);
+  render_thread_->AddFilter(devtools_agent_filter_);
 }
 
 void RenderView::OnMessageReceived(const IPC::Message& message) {
@@ -347,6 +346,8 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
   // If this is developer tools renderer intercept tools messages first.
   if (devtools_client_.get() && devtools_client_->OnMessageReceived(message))
     return;
+  if (devtools_agent_->OnMessageReceived(message))
+    return;
 
   IPC_BEGIN_MESSAGE_MAP(RenderView, message)
     IPC_MESSAGE_HANDLER(ViewMsg_CaptureThumbnail, SendThumbnail)
@@ -2477,7 +2478,7 @@ void RenderView::DownloadUrl(const GURL& url, const GURL& referrer) {
 }
 
 WebDevToolsAgentDelegate* RenderView::GetWebDevToolsAgentDelegate() {
-  return devtools_agent_;
+  return devtools_agent_.get();
 }
 
 void RenderView::PasteFromSelectionClipboard() {
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index b0568b1..3e9a56c 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -45,6 +45,7 @@ class AudioRendererImpl;
 class DictionaryValue;
 class DebugMessageHandler;
 class DevToolsAgent;
+class DevToolsAgentFilter;
 class DevToolsClient;
 class FilePath;
 class GURL;
@@ -756,7 +757,10 @@ class RenderView : public RenderWidget,
   scoped_refptr<DebugMessageHandler> debug_message_handler_;
 
   // Provides access to this renderer from the remote Inspector UI.
-  scoped_refptr<DevToolsAgent> devtools_agent_;
+  scoped_ptr<DevToolsAgent> devtools_agent_;
+
+  // Handles messages on the IO thread while Renderer is on breakpoint.
+  scoped_refptr<DevToolsAgentFilter> devtools_agent_filter_;
 
   // DevToolsClient for renderer hosting developer tools UI. It's NULL for other
   // render views.
diff --git a/chrome/renderer/renderer.vcproj b/chrome/renderer/renderer.vcproj
index 3b85de0..e5e1092 100644
--- a/chrome/renderer/renderer.vcproj
+++ b/chrome/renderer/renderer.vcproj
@@ -234,6 +234,14 @@
 			>
 		</File>
 		<File
+			RelativePath=".\devtools_agent_filter.cc"
+			>
+		</File>
+		<File
+			RelativePath=".\devtools_agent_filter.h"
+			>
+		</File>
+		<File
 			RelativePath=".\devtools_client.cc"
 			>
 		</File>