[remoteobjects] Implement remote method invocation

This change:
- implements the remote method invocation functionality of the WebView
  addJavascriptInterface API.
- adds several browser tests to verify basic cases.
When a method is called on the exposed object, the renderer first looks
it up in a cache. If not found, it asks the browser to confirm this
method exists and adds it to the cache.

Design doc:
https://docs.google.com/document/d/1T8Zj_gZK7jHsy80Etk-Rw4hXMIW4QeaTtXjy5ZKP3X0/

Bug: 794320
Change-Id: I3838bde1011cc63d4a4b55eedb7f6d27c946be6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023187Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749264}

content/browser/frame_host/render_frame_host_impl_browsertest.cc

@@ -3928,24 +3928,70 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest,
 // TODO(crbug.com/794320): the code below is temporary and will be removed when
 // Java Bridge is mojofied.
 #if defined(OS_ANDROID)
-const int32_t kObjectId = 5;
-const char* const kMethods[] = {"b", "c", "d"};
+
+struct ObjectData {
+  const int32_t id;
+  const std::vector<std::string> methods;
+};
+
+ObjectData kMainObject{5, {"getId", "getInnerObject", "readArray"}};
+ObjectData kInnerObject{10, {"getInnerId"}};
+
+class MockInnerObject : public blink::mojom::RemoteObject {
+ public:
+  void HasMethod(const std::string& name, HasMethodCallback callback) override {
+    bool has_method =
+        std::find(kInnerObject.methods.begin(), kInnerObject.methods.end(),
+                  name) != kInnerObject.methods.end();
+    std::move(callback).Run(has_method);
+  }
+  void GetMethods(GetMethodsCallback callback) override {
+    std::move(callback).Run(kInnerObject.methods);
+  }
+  void InvokeMethod(
+      const std::string& name,
+      std::vector<blink::mojom::RemoteInvocationArgumentPtr> arguments,
+      InvokeMethodCallback callback) override {
+    EXPECT_EQ("getInnerId", name);
+    blink::mojom::RemoteInvocationResultPtr result =
+        blink::mojom::RemoteInvocationResult::New();
+    result->error = blink::mojom::RemoteInvocationError::OK;
+    result->value = blink::mojom::RemoteInvocationResultValue::NewNumberValue(
+        kInnerObject.id);
+    std::move(callback).Run(std::move(result));
+  }
+};
 
 class MockObject : public blink::mojom::RemoteObject {
  public:
   void HasMethod(const std::string& name, HasMethodCallback callback) override {
-    // TODO(crbug.com/794320): implement this.
+    bool has_method =
+        std::find(kMainObject.methods.begin(), kMainObject.methods.end(),
+                  name) != kMainObject.methods.end();
+    std::move(callback).Run(has_method);
   }
 
   void GetMethods(GetMethodsCallback callback) override {
-    std::move(callback).Run(
-        std::vector<std::string>(std::begin(kMethods), std::end(kMethods)));
+    std::move(callback).Run(kMainObject.methods);
   }
   void InvokeMethod(
       const std::string& name,
       std::vector<blink::mojom::RemoteInvocationArgumentPtr> arguments,
       InvokeMethodCallback callback) override {
-    // TODO(crbug.com/794320): implement this.
+    blink::mojom::RemoteInvocationResultPtr result =
+        blink::mojom::RemoteInvocationResult::New();
+    result->error = blink::mojom::RemoteInvocationError::OK;
+    if (name == "getId") {
+      result->value = blink::mojom::RemoteInvocationResultValue::NewNumberValue(
+          kMainObject.id);
+    } else if (name == "readArray") {
+      result->value =
+          blink::mojom::RemoteInvocationResultValue::NewBooleanValue(true);
+    } else if (name == "getInnerObject") {
+      result->value = blink::mojom::RemoteInvocationResultValue::NewObjectId(
+          kInnerObject.id);
+    }
+    std::move(callback).Run(std::move(result));
   }
 };
 
