DevTools: process pause script request on the IO thread. To pause already running script we need to call v8::Debug::DebugBreak() on a thread different from the Render thread which may be busy with JS execution.

BUG=28892
TEST=DevToolsSanityTest.TestPauseWhenScriptIsRunning

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

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

18 files changed, 115 insertions, 41 deletions

diff --git a/chrome/browser/debugger/devtools_sanity_unittest.cc b/chrome/browser/debugger/devtools_sanity_unittest.cc
index 405d809..3c3b8d4 100644
--- a/chrome/browser/debugger/devtools_sanity_unittest.cc
+++ b/chrome/browser/debugger/devtools_sanity_unittest.cc
@@ -53,6 +53,8 @@ const wchar_t kPauseOnExceptionTestPage[] =
     L"files/devtools/pause_on_exception.html";
 const wchar_t kPauseWhenLoadingDevTools[] =
     L"files/devtools/pause_when_loading_devtools.html";
+const wchar_t kPauseWhenScriptIsRunning[] =
+    L"files/devtools/pause_when_script_is_running.html";
 const wchar_t kResourceContentLengthTestPage[] = L"files/devtools/image.html";
 const wchar_t kResourceTestPage[] = L"files/devtools/resource_test_page.html";
 const wchar_t kSimplePage[] = L"files/devtools/simple_page.html";
@@ -344,6 +346,12 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestPauseWhenLoadingDevTools) {
   RunTest("testPauseWhenLoadingDevTools", kPauseWhenLoadingDevTools);
 }
 
+// Tests that pressing 'Pause' will pause script execution if the script
+// is already running.
+IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestPauseWhenScriptIsRunning) {
+  RunTest("testPauseWhenScriptIsRunning", kPauseWhenScriptIsRunning);
+}
+
 // Tests eval on call frame.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestEvalOnCallFrame) {
   RunTest("testEvalOnCallFrame", kDebuggerTestPage);
diff --git a/chrome/common/devtools_messages_internal.h b/chrome/common/devtools_messages_internal.h
index be187a7..d80c0ed 100644
--- a/chrome/common/devtools_messages_internal.h
+++ b/chrome/common/devtools_messages_internal.h
@@ -86,6 +86,12 @@ IPC_BEGIN_MESSAGES(DevToolsAgent)
   IPC_MESSAGE_CONTROL1(DevToolsAgentMsg_DebuggerCommand,
                        std::string  /* command */)
 
