DevTools: split console evaluation into two steps: actual evaluation and result

wrapping. When second step is executed debugger_agent_manager will autocontinue
on break. We assume that second step doesn't call user scripts.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21635
Review URL: http://codereview.chromium.org/159395

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

12 files changed, 184 insertions, 30 deletions

diff --git a/chrome/browser/debugger/devtools_sanity_unittest.cc b/chrome/browser/debugger/devtools_sanity_unittest.cc
index 55fa3f4..b79a5ae 100644
--- a/chrome/browser/debugger/devtools_sanity_unittest.cc
+++ b/chrome/browser/debugger/devtools_sanity_unittest.cc
@@ -161,6 +161,11 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestSetBreakpoint) {
   RunTest("testSetBreakpoint", kDebuggerTestPage);
 }
 
+// Tests that 'Pause' button works for eval.
+IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, DISABLED_TestPauseInEval) {
+  RunTest("testPauseInEval", kDebuggerTestPage);
+}
+
 // Tests console eval.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestConsoleEval) {
   RunTest("testConsoleEval", kConsoleTestPage);
diff --git a/webkit/glue/devtools/debugger_agent_impl.cc b/webkit/glue/devtools/debugger_agent_impl.cc
index 312ac4c..6802d3c 100644
--- a/webkit/glue/devtools/debugger_agent_impl.cc
+++ b/webkit/glue/devtools/debugger_agent_impl.cc
@@ -158,6 +158,9 @@ String DebuggerAgentImpl::ExecuteUtilityFunction(
     return "";
   }
   v8::Context::Scope context_scope(context);
+
+  DebuggerAgentManager::UtilityContextScope utility_scope;
+
   v8::Handle<v8::Function> function = v8::Local<v8::Function>::Cast(
       context->Global()->Get(v8::String::New("devtools$$dispatch")));
 
@@ -180,6 +183,62 @@ String DebuggerAgentImpl::ExecuteUtilityFunction(
   }
 }
 
+String DebuggerAgentImpl::EvaluateJavaScript(
+    v8::Handle<v8::Context> utility_context,
+    const String& source,
+    String* exception) {
+  v8::HandleScope scope;
+  ASSERT(!utility_context.IsEmpty());
+  if (utility_context.IsEmpty()) {
+    *exception = "No window utility context.";
+    return "";
+  }
+
+  v8::Handle<v8::Value> res_obj;
+  { // Do evaluate.
+    DebuggerAgentManager::UtilityContextScope utility_scope;
+    v8::Handle<v8::Context> v8Context =
+        V8Proxy::context(GetPage()->mainFrame());
+    if (v8Context.IsEmpty()) {
+      *exception = "No window context.";
+      return "";
+    }
+    V8Proxy* proxy = V8Proxy::retrieve(GetPage()->mainFrame());
+    v8::Context::Scope context_scope(v8Context);
+    v8::TryCatch try_catch;
+    v8::Handle<v8::Script> script = proxy->compileScript(
+        v8ExternalString(source),
+        String(),  // url
+        0);  // source start
+    res_obj = proxy->runScript(script, true);
+    if (try_catch.HasCaught()) {
+      v8::Handle<v8::String> msg = try_catch.Message()->Get();
+      if (!msg.IsEmpty()) {
+        *exception = WebCore::toWebCoreString(msg);
+      } else {
+        *exception = "Failed to evaluate.";
+      }
+      return "";
+    }
+    DCHECK(!res_obj.IsEmpty());
+  }
+
+  { // Wrap the result.
+    v8::Context::Scope context_scope(utility_context);
+
+    v8::Handle<v8::Object> devtools = v8::Local<v8::Object>::Cast(
+        utility_context->Global()->Get(v8::String::New("devtools$$obj")));
+    v8::Handle<v8::Function> function = v8::Local<v8::Function>::Cast(
+        devtools->Get(v8::String::New("serializeConsoleObject")));
+
+    v8::Handle<v8::Value> args[] = {
+      res_obj
+    };
+    res_obj = function->Call(devtools, 1, args);
+    return WebCore::toWebCoreStringWithNullCheck(res_obj);
+  }
+}
+
 WebCore::Page* DebuggerAgentImpl::GetPage() {
   return web_view_impl_->page();
 }
