[devtools] Reduce performance overhead of message dispatch in the front-end.

rebase CL 2044170 part 1:
Adds a new `FrameNavigationControl::JavaScriptMethodExecuteRequest`
method. Previously every message was turned into an object literal in a new
script and sent to the renderer process executing the
front-end, which was very costly and leads to a lot of unnecessary
script executing in the V8 engine.

Bug: chromium:1029427
Change-Id: I9e803ea9d32ce333c4eea1d2cb06f1b2eaffaf02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431864Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Maksim Sadym <sadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812308}

content/browser/renderer_host/render_frame_host_impl.cc

@@ -1620,6 +1620,21 @@ void RenderFrameHostImpl::AddMessageToConsole(
   AddMessageToConsoleImpl(level, message, false /* discard_duplicates */);
 }
 
+void RenderFrameHostImpl::ExecuteJavaScriptMethod(
+    const base::string16& object_name,
+    const base::string16& method_name,
+    base::Value arguments,
+    JavaScriptResultCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(arguments.is_list());
+  CHECK(CanExecuteJavaScript());
+
+  const bool wants_result = !callback.is_null();
+  GetNavigationControl()->JavaScriptMethodExecuteRequest(
+      object_name, method_name, std::move(arguments), wants_result,
+      std::move(callback));
+}
+
 void RenderFrameHostImpl::ExecuteJavaScript(const base::string16& javascript,
                                             JavaScriptResultCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

content/browser/renderer_host/render_frame_host_impl.h

@@ -308,6 +308,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
   gfx::NativeView GetNativeView() override;
   void AddMessageToConsole(blink::mojom::ConsoleMessageLevel level,
                            const std::string& message) override;
+  void ExecuteJavaScriptMethod(const base::string16& object_name,
+                               const base::string16& method_name,
+                               base::Value arguments,
+                               JavaScriptResultCallback callback) override;
   void ExecuteJavaScript(const base::string16& javascript,
                          JavaScriptResultCallback callback) override;
   void ExecuteJavaScriptInIsolatedWorld(const base::string16& javascript,

content/browser/renderer_host/render_frame_host_impl_browsertest.cc

@@ -223,6 +223,62 @@ class RenderFrameHostImplBrowserTest : public ContentBrowserTest {
   net::EmbeddedTestServer https_server_;
 };
 
+std::string ExecuteJavaScriptMethodAndGetResult(
+    RenderFrameHostImpl* render_frame,
+    const std::string& object,
+    const std::string& method,
+    base::Value arguments) {
+  bool executing = true;
+  std::string result;
+  base::OnceCallback<void(base::Value)> call_back = base::BindOnce(
+      [](bool* flag, std::string* reason, base::Value value) {
+        *flag = false;
+        DCHECK(value.is_string());
+        *reason = value.GetString();
+      },
+      base::Unretained(&executing), base::Unretained(&result));
+
+  render_frame->ExecuteJavaScriptMethod(
+      base::UTF8ToUTF16(object), base::UTF8ToUTF16(method),
+      std::move(arguments), std::move(call_back));
+
+  while (executing) {
+    base::RunLoop loop;
+    loop.RunUntilIdle();
+  }
+
+  return result;
+}
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       ExecuteJavaScriptMethodWorksWithArguments) {
+  EXPECT_TRUE(NavigateToURL(
+      shell(), GetTestUrl("render_frame_host", "jsMethodTest.html")));
+
+  RenderFrameHostImpl* render_frame = web_contents()->GetMainFrame();
+  render_frame->AllowInjectingJavaScript();
+
+  base::Value empty_arguments(base::Value::Type::LIST);
+  std::string result = ExecuteJavaScriptMethodAndGetResult(
+      render_frame, "window", "someMethod", std::move(empty_arguments));
+  EXPECT_EQ(result, "called someMethod()");
+
+  base::Value single_arguments(base::Value::Type::LIST);
+  single_arguments.Append("arg1");
+  result = ExecuteJavaScriptMethodAndGetResult(
+      render_frame, "window", "someMethod", std::move(single_arguments));
+  EXPECT_EQ(result, "called someMethod(arg1)");
+
+  base::Value four_arguments(base::Value::Type::LIST);
+  four_arguments.Append("arg1");
+  four_arguments.Append("arg2");
+  four_arguments.Append("arg3");
+  four_arguments.Append("arg4");
+  result = ExecuteJavaScriptMethodAndGetResult(
+      render_frame, "window", "someMethod", std::move(four_arguments));
+  EXPECT_EQ(result, "called someMethod(arg1,arg2,arg3,arg4)");
+}
+
 // Test that when creating a new window, the main frame is correctly focused.
 IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, IsFocused_AtLoad) {
   EXPECT_TRUE(

content/common/frame.mojom

@@ -155,6 +155,24 @@ interface FrameNavigationControl {
       pending_associated_remote<blink.mojom.DevToolsAgentHost> agent_host,
       pending_associated_receiver<blink.mojom.DevToolsAgent> agent);
 
+  // Request for the renderer to execute a given JavaScript method on a given
+  // object (both identified by name) in the frame's context.
+  //
+  // |object_name| is the global name of the object.
+  //
+  // |method_name| is the name of the method on the object.
+  //
+  // |arguments| is the list of arguments to pass to the method invocation.
+  //
+  // |wants_result| is true if the result of this execution is required by the
+  // caller. If it is false, a reply is still required by Mojo, but a null value
+  // should be returned to avoid issues serializing a large, unwanted reply.
+  JavaScriptMethodExecuteRequest(
+      mojo_base.mojom.String16 object_name,
+      mojo_base.mojom.String16 method_name,
+      mojo_base.mojom.ListValue arguments,
+      bool wants_result) => (mojo_base.mojom.Value result);
+
   // Request for the renderer to execute JavaScript in the frame's context.
   //
   // |javascript| is the string containing the JavaScript to be executed in the

content/public/browser/render_frame_host.h

@@ -267,6 +267,23 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   // will be invoked on the UI thread.
   using JavaScriptResultCallback = base::OnceCallback<void(base::Value)>;
 
+  // This API allows to execute JavaScript methods in this frame, without
+  // having to serialize the arguments into a single string, and is a lot
+  // cheaper than ExecuteJavaScript below since it avoids the need to compile
+  // and evaluate new scripts all the time.
+  //
+  // Calling
+  //
+  //   ExecuteJavaScriptMethod("obj", "foo", [1, true], callback)
+  //
+  // is semantically equivalent to
+  //
+  //   ExecuteJavaScript("obj.foo(1, true)", callback)
+  virtual void ExecuteJavaScriptMethod(const base::string16& object_name,
+                                       const base::string16& method_name,
+                                       base::Value arguments,
+                                       JavaScriptResultCallback callback) = 0;
+
   // This is the default API to run JavaScript in this frame. This API can only
   // be called on chrome:// or devtools:// URLs.
   virtual void ExecuteJavaScript(const base::string16& javascript,

content/renderer/render_frame_impl.cc

@@ -23,6 +23,7 @@
 #include "base/guid.h"
 #include "base/i18n/char_iterator.h"
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ -1097,6 +1098,46 @@ CreateDefaultURLLoaderFactoryBundle() {
       base::BindOnce(&CreateDefaultURLLoaderFactory));
 }
 
+v8::MaybeLocal<v8::Value> GetProperty(v8::Local<v8::Context> context,
+                                      v8::Local<v8::Value> object,
+                                      const base::string16& name) {
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Local<v8::String> name_str =
+      gin::ConvertToV8(isolate, name).As<v8::String>();
+  v8::Local<v8::Object> object_obj;
+  if (!object->ToObject(context).ToLocal(&object_obj)) {
+    return v8::MaybeLocal<v8::Value>();
+  }
+  return object_obj->Get(context, name_str);
+}
+
+v8::MaybeLocal<v8::Value> CallMethodOnFrame(blink::WebNavigationControl* frame,
+                                            const base::string16& object_name,
+                                            const base::string16& method_name,
+                                            base::Value arguments) {
+  v8::Local<v8::Context> context = frame->MainWorldScriptContext();
+
+  v8::Context::Scope context_scope(context);
+  std::vector<v8::Local<v8::Value>> args;
+  V8ValueConverterImpl converter;
+  converter.SetDateAllowed(true);
+  converter.SetRegExpAllowed(true);
+  for (auto const& argument : arguments.GetList()) {
+    args.push_back(converter.ToV8Value(&argument, context));
+  }
+
+  v8::Local<v8::Value> object;
+  v8::Local<v8::Value> method;
+  if (!GetProperty(context, context->Global(), object_name).ToLocal(&object) ||
+      !GetProperty(context, object, method_name).ToLocal(&method)) {
+    return v8::MaybeLocal<v8::Value>();
+  }
+
+  return frame->ExecuteMethodAndReturnValue(
+      v8::Local<v8::Function>::Cast(method), object,
+      static_cast<int>(args.size()), args.data());
+}
+
 }  // namespace
 
 RenderFrameImpl::AssertNavigationCommits::AssertNavigationCommits(
@@ -2553,6 +2594,35 @@ void RenderFrameImpl::OnCustomContextMenuAction(
   }
 }
 
+void RenderFrameImpl::JavaScriptMethodExecuteRequest(
+    const base::string16& object_name,
+    const base::string16& method_name,
+    base::Value arguments,
+    bool wants_result,
+    JavaScriptMethodExecuteRequestCallback callback) {
+  TRACE_EVENT_INSTANT0("test_tracing", "JavaScriptMethodExecuteRequest",
+                       TRACE_EVENT_SCOPE_THREAD);
+
+  // Note that CallMethodOnFrame may end up killing this object.
+  base::WeakPtr<RenderFrameImpl> weak_this = weak_factory_.GetWeakPtr();
+
+  v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
+  v8::Local<v8::Value> result;
+  if (!CallMethodOnFrame(frame_, object_name, method_name, std::move(arguments))
+           .ToLocal(&result)) {
+    std::move(callback).Run({});
+    return;
+  }
+
+  if (!weak_this)
+    return;
+
+  if (wants_result)
+    std::move(callback).Run(GetJavaScriptExecutionResult(result));
+  else
+    std::move(callback).Run({});
+}
+
 void RenderFrameImpl::JavaScriptExecuteRequest(
     const base::string16& javascript,
     bool wants_result,

content/renderer/render_frame_impl.h

@@ -562,6 +562,12 @@ class CONTENT_EXPORT RenderFrameImpl
       mojo::PendingAssociatedRemote<blink::mojom::DevToolsAgentHost> host,
       mojo::PendingAssociatedReceiver<blink::mojom::DevToolsAgent> receiver)
       override;
+  void JavaScriptMethodExecuteRequest(
+      const base::string16& object_name,
+      const base::string16& method_name,
+      base::Value arguments,
+      bool wants_result,
+      JavaScriptMethodExecuteRequestCallback callback) override;
   void JavaScriptExecuteRequest(
       const base::string16& javascript,
       bool wants_result,

content/test/data/render_frame_host/jsMethodTest.html

+<!DOCTYPE html>
+<html>
+<script>
+window.someMethod = function(){
+  var str = "called someMethod(" + Array.from(arguments).join(",") + ")";
+  return str;
+}
+</script>
+
+</html>
\ No newline at end of file

third_party/blink/public/web/web_local_frame.h

@@ -340,6 +340,13 @@ class WebLocalFrame : public WebFrame {
   virtual v8::Local<v8::Value> ExecuteScriptAndReturnValue(
       const WebScriptSource&) = 0;
 
+  // Call the function with the given receiver and arguments
+  virtual v8::MaybeLocal<v8::Value> ExecuteMethodAndReturnValue(
+      v8::Local<v8::Function>,
+      v8::Local<v8::Value>,
+      int argc,
+      v8::Local<v8::Value> argv[]) = 0;
+
   // Call the function with the given receiver and arguments, bypassing
   // canExecute().
   virtual v8::MaybeLocal<v8::Value> CallFunctionEvenIfScriptDisabled(

third_party/blink/renderer/bindings/core/v8/script_controller.cc

@@ -295,20 +295,17 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld(
     SanitizeScriptErrors sanitize_script_errors,
     const ScriptFetchOptions& fetch_options,
     ExecuteScriptPolicy policy) {
-  if (policy == kDoNotExecuteScriptWhenScriptsDisabled &&
-      !window_->CanExecuteScripts(kAboutToExecuteScript))
+  if (!CanExecuteScript(policy)) {
     return v8::Local<v8::Value>();
+  }
 
-  // |context| should be initialized already due to the MainWorldProxy() call.
+  // |context| should be initialized already due to the
+  // MainWorldProxy() call.
   v8::Local<v8::Context> context =
       window_proxy_manager_->MainWorldProxy()->ContextIfInitialized();
-
   v8::Context::Scope scope(context);
   v8::EscapableHandleScope handle_scope(GetIsolate());
 
-  if (window_->document()->IsInitialEmptyDocument())
-    window_->GetFrame()->Loader().DidAccessInitialDocument();
-
   v8::Local<v8::Value> object = ExecuteScriptAndReturnValue(
       context, source_code, base_url, sanitize_script_errors, fetch_options);
 
@@ -318,6 +315,50 @@ v8::Local<v8::Value> ScriptController::EvaluateScriptInMainWorld(
   return handle_scope.Escape(object);
 }
 
+v8::Local<v8::Value> ScriptController::EvaluateMethodInMainWorld(
+    v8::Local<v8::Function> function,
+    v8::Local<v8::Value> receiver,
+    int argc,
+    v8::Local<v8::Value> argv[],
+    ExecuteScriptPolicy policy) {
+  if (!CanExecuteScript(policy)) {
+    return v8::Local<v8::Value>();
+  }
+
+  // |context| should be initialized already due to the
+  // MainWorldProxy() call.
+  v8::Local<v8::Context> context =
+      window_proxy_manager_->MainWorldProxy()->ContextIfInitialized();
+  v8::Context::Scope scope(context);
+  v8::EscapableHandleScope handle_scope(GetIsolate());
+
+  v8::TryCatch try_catch(GetIsolate());
+  try_catch.SetVerbose(true);
+
+  ExecutionContext* executionContext =
+      ExecutionContext::From(ScriptState::From(context));
+
+  v8::MaybeLocal<v8::Value> resultObj = V8ScriptRunner::CallFunction(
+      function, executionContext, receiver, argc,
+      static_cast<v8::Local<v8::Value>*>(argv), ToIsolate(window_->GetFrame()));
+
+  if (resultObj.IsEmpty())
+    return v8::Local<v8::Value>();
+
+  return handle_scope.Escape(resultObj.ToLocalChecked());
+}
+
+bool ScriptController::CanExecuteScript(ExecuteScriptPolicy policy) {
+  if (policy == kDoNotExecuteScriptWhenScriptsDisabled &&
+      !window_->CanExecuteScripts(kAboutToExecuteScript))
+    return false;
+
+  if (window_->document()->IsInitialEmptyDocument())
+    window_->GetFrame()->Loader().DidAccessInitialDocument();
+
+  return true;
+}
+
 v8::Local<v8::Value> ScriptController::ExecuteScriptInIsolatedWorld(
     int32_t world_id,
     const ScriptSourceCode& source,

third_party/blink/renderer/bindings/core/v8/script_controller.h

@@ -84,6 +84,14 @@ class CORE_EXPORT ScriptController final
       SanitizeScriptErrors,
       const ScriptFetchOptions& = ScriptFetchOptions());
 
+  v8::Local<v8::Value> EvaluateMethodInMainWorld(
+      v8::Local<v8::Function> function,
+      v8::Local<v8::Value> receiver,
+      int argc,
+      v8::Local<v8::Value> argv[],
+      ScriptController::ExecuteScriptPolicy = ScriptController::
+          ExecuteScriptPolicy::kDoNotExecuteScriptWhenScriptsDisabled);
+
   // Evaluate JavaScript in the main world.
   v8::Local<v8::Value> EvaluateScriptInMainWorld(const ScriptSourceCode&,
                                                  const KURL& base_url,
@@ -134,6 +142,7 @@ class CORE_EXPORT ScriptController final
   static v8::ExtensionConfiguration ExtensionsFor(const ExecutionContext*);
 
  private:
+  bool CanExecuteScript(ExecuteScriptPolicy policy);
   v8::Isolate* GetIsolate() const {
     return window_proxy_manager_->GetIsolate();
   }

third_party/blink/renderer/core/frame/web_local_frame_impl.cc

@@ -891,6 +891,19 @@ void WebLocalFrameImpl::CollectGarbageForTesting() {
   ThreadState::Current()->CollectAllGarbageForTesting();
 }
 
+v8::MaybeLocal<v8::Value> WebLocalFrameImpl::ExecuteMethodAndReturnValue(
+    v8::Local<v8::Function> function,
+    v8::Local<v8::Value> receiver,
+    int argc,
+    v8::Local<v8::Value> argv[]) {
+  DCHECK(GetFrame());
+
+  return GetFrame()
+      ->DomWindow()
+      ->GetScriptController()
+      .EvaluateMethodInMainWorld(function, receiver, argc, argv);
+}
+
 v8::Local<v8::Value> WebLocalFrameImpl::ExecuteScriptAndReturnValue(
     const WebScriptSource& source) {
   DCHECK(GetFrame());

third_party/blink/renderer/core/frame/web_local_frame_impl.h

@@ -155,6 +155,12 @@ class CORE_EXPORT WebLocalFrameImpl final
   void ClearIsolatedWorldCSPForTesting(int32_t world_id) override;
   v8::Local<v8::Value> ExecuteScriptAndReturnValue(
       const WebScriptSource&) override;
+  // Call the function with the given receiver and arguments
+  v8::MaybeLocal<v8::Value> ExecuteMethodAndReturnValue(
+      v8::Local<v8::Function>,
+      v8::Local<v8::Value>,
+      int argc,
+      v8::Local<v8::Value> argv[]) override;
   v8::MaybeLocal<v8::Value> CallFunctionEvenIfScriptDisabled(
       v8::Local<v8::Function>,
       v8::Local<v8::Value>,