@@ -3954,9 +4000,13 @@ class MockObjectHost : public blink::mojom::RemoteObjectHost {
   void GetObject(
       int32_t object_id,
       mojo::PendingReceiver<blink::mojom::RemoteObject> receiver) override {
-    EXPECT_EQ(kObjectId, object_id);
+    if (object_id == kMainObject.id) {
       mojo::MakeSelfOwnedReceiver(std::make_unique<MockObject>(),
                                   std::move(receiver));
+    } else if (object_id == kInnerObject.id) {
+      mojo::MakeSelfOwnedReceiver(std::make_unique<MockInnerObject>(),
+                                  std::move(receiver));
+    }
   }
 
   void ReleaseObject(int32_t) override {
@@ -3971,9 +4021,9 @@ class MockObjectHost : public blink::mojom::RemoteObjectHost {
   mojo::Receiver<blink::mojom::RemoteObjectHost> receiver_{this};
 };
 
-class RenderFrameHostObserver : public WebContentsObserver {
+class RemoteObjectInjector : public WebContentsObserver {
  public:
-  explicit RenderFrameHostObserver(WebContents* web_contents)
+  explicit RemoteObjectInjector(WebContents* web_contents)
       : WebContentsObserver(web_contents) {}
 
  private:
@@ -3985,48 +4035,115 @@ class RenderFrameHostObserver : public WebContentsObserver {
         ->GetInterface(factory.BindNewPipeAndPassReceiver());
     factory->CreateRemoteObjectGateway(host_.GetRemote(),
                                        gateway.BindNewPipeAndPassReceiver());
-    gateway->AddNamedObject("testObject", kObjectId);
+    gateway->AddNamedObject("testObject", kMainObject.id);
   }
 
   MockObjectHost host_;
 
-  DISALLOW_COPY_AND_ASSIGN(RenderFrameHostObserver);
+  DISALLOW_COPY_AND_ASSIGN(RemoteObjectInjector);
 };
 
+namespace {
+void SetupRemoteObjectInvocation(Shell* shell, const GURL& url) {
+  WebContents* web_contents = shell->web_contents();
+
+  // The first load triggers RenderFrameCreated on a RenderFrameHostObserver
+  // instance, where the object injection happens.
+  shell->LoadURL(url);
+  EXPECT_TRUE(WaitForLoadStop(web_contents));
+  // Injected objects become visible only after reload.
+  web_contents->GetController().Reload(ReloadType::NORMAL, false);
+  EXPECT_TRUE(WaitForLoadStop(web_contents));
+}
+}  // namespace
+
 // TODO(crbug.com/794320): Remove this when the new Java Bridge code is
 // integrated into WebView.
 // This test is a temporary way of verifying that the renderer part
 // works as expected.
 IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
                        RemoteObjectEnumerateProperties) {
-  GURL url1(embedded_test_server()->GetURL("/empty.html"));
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
 
   WebContents* web_contents = shell()->web_contents();
-  RenderFrameHostObserver rfh_observer(web_contents);
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
 
-  {
-    // The first load triggers RenderFrameCreated on |rfh_observer|, where the
-    // object injection happens.
-    TestNavigationObserver observer(web_contents);
-    shell()->LoadURL(url1);
-    observer.Wait();
-  }
-
-  {
-    // Injected objects become visible only after reload
-    // (see JavaBridgeBasicsTest#testEnumerateMembers in
-    // JavaBridgeBasicsTest.java).
-    TestNavigationObserver observer(web_contents);
-    web_contents->GetController().Reload(ReloadType::NORMAL, false);
-    observer.Wait();
-  }
-
-  const std::string kScript = "Object.keys(testObject).join(' ');";
+  std::string kScript = "Object.keys(testObject).join(' ');";
   auto result = EvalJs(web_contents, kScript);
-  EXPECT_EQ(base::JoinString(std::vector<std::string>(std::begin(kMethods),
-                                                      std::end(kMethods)),
-                             " "),
+  EXPECT_EQ(base::JoinString(kMainObject.methods, " "),
             result.value.GetString());
 }
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       RemoteObjectInvokeNonexistentMethod) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  std::string kScript = "testObject.getInnerId();";
+  EXPECT_FALSE(EvalJs(web_contents, kScript).error.empty());
+}
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       RemoteObjectInvokeMethodReturningNumber) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  std::string kScript = "testObject.getId();";
+  EXPECT_EQ(kMainObject.id, EvalJs(web_contents, kScript));
+}
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       RemoteObjectInvokeMethodTakingArray) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  std::string kScript = "testObject.readArray([6, 8, 2]);";
+  EXPECT_TRUE(EvalJs(web_contents, kScript).error.empty());
+}
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       RemoteObjectInvokeMethodReturningObject) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  std::string kScript = "testObject.getInnerObject().getInnerId();";
+  EXPECT_EQ(kInnerObject.id, EvalJs(web_contents, kScript));
+}
+
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
+                       RemoteObjectInvokeMethodException) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  std::string error_message = "hahaha";
+
+  std::string kScript = JsReplace(R"(
+      const array = [1, 2, 3];
+      Object.defineProperty(array, 0, {
+        get() { throw new Error($1); }
+      });
+      testObject.readArray(array);
+    )",
+                                  error_message);
+  auto error = EvalJs(web_contents, kScript).error;
+  EXPECT_NE(error.find(error_message), std::string::npos);
+}
+
 #endif  // OS_ANDROID
 }  // namespace content