diff --git a/webkit/glue/devtools/debugger_agent_impl.h b/webkit/glue/devtools/debugger_agent_impl.h
index 4d57b91..1c8ca3a9 100644
--- a/webkit/glue/devtools/debugger_agent_impl.h
+++ b/webkit/glue/devtools/debugger_agent_impl.h
@@ -57,6 +57,11 @@ class DebuggerAgentImpl : public DebuggerAgent {
       const WebCore::String& json_args,
       WebCore::String* exception);
 
+  WebCore::String EvaluateJavaScript(
+      v8::Handle<v8::Context> utility_context,
+      const WebCore::String& source,
+      WebCore::String* exception);
+
   WebCore::Page* GetPage();
   WebDevToolsAgentImpl* webdevtools_agent() { return webdevtools_agent_; };
 
diff --git a/webkit/glue/devtools/debugger_agent_manager.cc b/webkit/glue/devtools/debugger_agent_manager.cc
index 12dd202..fafa308 100644
--- a/webkit/glue/devtools/debugger_agent_manager.cc
+++ b/webkit/glue/devtools/debugger_agent_manager.cc
@@ -29,6 +29,12 @@ bool DebuggerAgentManager::in_host_dispatch_handler_ = false;
 // static
 DebuggerAgentManager::DeferrersMap DebuggerAgentManager::page_deferrers_;
 