+  // This command is sent to debugger when user wants to pause script execution
+  // immediately. This message should be processed on the IO thread so that it
+  // can have effect even if the Renderer thread is busy with JavaScript
+  // execution.
+  IPC_MESSAGE_CONTROL0(DevToolsAgentMsg_DebuggerPauseScript)
+
   // Inspect element with the given coordinates.
   IPC_MESSAGE_CONTROL2(DevToolsAgentMsg_InspectElement,
                        int /* x */,
diff --git a/chrome/renderer/devtools_agent_filter.cc b/chrome/renderer/devtools_agent_filter.cc
index f67f183..93b6c75 100644
--- a/chrome/renderer/devtools_agent_filter.cc
+++ b/chrome/renderer/devtools_agent_filter.cc
@@ -34,21 +34,23 @@ DevToolsAgentFilter::~DevToolsAgentFilter() {
 }
 
 bool DevToolsAgentFilter::OnMessageReceived(const IPC::Message& message) {
-  if (message.type() == DevToolsAgentMsg_DebuggerCommand::ID) {
-    // Dispatch command directly from IO.
-    bool handled = true;
-    current_routing_id_ = message.routing_id();
-    IPC_BEGIN_MESSAGE_MAP(DevToolsAgentFilter, message)
-      IPC_MESSAGE_HANDLER(DevToolsAgentMsg_DebuggerCommand, OnDebuggerCommand)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_END_MESSAGE_MAP()
-    return handled;
-  } else {
-    return false;
-  }
+  // Dispatch debugger commands directly from IO.
+  bool handled = true;
+  current_routing_id_ = message.routing_id();
+  IPC_BEGIN_MESSAGE_MAP(DevToolsAgentFilter, message)
+    IPC_MESSAGE_HANDLER(DevToolsAgentMsg_DebuggerCommand, OnDebuggerCommand)
+    IPC_MESSAGE_HANDLER(DevToolsAgentMsg_DebuggerPauseScript,
+                        OnDebuggerPauseScript)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
 }
 
 void DevToolsAgentFilter::OnDebuggerCommand(const std::string& command) {
   WebDevToolsAgent::executeDebuggerCommand(
       WebString::fromUTF8(command), current_routing_id_);
 }
+
+void DevToolsAgentFilter::OnDebuggerPauseScript() {
+  WebDevToolsAgent::debuggerPauseScript();
+}
diff --git a/chrome/renderer/devtools_agent_filter.h b/chrome/renderer/devtools_agent_filter.h
index 9ee6296..4bf4460 100644
--- a/chrome/renderer/devtools_agent_filter.h
+++ b/chrome/renderer/devtools_agent_filter.h
@@ -31,6 +31,7 @@ class DevToolsAgentFilter : public IPC::ChannelProxy::MessageFilter {
   // 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);
+  void OnDebuggerPauseScript();
 
   int current_routing_id_;
 
diff --git a/chrome/renderer/devtools_client.cc b/chrome/renderer/devtools_client.cc
index 9b17c8b..8083288 100644
--- a/chrome/renderer/devtools_client.cc
+++ b/chrome/renderer/devtools_client.cc
@@ -64,6 +64,10 @@ void DevToolsClient::sendDebuggerCommandToAgent(const WebString& command) {
   Send(DevToolsAgentMsg_DebuggerCommand(command.utf8()));
 }
 
+void DevToolsClient::sendDebuggerPauseScript() {
+  Send(DevToolsAgentMsg_DebuggerPauseScript());
+}
+
 void DevToolsClient::activateWindow() {
   render_view_->Send(new ViewHostMsg_ActivateDevToolsWindow(
       render_view_->routing_id()));
diff --git a/chrome/renderer/devtools_client.h b/chrome/renderer/devtools_client.h
index e83ec12..aad5c09 100644
--- a/chrome/renderer/devtools_client.h
+++ b/chrome/renderer/devtools_client.h
@@ -43,6 +43,7 @@ class DevToolsClient : public WebKit::WebDevToolsFrontendClient {
                                   const WebKit::WebString& param2,
                                   const WebKit::WebString& param3);
   virtual void sendDebuggerCommandToAgent(const WebKit::WebString& command);
+  virtual void sendDebuggerPauseScript();
 
   virtual void activateWindow();
   virtual void closeWindow();
diff --git a/chrome/test/data/devtools/pause_when_script_is_running.html b/chrome/test/data/devtools/pause_when_script_is_running.html
new file mode 100644
index 0000000..380d8e1
--- /dev/null
+++ b/chrome/test/data/devtools/pause_when_script_is_running.html
@@ -0,0 +1,13 @@
+<html>
+<head>
+<script>
+function handleClick() {
+  while(true) {
+  }
+}
+</script>
+</head>
+<body>
+<input type='button' onclick='handleClick()' value='Test'/>
+</body>
+</html>
\ No newline at end of file
diff --git a/webkit/glue/devtools/debugger_agent.h b/webkit/glue/devtools/debugger_agent.h
index 356837c..9549447 100644
--- a/webkit/glue/devtools/debugger_agent.h
+++ b/webkit/glue/devtools/debugger_agent.h
@@ -8,9 +8,6 @@
 #include "webkit/glue/devtools/devtools_rpc.h"
 
 #define DEBUGGER_AGENT_STRUCT(METHOD0, METHOD1, METHOD2, METHOD3) \
-  /* Stops v8 execution as soon as it gets control. */ \
-  METHOD0(DebugBreak) \
-  \
   /* Requests global context id of the inspected tab. */ \
   METHOD0(GetContextId) \
   \
diff --git a/webkit/glue/devtools/debugger_agent_impl.cc b/webkit/glue/devtools/debugger_agent_impl.cc
index 8957cb3..a354cde 100644
--- a/webkit/glue/devtools/debugger_agent_impl.cc
+++ b/webkit/glue/devtools/debugger_agent_impl.cc
@@ -53,10 +53,6 @@ DebuggerAgentImpl::~DebuggerAgentImpl() {
   DebuggerAgentManager::DebugDetach(this);
 }
 
-void DebuggerAgentImpl::DebugBreak() {
-  DebuggerAgentManager::DebugBreak(this);
-}
-
 void DebuggerAgentImpl::GetContextId() {
   delegate_->SetContextId(webdevtools_agent_->host_id());
 }
diff --git a/webkit/glue/devtools/debugger_agent_impl.h b/webkit/glue/devtools/debugger_agent_impl.h
index 412e898..96400f8 100644
--- a/webkit/glue/devtools/debugger_agent_impl.h
+++ b/webkit/glue/devtools/debugger_agent_impl.h
@@ -36,7 +36,6 @@ class DebuggerAgentImpl : public DebuggerAgent {
   virtual ~DebuggerAgentImpl();
 
   // DebuggerAgent implementation.
-  virtual void DebugBreak();
   virtual void GetContextId();
 
   virtual void StartProfiling(int flags);
diff --git a/webkit/glue/devtools/debugger_agent_manager.cc b/webkit/glue/devtools/debugger_agent_manager.cc
index 1adb171..a093608 100644
--- a/webkit/glue/devtools/debugger_agent_manager.cc
+++ b/webkit/glue/devtools/debugger_agent_manager.cc
@@ -163,19 +163,6 @@ void DebuggerAgentManager::DebugDetach(DebuggerAgentImpl* debugger_agent) {
 }
 
 // static
-void DebuggerAgentManager::DebugBreak(DebuggerAgentImpl* debugger_agent) {
-#if USE(V8)
-  ASSERT(DebuggerAgentForHostId(debugger_agent->webdevtools_agent()->host_id())
-             == debugger_agent);
-  if (in_utility_context_) {
-    debug_break_delayed_ = true;
-  } else {
-    v8::Debug::DebugBreak();
-  }
-#endif
-}
-
-// static
 void DebuggerAgentManager::OnV8DebugMessage(const v8::Debug::Message& message) {
   v8::HandleScope scope;
   v8::String::Value value(message.GetJSON());
@@ -248,6 +235,15 @@ void DebuggerAgentManager::OnV8DebugMessage(const v8::Debug::Message& message) {
 }
 
 // static
+void DebuggerAgentManager::PauseScript() {
+  if (in_utility_context_) {
+    debug_break_delayed_ = true;
+  } else {
+    v8::Debug::DebugBreak();
+  }
+}
+
+// static
 void DebuggerAgentManager::ExecuteDebuggerCommand(
     const String& command,
     int caller_id) {
diff --git a/webkit/glue/devtools/debugger_agent_manager.h b/webkit/glue/devtools/debugger_agent_manager.h
index f3c9c4f..0d1c0d9 100644
--- a/webkit/glue/devtools/debugger_agent_manager.h
+++ b/webkit/glue/devtools/debugger_agent_manager.h
@@ -39,9 +39,7 @@ class DebuggerAgentManager : public Noncopyable {
  public:
   static void DebugAttach(DebuggerAgentImpl* debugger_agent);
   static void DebugDetach(DebuggerAgentImpl* debugger_agent);
-  static void DebugBreak(DebuggerAgentImpl* debugger_agent);
-  static void DebugCommand(const WebCore::String& command);
-
+  static void PauseScript();
   static void ExecuteDebuggerCommand(const WebCore::String& command,
                                      int caller_id);
   static void SetMessageLoopDispatchHandler(
diff --git a/webkit/glue/devtools/js/debugger_agent.js b/webkit/glue/devtools/js/debugger_agent.js
index 1aa91a8..86ca977 100644
--- a/webkit/glue/devtools/js/debugger_agent.js
+++ b/webkit/glue/devtools/js/debugger_agent.js
@@ -247,7 +247,7 @@ devtools.DebuggerAgent.prototype.resolveScriptSource = function(
  * Tells the v8 debugger to stop on as soon as possible.
  */
 devtools.DebuggerAgent.prototype.pauseExecution = function() {
-  RemoteDebuggerAgent.DebugBreak();
+  RemoteDebuggerCommandExecutor.DebuggerPauseScript();
 };
 
 
diff --git a/webkit/glue/devtools/js/devtools_host_stub.js b/webkit/glue/devtools/js/devtools_host_stub.js
index 9a90d04..c2bd4d3 100644
--- a/webkit/glue/devtools/js/devtools_host_stub.js
+++ b/webkit/glue/devtools/js/devtools_host_stub.js
@@ -19,10 +19,6 @@ RemoteDebuggerAgentStub = function() {
 };
 
 
-RemoteDebuggerAgentStub.prototype.DebugBreak = function() {
-};
-
-
 RemoteDebuggerAgentStub.prototype.GetContextId = function() {
   RemoteDebuggerAgent.SetContextId(3);
 };
@@ -199,6 +195,10 @@ RemoteDebuggerCommandExecutorStub.prototype.DebuggerCommand = function(cmd) {
 };
 
 
+RemoteDebuggerCommandExecutorStub.prototype.DebuggerPauseScript = function() {
+};
+
+
 RemoteDebuggerCommandExecutorStub.prototype.sendResponse_ = function(response) {
   setTimeout(function() {
     RemoteDebuggerAgent.DebuggerOutput(response);
diff --git a/webkit/glue/devtools/js/tests.js b/webkit/glue/devtools/js/tests.js
index 55a46e1..c5ca9b6 100644
--- a/webkit/glue/devtools/js/tests.js
+++ b/webkit/glue/devtools/js/tests.js
@@ -719,6 +719,40 @@ TestSuite.prototype.testPauseWhenLoadingDevTools = function() {
 };
 
 
+// Tests that pressing 'Pause' will pause script execution if the script
+// is already running.
+TestSuite.prototype.testPauseWhenScriptIsRunning = function() {
+  this.showPanel('scripts');
+  var test = this;
+
+  test.evaluateInConsole_(
+      'setTimeout("handleClick()" , 0)',
+      function(resultText) {
+        test.assertTrue(!isNaN(resultText),
+                        'Failed to get timer id: ' + resultText);
+        testScriptPause();
+      });
+
+  function testScriptPause() {
+    // The script should be in infinite loop. Click 'Pause' button to
+    // pause it and wait for the result.
+    WebInspector.panels.scripts.pauseButton.click();
+
+    test._waitForScriptPause(
+        {
+          functionsOnStack: ['handleClick', '(anonymous function)'],
+          lineNumber: 5,
+          lineText: '  while(true) {'
+        },
+        function() {
+          test.releaseControl();
+        });
+  }
+
+  this.takeControl();
+};
+
+
 /**
  * Serializes options collection to string.
  * @param {HTMLOptionsCollection} options
diff --git a/webkit/glue/webdevtoolsagent_impl.cc b/webkit/glue/webdevtoolsagent_impl.cc
index e621058..2335a2b 100644
--- a/webkit/glue/webdevtoolsagent_impl.cc
+++ b/webkit/glue/webdevtoolsagent_impl.cc
@@ -526,6 +526,11 @@ void WebDevToolsAgent::executeDebuggerCommand(
 }
 
 // static
+void WebDevToolsAgent::debuggerPauseScript() {
+  DebuggerAgentManager::PauseScript();
+}
+
+// static
 void WebDevToolsAgent::setMessageLoopDispatchHandler(
     MessageLoopDispatchHandler handler) {
   DebuggerAgentManager::SetMessageLoopDispatchHandler(handler);
diff --git a/webkit/glue/webdevtoolsfrontend_impl.cc b/webkit/glue/webdevtoolsfrontend_impl.cc
index cfdd896b..e0d5fa2 100644
--- a/webkit/glue/webdevtoolsfrontend_impl.cc
+++ b/webkit/glue/webdevtoolsfrontend_impl.cc
@@ -142,6 +142,9 @@ WebDevToolsFrontendImpl::WebDevToolsFrontendImpl(
   debugger_command_executor_obj_->AddProtoFunction(
       "DebuggerCommand",
       WebDevToolsFrontendImpl::JsDebuggerCommand);
+  debugger_command_executor_obj_->AddProtoFunction(
+      "DebuggerPauseScript",
+      WebDevToolsFrontendImpl::JsDebuggerPauseScript);
   debugger_command_executor_obj_->Build();
 
   dev_tools_host_.set(new BoundObject(frame_context, this, "DevToolsHost"));
@@ -409,3 +412,12 @@ v8::Handle<v8::Value> WebDevToolsFrontendImpl::JsDebuggerCommand(
   frontend->client_->sendDebuggerCommandToAgent(std_command);
   return v8::Undefined();
 }
+
+// static
+v8::Handle<v8::Value> WebDevToolsFrontendImpl::JsDebuggerPauseScript(
+    const v8::Arguments& args) {
+  WebDevToolsFrontendImpl* frontend = static_cast<WebDevToolsFrontendImpl*>(
+      v8::External::Cast(*args.Data())->Value());
+  frontend->client_->sendDebuggerPauseScript();
+  return v8::Undefined();
+}
diff --git a/webkit/glue/webdevtoolsfrontend_impl.h b/webkit/glue/webdevtoolsfrontend_impl.h
index 877e4b0..d6ebfe6 100644
--- a/webkit/glue/webdevtoolsfrontend_impl.h
+++ b/webkit/glue/webdevtoolsfrontend_impl.h
@@ -80,6 +80,8 @@ class WebDevToolsFrontendImpl : public WebKit::WebDevToolsFrontend,
       const v8::Arguments& args);
   static v8::Handle<v8::Value> JsDebuggerCommand(
       const v8::Arguments& args);
+  static v8::Handle<v8::Value> JsDebuggerPauseScript(
+      const v8::Arguments& args);
 
   WebKit::WebViewImpl* web_view_impl_;
   WebKit::WebDevToolsFrontendClient* client_;