third_party/blink/renderer/modules/remote_objects/remote_object.cc

@@ -3,11 +3,162 @@
 // found in the LICENSE file.
 
 #include "third_party/blink/renderer/modules/remote_objects/remote_object.h"
+#include "gin/converter.h"
+#include "third_party/blink/public/web/blink.h"
+#include "third_party/blink/renderer/platform/bindings/v8_binding.h"
+#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
 
 namespace blink {
 
 gin::WrapperInfo RemoteObject::kWrapperInfo = {gin::kEmbedderNativeGin};
 
+namespace {
+const char kMethodInvocationAsConstructorDisallowed[] =
+    "Java bridge method can't be invoked as a constructor";
+const char kMethodInvocationNonexistentMethod[] =
+    "Java bridge method does not exist for this object";
+const char kMethodInvocationOnNonInjectedObjectDisallowed[] =
+    "Java bridge method can't be invoked on a non-injected object";
+const char kMethodInvocationErrorMessage[] =
+    "Java bridge method invocation error";
+
+String RemoteInvocationErrorToString(
+    mojom::blink::RemoteInvocationError value) {
+  switch (value) {
+    case mojom::blink::RemoteInvocationError::METHOD_NOT_FOUND:
+      return "method not found";
+    case mojom::blink::RemoteInvocationError::OBJECT_GET_CLASS_BLOCKED:
+      return "invoking Object.getClass() is not permitted";
+    case mojom::blink::RemoteInvocationError::EXCEPTION_THROWN:
+      return "an exception was thrown";
+    default:
+      return String::Format("unknown RemoteInvocationError value: %d", value);
+  }
+}
+
+v8::Local<v8::Object> GetMethodCache(v8::Isolate* isolate,
+                                     v8::Local<v8::Object> object) {
+  static const V8PrivateProperty::SymbolKey kMethodCacheKey;
+  V8PrivateProperty::Symbol method_cache_symbol =
+      V8PrivateProperty::GetSymbol(isolate, kMethodCacheKey);
+  v8::Local<v8::Value> result;
+  if (!method_cache_symbol.GetOrUndefined(object).ToLocal(&result))
+    return v8::Local<v8::Object>();
+
+  if (result->IsUndefined()) {
+    result = v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
+    ignore_result(method_cache_symbol.Set(object, result));
+  }
+
+  DCHECK(result->IsObject());
+  return result.As<v8::Object>();
+}
+
+mojom::blink::RemoteInvocationArgumentPtr JSValueToMojom(
+    const v8::Local<v8::Value>& js_value,
+    v8::Isolate* isolate) {
+  if (js_value->IsNumber()) {
+    return mojom::blink::RemoteInvocationArgument::NewNumberValue(
+        js_value->NumberValue(isolate->GetCurrentContext()).ToChecked());
+  }
+
+  if (js_value->IsBoolean()) {
+    return mojom::blink::RemoteInvocationArgument::NewBooleanValue(
+        js_value->BooleanValue(isolate));
+  }
+
+  if (js_value->IsString()) {
+    return mojom::blink::RemoteInvocationArgument::NewStringValue(
+        ToCoreString(js_value.As<v8::String>()));
+  }
+
+  if (js_value->IsNull()) {
+    return mojom::blink::RemoteInvocationArgument::NewSingletonValue(
+        mojom::blink::SingletonJavaScriptValue::kNull);
+  }
+
+  if (js_value->IsUndefined()) {
+    return mojom::blink::RemoteInvocationArgument::NewSingletonValue(
+        mojom::blink::SingletonJavaScriptValue::kUndefined);
+  }
+
+  if (js_value->IsArray()) {
+    auto array = js_value.As<v8::Array>();
+    WTF::Vector<mojom::blink::RemoteInvocationArgumentPtr> nested_arguments;
+    for (uint32_t i = 0; i < array->Length(); ++i) {
+      v8::Local<v8::Value> element_v8;
+
+      if (!array->Get(isolate->GetCurrentContext(), i).ToLocal(&element_v8))
+        return nullptr;
+
+      // The array length might change during iteration. Set the output array
+      // elements to null for nonexistent input array elements.
+      if (!array->HasRealIndexedProperty(isolate->GetCurrentContext(), i)
+               .FromMaybe(false)) {
+        nested_arguments.push_back(
+            mojom::blink::RemoteInvocationArgument::NewSingletonValue(
+                mojom::blink::SingletonJavaScriptValue::kNull));
+      } else {
+        mojom::blink::RemoteInvocationArgumentPtr nested_argument;
+
+        // This code prevents infinite recursion on the sender side.
+        // Null value is sent according to the Java-side conversion rules for
+        // expected parameter types:
+        // - multi-dimensional and object arrays are not allowed and are
+        // converted to nulls;
+        // - for primitive arrays, the null value will be converted to primitive
+        // zero;
+        // - for string arrays, the null value will be converted to a null
+        // string. See RemoteObjectImpl.convertArgument() in
+        // content/public/android/java/src/org/chromium/content/browser/remoteobjects/RemoteObjectImpl.java
+        if (element_v8->IsObject()) {
+          nested_argument =
+              mojom::blink::RemoteInvocationArgument::NewSingletonValue(
+                  mojom::blink::SingletonJavaScriptValue::kNull);
+        } else {
+          nested_argument = JSValueToMojom(element_v8, isolate);
+        }
+
+        if (!nested_argument)
+          return nullptr;
+
+        nested_arguments.push_back(std::move(nested_argument));
+      }
+
+      return mojom::blink::RemoteInvocationArgument::NewArrayValue(
+          std::move(nested_arguments));
+    }
+  }
+
+  return nullptr;
+}
+
+v8::Local<v8::Value> MojomToJSValue(
+    const mojom::blink::RemoteInvocationResultValuePtr& result_value,
+    v8::Isolate* isolate) {
+  if (result_value->is_number_value()) {
+    return v8::Number::New(isolate, result_value->get_number_value());
+  }
+
+  if (result_value->is_boolean_value()) {
+    return v8::Boolean::New(isolate, result_value->get_boolean_value());
+  }
+
+  if (result_value->is_string_value()) {
+    return V8String(isolate, result_value->get_string_value());
+  }
+
+  switch (result_value->get_singleton_value()) {
+    case mojom::blink::SingletonJavaScriptValue::kNull:
+      return v8::Null(isolate);
+    case mojom::blink::SingletonJavaScriptValue::kUndefined:
+      return v8::Undefined(isolate);
+  }
+
+  return v8::Local<v8::Value>();
+}
+}  // namespace
+
 RemoteObject::RemoteObject(v8::Isolate* isolate,
                            RemoteObjectGatewayImpl* gateway,
                            int32_t object_id)
@@ -21,19 +172,131 @@ gin::ObjectTemplateBuilder RemoteObject::GetObjectTemplateBuilder(
       .AddNamedPropertyInterceptor();
 }
 
+void RemoteObject::RemoteObjectInvokeCallback(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  if (info.IsConstructCall()) {
+    // This is not a constructor. Throw and return.
+    isolate->ThrowException(v8::Exception::Error(
+        V8String(isolate, kMethodInvocationAsConstructorDisallowed)));
+    return;
+  }
+
+  RemoteObject* remote_object;
+  if (!gin::ConvertFromV8(isolate, info.Holder(), &remote_object)) {
+    // Someone messed with the |this| pointer. Throw and return.
+    isolate->ThrowException(v8::Exception::Error(
+        V8String(isolate, kMethodInvocationOnNonInjectedObjectDisallowed)));
+    return;
+  }
+
+  String method_name = ToCoreString(info.Data().As<v8::String>());
+
+  v8::Local<v8::Object> method_cache = GetMethodCache(
+      isolate, remote_object->GetWrapper(isolate).ToLocalChecked());
+  if (method_cache.IsEmpty())
+    return;
+
+  v8::Local<v8::Value> cached_method =
+      method_cache
+          ->Get(isolate->GetCurrentContext(), info.Data().As<v8::String>())
+          .ToLocalChecked();
+
+  if (cached_method->IsUndefined()) {
+    isolate->ThrowException(v8::Exception::Error(
+        V8String(isolate, kMethodInvocationNonexistentMethod)));
+    return;
+  }
+
+  WTF::Vector<mojom::blink::RemoteInvocationArgumentPtr> arguments;
+  arguments.ReserveInitialCapacity(info.Length());
+
+  for (int i = 0; i < info.Length(); i++) {
+    auto argument = JSValueToMojom(info[i], isolate);
+    if (!argument)
+      return;
+
+    arguments.push_back(std::move(argument));
+  }
+
+  remote_object->EnsureRemoteIsBound();
+  mojom::blink::RemoteInvocationResultPtr result;
+  remote_object->object_->InvokeMethod(method_name, std::move(arguments),
+                                       &result);
+
+  if (result->error != mojom::blink::RemoteInvocationError::OK) {
+    String message = String::Format("%s : ", kMethodInvocationErrorMessage) +
+                     RemoteInvocationErrorToString(result->error);
+    isolate->ThrowException(v8::Exception::Error(V8String(isolate, message)));
+    return;
+  }
+
+  if (!result->value)
+    return;
+
+  if (result->value->is_object_id()) {
+    // TODO(crbug.com/794320): need to check whether an object with this id has
+    // already been injected
+    RemoteObject* object_result =
+        new RemoteObject(info.GetIsolate(), remote_object->gateway_,
+                         result->value->get_object_id());
+    gin::Handle<RemoteObject> controller =
+        gin::CreateHandle(isolate, object_result);
+    if (controller.IsEmpty())
+      info.GetReturnValue().SetUndefined();
+    else
+      info.GetReturnValue().Set(controller.ToV8());
+  } else {
+    info.GetReturnValue().Set(MojomToJSValue(result->value, isolate));
+  }
+}
+
+void RemoteObject::EnsureRemoteIsBound() {
+  if (!object_.is_bound()) {
+    gateway_->BindRemoteObjectReceiver(object_id_,
+                                       object_.BindNewPipeAndPassReceiver());
+  }
+}
+
 v8::Local<v8::Value> RemoteObject::GetNamedProperty(
     v8::Isolate* isolate,
     const std::string& property) {
-  // TODO(crbug.com/794320): implement this.
-  return gin::StringToSymbol(isolate, property);
+  auto wtf_property = WTF::String::FromUTF8(property);
+
+  v8::Local<v8::String> v8_property = V8AtomicString(isolate, wtf_property);
+  v8::Local<v8::Object> method_cache =
+      GetMethodCache(isolate, GetWrapper(isolate).ToLocalChecked());
+  if (method_cache.IsEmpty())
+    return v8::Local<v8::Value>();
+
+  v8::Local<v8::Value> cached_method =
+      method_cache->Get(isolate->GetCurrentContext(), v8_property)
+          .ToLocalChecked();
+
+  if (!cached_method->IsUndefined())
+    return cached_method;
+
+  // if not in the cache, ask the browser
+  EnsureRemoteIsBound();
+  bool method_exists = false;
+  object_->HasMethod(wtf_property, &method_exists);
+
+  if (!method_exists) {
+    return v8::Local<v8::Value>();
+  }
+
+  auto function = v8::Function::New(isolate->GetCurrentContext(),
+                                    RemoteObjectInvokeCallback, v8_property)
+                      .ToLocalChecked();
+
+  ignore_result(method_cache->CreateDataProperty(isolate->GetCurrentContext(),
+                                                 v8_property, function));
+  return function;
 }
 
 std::vector<std::string> RemoteObject::EnumerateNamedProperties(
     v8::Isolate* isolate) {
-  if (!object_.is_bound()) {
-    gateway_->BindRemoteObjectReceiver(object_id_,
-                                       object_.BindNewPipeAndPassReceiver());
-  }
+  EnsureRemoteIsBound();
   WTF::Vector<WTF::String> methods;
   object_->GetMethods(&methods);
   std::vector<std::string> result;

third_party/blink/renderer/modules/remote_objects/remote_object.h

@@ -38,6 +38,10 @@ class RemoteObject : public gin::Wrappable<RemoteObject>,
       v8::Isolate* isolate) override;
 
  private:
+  static void RemoteObjectInvokeCallback(
+      const v8::FunctionCallbackInfo<v8::Value>& info);
+  void EnsureRemoteIsBound();
+
   WeakPersistent<RemoteObjectGatewayImpl> gateway_{nullptr};
   mojo::Remote<mojom::blink::RemoteObject> object_;
   int32_t object_id_;

third_party/blink/renderer/modules/remote_objects/remote_object_gateway_impl.cc

@@ -23,7 +23,6 @@ RemoteObjectGatewayImpl* RemoteObjectGatewayImpl::From(LocalFrame& frame) {
 
 void RemoteObjectGatewayImpl::InjectNamed(const WTF::String& object_name,
                                           int32_t object_id) {
-  // TODO(crbug.com/794320): implement this.
   ScriptState* script_state = ToScriptStateForMainWorld(GetSupplementable());
   ScriptState::Scope scope(script_state);
   v8::Isolate* isolate = script_state->GetIsolate();