+// static
+bool DebuggerAgentManager::in_utility_context_ = false;
+
+// static
+bool DebuggerAgentManager::debug_break_delayed_ = false;
+
 namespace {
 
 class CallerIdWrapper : public v8::Debug::ClientData {
@@ -157,7 +163,11 @@ void DebuggerAgentManager::DebugBreak(DebuggerAgentImpl* debugger_agent) {
 #if USE(V8)
   DCHECK(DebuggerAgentForHostId(debugger_agent->webdevtools_agent()->host_id())
              == debugger_agent);
-  v8::Debug::DebugBreak();
+  if (in_utility_context_) {
+    debug_break_delayed_ = true;
+  } else {
+    v8::Debug::DebugBreak();
+  }
 #endif
 }
 
@@ -201,14 +211,24 @@ void DebuggerAgentManager::OnV8DebugMessage(const v8::Debug::Message& message) {
     return;
   }
 
-  // If the context is from one of the inpected tabs or injected extension
-  // scripts it must have host_id in the data field.
-  int host_id = WebCore::V8Proxy::contextDebugId(context);
-  if (host_id != -1) {
-    DebuggerAgentImpl* agent = DebuggerAgentForHostId(host_id);
-    if (agent) {
-      agent->DebuggerOutput(out);
-      return;
+  if (in_utility_context_) {
+    if (message.GetEvent() == v8::Break) {
+      // This may happen when two tabs are being debugged in the same process.
+      // Suppose that first debugger is pauesed on an exception. It will run
+      // nested MessageLoop which may process Break request from the second
+      // debugger.
+      debug_break_delayed_ = true;
+    }
+  } else {
+    // If the context is from one of the inpected tabs or injected extension
+    // scripts it must have host_id in the data field.
+    int host_id = WebCore::V8Proxy::contextDebugId(context);
+    if (host_id != -1) {
+      DebuggerAgentImpl* agent = DebuggerAgentForHostId(host_id);
+      if (agent) {
+        agent->DebuggerOutput(out);
+        return;
+      }
     }
   }
 
diff --git a/webkit/glue/devtools/debugger_agent_manager.h b/webkit/glue/devtools/debugger_agent_manager.h
index eb8b225..a095fe2 100644
--- a/webkit/glue/devtools/debugger_agent_manager.h
+++ b/webkit/glue/devtools/debugger_agent_manager.h
@@ -52,6 +52,23 @@ class DebuggerAgentManager {
 
   static void OnNavigate();
 
+  class UtilityContextScope {
+   public:
+    UtilityContextScope() {
+      DCHECK(!in_utility_context_);
+      in_utility_context_ = true;
+    }
+    ~UtilityContextScope() {
+      if (debug_break_delayed_) {
+        v8::Debug::DebugBreak();
+        debug_break_delayed_ = false;
+      }
+      in_utility_context_ = false;
+    }
+   private:
+    DISALLOW_COPY_AND_ASSIGN(UtilityContextScope);
+  };
+
  private:
   DebuggerAgentManager();
   ~DebuggerAgentManager();
@@ -75,6 +92,9 @@ class DebuggerAgentManager {
       DeferrersMap;
   static DeferrersMap page_deferrers_;
 
+  static bool in_utility_context_;
+  static bool debug_break_delayed_;
+
   DISALLOW_COPY_AND_ASSIGN(DebuggerAgentManager);
 };
 
diff --git a/webkit/glue/devtools/js/devtools.js b/webkit/glue/devtools/js/devtools.js
index c3f8ac7..79a2a71 100644
--- a/webkit/glue/devtools/js/devtools.js
+++ b/webkit/glue/devtools/js/devtools.js
@@ -67,18 +67,21 @@ devtools.ToolsAgent.prototype.reset = function() {
 /**
  * @param {string} script Script exression to be evaluated in the context of the
  *     inspected page.
- * @param {function(string):undefined} callback Function to call with the
- *     result.
+ * @param {function(Object|string, boolean):undefined} opt_callback Function to call
+ *     with the result.
  */
-devtools.ToolsAgent.prototype.evaluateJavaScript = function(script, callback) {
-  var callbackId = devtools.Callback.wrap(function(result) {
-    var pair = JSON.parse(result);
-    if (callback) {
-      callback(pair[0], pair[1]);
+devtools.ToolsAgent.prototype.evaluateJavaScript = function(script,
+    opt_callback) {
+  var callbackId = devtools.Callback.wrap(function(result, exception) {
+    if (opt_callback) {
+      if (exception) {
+        opt_callback(exception, true /* result is exception */);
+      } else {
+        opt_callback(JSON.parse(result), false);
+      }
     }
   });
-  RemoteToolsAgent.ExecuteUtilityFunction(callbackId,
-      'evaluate', JSON.stringify([script]));
+  RemoteToolsAgent.EvaluateJavaScript(callbackId, script);
 };
 
 
diff --git a/webkit/glue/devtools/js/inject.js b/webkit/glue/devtools/js/inject.js
index ec26991..6fe992a 100644
--- a/webkit/glue/devtools/js/inject.js
+++ b/webkit/glue/devtools/js/inject.js
@@ -545,7 +545,7 @@ devtools.Injected.prototype.getUniqueStyleProperties_ = function(style) {
  */
 devtools.Injected.prototype.wrapConsoleObject = function(obj) {
   var type = typeof obj;
-  if (type == 'object' || type == 'function') {
+  if ((type == 'object' && obj != null) || type == 'function') {
     var objId = '#consoleobj#' + this.lastCachedConsoleObjectId_++;
     this.cachedConsoleObjects_[objId] = obj;
     var result = { ___devtools_id : objId };
@@ -554,24 +554,25 @@ devtools.Injected.prototype.wrapConsoleObject = function(obj) {
       result[name] = '';
     }
     return result;
-  } else {
-    return obj;
   }
+  return obj;
 };
 
 
 /**
  * Caches console object for subsequent calls to getConsoleObjectProperties.
  * @param {Object} obj Object to cache.
- * @return {string} console object id.
+ * @return {string} Console object wrapper serialized into a JSON string.
  */
-devtools.Injected.prototype.evaluate = function(expression) {
-  try {
-    // Evaluate the expression in the global context of the inspected window.
-    return [ this.wrapConsoleObject(contentWindow.eval(expression)), false ];
-  } catch (e) {
-    return [ e.toString(), true ];
-  }
+devtools.Injected.prototype.serializeConsoleObject = function(obj) {
+  var result = this.wrapConsoleObject(obj);
+  return JSON.stringify(result,
+      function (key, value) {
+        if (value === undefined) {
+         return 'undefined';
+        }
+        return value;
+      });
 };
 
 
diff --git a/webkit/glue/devtools/js/tests.js b/webkit/glue/devtools/js/tests.js
index ade8de3..7f75f71 100644
--- a/webkit/glue/devtools/js/tests.js
+++ b/webkit/glue/devtools/js/tests.js
@@ -444,6 +444,28 @@ TestSuite.prototype.testSetBreakpoint = function() {
 
 
 /**
+ * Tests 'Pause' button will pause debugger when a snippet is evaluated.
+ */
+TestSuite.prototype.testPauseInEval = function() {
+  this.showPanel('scripts');
+
+  var test = this;
+
+  var pauseButton = document.getElementById('scripts-pause');
+  pauseButton.click();
+
+  devtools.tools.evaluateJavaScript('fib(10)');
+
+  this.addSniffer(WebInspector, 'pausedScript',
+      function() {
+        test.releaseControl();
+      });
+
+  test.takeControl();
+};
+
+
+/**
  * Key event with given key identifier.
  */
 TestSuite.KeyEvent = function(key) {
diff --git a/webkit/glue/devtools/tools_agent.h b/webkit/glue/devtools/tools_agent.h
index ae1e15e..be72dc8 100644
--- a/webkit/glue/devtools/tools_agent.h
+++ b/webkit/glue/devtools/tools_agent.h
@@ -20,6 +20,9 @@
   METHOD3(ExecuteUtilityFunction, int /* call_id */, \
       String /* function_name */, String /* json_args */) \
   \
+  /* Requests that the js source is executed within the inspected page. */ \
+  METHOD2(EvaluateJavaScript, int /* call_id */, String /* source*/) \
+  \
   /* Clears cached console messages. */ \
   METHOD0(ClearConsoleMessages) \
   \
@@ -44,6 +47,10 @@ DEFINE_RPC_CLASS(ToolsAgent, TOOLS_AGENT_STRUCT)
   METHOD3(DidExecuteUtilityFunction, int /* call_id */, String /* result */, \
       String /* exception */) \
   \
+  /* Response to the EvaluateJavaScript. */ \
+  METHOD3(DidEvaluateJavaScript, int /* call_id */, String /* result */, \
+      String /* exception */) \
+  \
   /* Sends InspectorFrontend message to be dispatched on client. */ \
   METHOD1(DispatchOnClient, String /* data */) \
   \
diff --git a/webkit/glue/webdevtoolsagent_impl.cc b/webkit/glue/webdevtoolsagent_impl.cc
index 4f2053e..d61f3b7 100644
--- a/webkit/glue/webdevtoolsagent_impl.cc
+++ b/webkit/glue/webdevtoolsagent_impl.cc
@@ -213,6 +213,15 @@ void WebDevToolsAgentImpl::ExecuteUtilityFunction(
       result, exception);
 }
 
+void WebDevToolsAgentImpl::EvaluateJavaScript(
+    int call_id,
+    const WebCore::String& source) {
+  String exception;
+  String result = debugger_agent_impl_->EvaluateJavaScript(utility_context_,
+      source, &exception);
+  tools_agent_delegate_stub_->DidEvaluateJavaScript(call_id, result, exception);
+}
+
 void WebDevToolsAgentImpl::ClearConsoleMessages() {
   Page* page = web_view_impl_->page();
   if (page) {
diff --git a/webkit/glue/webdevtoolsagent_impl.h b/webkit/glue/webdevtoolsagent_impl.h
index 599bc0f..d06705e 100644
--- a/webkit/glue/webdevtoolsagent_impl.h
+++ b/webkit/glue/webdevtoolsagent_impl.h
@@ -48,6 +48,9 @@ class WebDevToolsAgentImpl
       int call_id,
       const WebCore::String& function_name,
       const WebCore::String& json_args);
+  virtual void EvaluateJavaScript(
+      int call_id,
+      const WebCore::String& source);
   virtual void ClearConsoleMessages();
   virtual void GetResourceContent(
       int call_id,
diff --git a/webkit/glue/webkit_resources.grd b/webkit/glue/webkit_resources.grd
index bbd40b6..7a2c6d9 100644
--- a/webkit/glue/webkit_resources.grd
+++ b/webkit/glue/webkit_resources.grd
@@ -36,7 +36,7 @@
         <include name="IDR_LINUX_RADIO_DISABLED_OFF" file="resources\linux-radio-disabled-off.png" type="BINDATA" />
         <include name="IDR_LINUX_RADIO_DISABLED_ON" file="resources\linux-radio-disabled-on.png" type="BINDATA" />
       </if>
-
+ 
       <include name="IDC_ALIAS" file="resources\aliasb.cur" type="CURSOR" />
       <include name="IDC_CELL" file="resources\cell.cur" type="CURSOR" />
       <include name="IDC_COLRESIZE" file="resources\col_resize.cur" type="CURSOR" />