[unseasoned-pdf] Use a property interceptor in PostMessageReceiver

An interceptor allows for invocations of `postMessage()` on plugin
elements to be directed to the correct native PostMessageReceiver
instance. Direct Gin bindings don't work because of the implications of
having an HTMLEmbedElement be the receiving object for a plugin.

The current implementation expects
gin::WrappableBase::GetObjectTemplateBuilder() to be called once per
PostMessageReceiver instance (thus once per PDF Viewer instance).
However, Gin creates the object template once per isolate, meaning that
invocations of `postMessage()` on every PDF plugin element in the same
process were being directed to the same PostMessageReceiver instance.

This implementation by Gin is usually reasonable since the receiving
JavaScript object is expected to be the first parameter of an object
method, and object templates can be shared. However, the actual
receiving object for a plugin is an HTMLEmbedElement, and Blink
internally forwards the parameters to the exposed scriptable object.

Gin actually checks that the first parameters to function templates
created with a member function pointer (MFP) is the JavaScript `this`
object corresponding to the scriptable object. But crrev.com/862907
erroneously attempted to bypass the check by wrapping the MFP in a
repeating callback.

A named property interceptor works because it allows to create function
templates dynamically. This implementation mimics that of the Pepper
MessageChannel class.

Fixed: 1196388
Change-Id: Iececfd5a80088c4777bebcf022eaed392f8f34be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2828774
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#873449}

pdf/post_message_receiver.cc

@@ -5,9 +5,12 @@
 #include "pdf/post_message_receiver.h"
 
 #include <memory>
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
+#include "base/check_op.h"
 #include "base/location.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
@@ -15,7 +18,9 @@
 #include "base/sequenced_task_runner.h"
 #include "base/values.h"
 #include "content/public/renderer/v8_value_converter.h"
+#include "gin/function_template.h"
 #include "gin/handle.h"
+#include "gin/interceptor.h"
 #include "gin/object_template_builder.h"
 #include "gin/public/wrapper_info.h"
 #include "gin/wrappable.h"
@@ -23,6 +28,12 @@
 
 namespace chrome_pdf {
 
+namespace {
+
+constexpr char kPropertyName[] = "postMessage";
+
+}  // namespace
+
 // static
 gin::WrapperInfo PostMessageReceiver::kWrapperInfo = {gin::kEmbedderNativeGin};
 
@@ -44,31 +55,65 @@ PostMessageReceiver::PostMessageReceiver(
     v8::Isolate* isolate,
     base::WeakPtr<Client> client,
     scoped_refptr<base::SequencedTaskRunner> client_task_runner)
-    : isolate_(isolate),
+    : gin::NamedPropertyInterceptor(isolate, this),
+      isolate_(isolate),
       client_(std::move(client)),
       client_task_runner_(std::move(client_task_runner)) {}
 
 gin::ObjectTemplateBuilder PostMessageReceiver::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
-  // The function template needs to be created with a repeating callback instead
-  // of a member function pointer (MFP). Gin expects the first parameter for a
-  // callback to a MFP to be the JavaScript `this` object corresponding to this
-  // scriptable object exposed through Blink. However, the actual receiving
-  // object for a plugins is a HTMLEmbedElement and Blink internally forwards
-  // the parameters to this scriptable object.
+  // `gin::ObjectTemplateBuilder::SetMethod()` can't be used here because it
+  // would create a function template which expects the first parameter to a
+  // member function pointer to be the JavaScript `this` object corresponding
+  // to this scriptable object exposed through Blink. However, the actual
+  // receiving object for a plugin is an HTMLEmbedElement and Blink internally
+  // forwards the parameters to this scriptable object.
   //
-  // `base::Unretained(this)` is safe to use because the callback will only be
-  // called within the lifetime of the wrapped PostMessageReceiver object.
+  // Also, passing a callback would cause Gin to ignore the target. Because Gin
+  // creates the object template of a type only once per isolate, the member
+  // method of the first `PostMessageReceiver` instance would get effectively
+  // treated like a static method for all other instances.
+  //
+  // An interceptor allows for the creation of a function template per instance.
   return gin::Wrappable<PostMessageReceiver>::GetObjectTemplateBuilder(isolate)
-      .SetMethod("postMessage",
-                 base::BindRepeating(&PostMessageReceiver::PostMessage,
-                                     base::Unretained(this)));
+      .AddNamedPropertyInterceptor();
 }
 
 const char* PostMessageReceiver::GetTypeName() {
   return "ChromePdfPostMessageReceiver";
 }
 
+v8::Local<v8::Value> PostMessageReceiver::GetNamedProperty(
+    v8::Isolate* isolate,
+    const std::string& property) {
+  DCHECK_EQ(isolate_, isolate);
+
+  if (property != kPropertyName)
+    return v8::Local<v8::Value>();
+
+  return GetFunctionTemplate()
+      ->GetFunction(isolate->GetCurrentContext())
+      .ToLocalChecked();
+}
+
+std::vector<std::string> PostMessageReceiver::EnumerateNamedProperties(
+    v8::Isolate* isolate) {
+  DCHECK_EQ(isolate_, isolate);
+  return {kPropertyName};
+}
+
+v8::Local<v8::FunctionTemplate> PostMessageReceiver::GetFunctionTemplate() {
+  if (function_template_.IsEmpty()) {
+    function_template_.Reset(
+        isolate_,
+        gin::CreateFunctionTemplate(
+            isolate_, base::BindRepeating(&PostMessageReceiver::PostMessage,
+                                          weak_factory_.GetWeakPtr())));
+  }
+
+  return function_template_.Get(isolate_);
+}
+
 std::unique_ptr<base::Value> PostMessageReceiver::ConvertMessage(
     v8::Local<v8::Value> message) {
   if (!v8_value_converter_)

pdf/post_message_receiver.h

@@ -9,6 +9,7 @@
 
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
+#include "gin/interceptor.h"
 #include "gin/public/wrapper_info.h"
 #include "gin/wrappable.h"
 #include "v8/include/v8.h"
@@ -33,7 +34,8 @@ namespace chrome_pdf {
 // `PostMessageReceiver`'s lifetime is managed by the V8 garbage collector,
 // meaning it can outlive the `Client`. Messages are dropped if the `Client` is
 // destroyed.
-class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver> {
+class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver>,
+                                  public gin::NamedPropertyInterceptor {
  public:
   // The interface for a plugin client that handles messages from its embedder.
   class Client {
@@ -72,6 +74,15 @@ class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver> {
       v8::Isolate* isolate) override;
   const char* GetTypeName() override;
 
+  // gin::NamedPropertyInterceptor:
+  v8::Local<v8::Value> GetNamedProperty(v8::Isolate* isolate,
+                                        const std::string& property) override;
+  std::vector<std::string> EnumerateNamedProperties(
+      v8::Isolate* isolate) override;
+
+  // Lazily creates and retrieves `function_template_`.
+  v8::Local<v8::FunctionTemplate> GetFunctionTemplate();
+
   // Converts `message` so it can be consumed by `client_`.
   std::unique_ptr<base::Value> ConvertMessage(v8::Local<v8::Value> message);
 
@@ -80,11 +91,15 @@ class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver> {
 
   std::unique_ptr<content::V8ValueConverter> v8_value_converter_;
 
+  v8::Persistent<v8::FunctionTemplate> function_template_;
+
   v8::Isolate* isolate_;
 
   base::WeakPtr<Client> client_;
 
   scoped_refptr<base::SequencedTaskRunner> client_task_runner_;
+
+  base::WeakPtrFactory<PostMessageReceiver> weak_factory_{this};
 };
 
 }  // namespace chrome_pdf