Remove dependency from //pdf to //content.

It is not obvious if V8ValueConverter can move from //content into
//third_party/blink. Rather than waiting for that to happen, use
delegation to ask the PdfViewWebPlugin::Client to do the conversion.
Thus breaking the direct dependency.

Bug: 1220865
Change-Id: Ibfeb9294590a5f080666d9f2426bb03b8054c4a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3489766
Reviewed-by: K. Moon <kmoon@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975421}

components/pdf/renderer/pdf_view_web_plugin_client.cc

@@ -8,6 +8,7 @@
 #include "components/pdf/renderer/pdf_accessibility_tree.h"
 #include "content/public/common/use_zoom_for_dsf_policy.h"
 #include "content/public/renderer/render_thread.h"
+#include "content/public/renderer/v8_value_converter.h"
 #include "printing/buildflags/buildflags.h"
 #include "third_party/blink/public/web/web_element.h"
 
@@ -19,12 +20,30 @@ namespace pdf {
 
 PdfViewWebPluginClient::PdfViewWebPluginClient(
     content::RenderFrame* render_frame)
-    : render_frame_(render_frame) {
+    : render_frame_(render_frame),
+      v8_value_converter_(content::V8ValueConverter::Create()) {
   DCHECK(render_frame_);
 }
 
 PdfViewWebPluginClient::~PdfViewWebPluginClient() = default;
 
+std::unique_ptr<base::Value> PdfViewWebPluginClient::FromV8Value(
+    v8::Local<v8::Value> value,
+    v8::Local<v8::Context> context) {
+  return v8_value_converter_->FromV8Value(value, context);
+}
+
+v8::Local<v8::Value> PdfViewWebPluginClient::ToV8Value(
+    const base::Value& value,
+    v8::Local<v8::Context> context) {
+  return v8_value_converter_->ToV8Value(&value, context);
+}
+
+base::WeakPtr<chrome_pdf::PdfViewWebPlugin::Client>
+PdfViewWebPluginClient::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
 void PdfViewWebPluginClient::Print(const blink::WebElement& element) {
   DCHECK(!element.IsNull());
 #if BUILDFLAG(ENABLE_PRINTING)

components/pdf/renderer/pdf_view_web_plugin_client.h

@@ -5,11 +5,15 @@
 #ifndef COMPONENTS_PDF_RENDERER_PDF_VIEW_WEB_PLUGIN_CLIENT_H_
 #define COMPONENTS_PDF_RENDERER_PDF_VIEW_WEB_PLUGIN_CLIENT_H_
 
+#include <memory>
+
+#include "base/memory/weak_ptr.h"
 #include "pdf/pdf_view_web_plugin.h"
 
 namespace content {
 class RenderFrame;
-}
+class V8ValueConverter;
+}  // namespace content
 
 namespace pdf {
 
@@ -21,6 +25,12 @@ class PdfViewWebPluginClient : public chrome_pdf::PdfViewWebPlugin::Client {
   ~PdfViewWebPluginClient() override;
 
   // chrome_pdf::PdfViewWebPlugin::Client:
+  std::unique_ptr<base::Value> FromV8Value(
+      v8::Local<v8::Value> value,
+      v8::Local<v8::Context> context) override;
+  v8::Local<v8::Value> ToV8Value(const base::Value& value,
+                                 v8::Local<v8::Context> context) override;
+  base::WeakPtr<chrome_pdf::PdfViewWebPlugin::Client> GetWeakPtr() override;
   void Print(const blink::WebElement& element) override;
   void RecordComputedAction(const std::string& action) override;
   std::unique_ptr<chrome_pdf::PdfAccessibilityDataHandler>
@@ -30,6 +40,10 @@ class PdfViewWebPluginClient : public chrome_pdf::PdfViewWebPlugin::Client {
 
  private:
   content::RenderFrame* const render_frame_;
+
+  const std::unique_ptr<content::V8ValueConverter> v8_value_converter_;
+
+  base::WeakPtrFactory<PdfViewWebPluginClient> weak_factory_{this};
 };
 
 }  // namespace pdf

pdf/BUILD.gn

@@ -350,6 +350,7 @@ if (enable_pdf) {
       "post_message_receiver.h",
       "post_message_sender.cc",
       "post_message_sender.h",
+      "v8_value_converter.h",
     ]
 
     configs += [ ":strict" ]
@@ -365,7 +366,6 @@ if (enable_pdf) {
       "//base",
       "//base:i18n",
       "//cc/paint",
-      "//content/public/renderer",
       "//gin",
       "//ppapi/cpp:objects",  # TODO(crbug.com/1114263): PDFEngine::Client only.
       "//printing",

pdf/DEPS

@@ -1,6 +1,5 @@
 include_rules = [
   "+cc/paint",
-  "+content/public/renderer/v8_value_converter.h",
   "+gin",
   "+mojo/core/embedder",
   "+mojo/public/cpp/bindings",

pdf/pdf_view_web_plugin.cc

@@ -292,6 +292,7 @@ PdfViewWebPlugin::PdfViewWebPlugin(
     : client_(std::move(client)),
       pdf_service_remote_(std::move(pdf_service_remote)),
       initial_params_(params),
+      post_message_sender_(client_.get()),
       pdf_accessibility_data_handler_(
           client_->CreateAccessibilityDataHandler(this)) {
   auto* service = GetPdfService();
@@ -395,9 +396,9 @@ v8::Local<v8::Object> PdfViewWebPlugin::V8ScriptableObject(
     // TODO(crbug.com/1123731): Messages should not be handled on the renderer
     // main thread.
     scriptable_receiver_.Reset(
-        isolate,
-        PostMessageReceiver::Create(isolate, weak_factory_.GetWeakPtr(),
-                                    base::SequencedTaskRunnerHandle::Get()));
+        isolate, PostMessageReceiver::Create(
+                     isolate, client_->GetWeakPtr(), weak_factory_.GetWeakPtr(),
+                     base::SequencedTaskRunnerHandle::Get()));
   }
 
   return scriptable_receiver_.Get(isolate);

pdf/pdf_view_web_plugin.h

@@ -23,6 +23,7 @@
 #include "pdf/post_message_sender.h"
 #include "pdf/ppapi_migration/graphics.h"
 #include "pdf/ppapi_migration/url_loader.h"
+#include "pdf/v8_value_converter.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_text_input_type.h"
 #include "third_party/blink/public/web/web_plugin.h"
@@ -142,10 +143,12 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   };
 
   // Allows for dependency injections into `PdfViewWebPlugin`.
-  class Client {
+  class Client : public V8ValueConverter {
    public:
     virtual ~Client() = default;
 
+    virtual base::WeakPtr<Client> GetWeakPtr() = 0;
+
     // Prints the given `element`.
     virtual void Print(const blink::WebElement& element) {}
 

pdf/pdf_view_web_plugin_unittest.cc

@@ -11,6 +11,7 @@
 
 #include "base/location.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/string_piece.h"
 #include "base/test/bind.h"
@@ -242,7 +243,19 @@ class FakePdfViewWebPluginClient : public PdfViewWebPlugin::Client {
   ~FakePdfViewWebPluginClient() override = default;
 
   // PdfViewWebPlugin::Client:
+  MOCK_METHOD(std::unique_ptr<base::Value>,
+              FromV8Value,
+              (v8::Local<v8::Value>, v8::Local<v8::Context>),
+              (override));
+  MOCK_METHOD(v8::Local<v8::Value>,
+              ToV8Value,
+              (const base::Value&, v8::Local<v8::Context>),
+              (override));
+  MOCK_METHOD(base::WeakPtr<Client>, GetWeakPtr, (), (override));
   MOCK_METHOD(bool, IsUseZoomForDSFEnabled, (), (const, override));
+
+ private:
+  base::WeakPtrFactory<FakePdfViewWebPluginClient> weak_factory_{this};
 };
 
 }  // namespace

pdf/post_message_receiver.cc

@@ -16,13 +16,13 @@
 #include "base/memory/weak_ptr.h"
 #include "base/task/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"
+#include "pdf/v8_value_converter.h"
 #include "v8/include/v8.h"
 
 namespace chrome_pdf {
@@ -39,11 +39,13 @@ gin::WrapperInfo PostMessageReceiver::kWrapperInfo = {gin::kEmbedderNativeGin};
 // static
 v8::Local<v8::Object> PostMessageReceiver::Create(
     v8::Isolate* isolate,
+    base::WeakPtr<V8ValueConverter> v8_value_converter,
     base::WeakPtr<Client> client,
     scoped_refptr<base::SequencedTaskRunner> client_task_runner) {
   return gin::CreateHandle(
-             isolate, new PostMessageReceiver(isolate, std::move(client),
-                                              std::move(client_task_runner)))
+             isolate, new PostMessageReceiver(
+                          isolate, std::move(v8_value_converter),
+                          std::move(client), std::move(client_task_runner)))
       .ToV8()
       .As<v8::Object>();
 }
@@ -52,9 +54,11 @@ PostMessageReceiver::~PostMessageReceiver() = default;
 
 PostMessageReceiver::PostMessageReceiver(
     v8::Isolate* isolate,
+    base::WeakPtr<V8ValueConverter> v8_value_converter,
     base::WeakPtr<Client> client,
     scoped_refptr<base::SequencedTaskRunner> client_task_runner)
     : gin::NamedPropertyInterceptor(isolate, this),
+      v8_value_converter_(std::move(v8_value_converter)),
       isolate_(isolate),
       client_(std::move(client)),
       client_task_runner_(std::move(client_task_runner)) {}
@@ -113,20 +117,12 @@ v8::Local<v8::FunctionTemplate> PostMessageReceiver::GetFunctionTemplate() {
   return function_template_.Get(isolate_);
 }
 
-std::unique_ptr<base::Value> PostMessageReceiver::ConvertMessage(
-    v8::Local<v8::Value> message) {
-  if (!v8_value_converter_)
-    v8_value_converter_ = content::V8ValueConverter::Create();
-
-  return v8_value_converter_->FromV8Value(message,
-                                          isolate_->GetCurrentContext());
-}
-
 void PostMessageReceiver::PostMessage(v8::Local<v8::Value> message) {
-  if (!client_)
+  if (!client_ || !v8_value_converter_)
     return;
 
-  std::unique_ptr<base::Value> converted_message = ConvertMessage(message);
+  std::unique_ptr<base::Value> converted_message =
+      v8_value_converter_->FromV8Value(message, isolate_->GetCurrentContext());
   DCHECK(converted_message) << "The PDF Viewer UI should not be sending "
                                "messages that cannot be converted.";
 

pdf/post_message_receiver.h

@@ -20,16 +20,14 @@ class SequencedTaskRunner;
 class Value;
 }  // namespace base
 
-namespace content {
-class V8ValueConverter;
-}  // namespace content
-
 namespace gin {
 class ObjectTemplateBuilder;
 }  // namespace gin
 
 namespace chrome_pdf {
 
+class V8ValueConverter;
+
 // Implements the `postMessage()` API exposed to the plugin embedder. The
 // received messages are converted and forwarded to the `Client`.
 // `PostMessageReceiver`'s lifetime is managed by the V8 garbage collector,
@@ -55,6 +53,7 @@ class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver>,
   // Messages are posted asynchronously to `client` using `client_task_runner`.
   static v8::Local<v8::Object> Create(
       v8::Isolate* isolate,
+      base::WeakPtr<V8ValueConverter> v8_value_converter,
       base::WeakPtr<Client> client,
       scoped_refptr<base::SequencedTaskRunner> client_task_runner);
 
@@ -67,6 +66,7 @@ class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver>,
  private:
   PostMessageReceiver(
       v8::Isolate* isolate,
+      base::WeakPtr<V8ValueConverter> v8_value_converter,
       base::WeakPtr<Client> client,
       scoped_refptr<base::SequencedTaskRunner> client_task_runner);
 
@@ -84,13 +84,10 @@ class PostMessageReceiver final : public gin::Wrappable<PostMessageReceiver>,
   // 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);
-
   // Implements the `postMessage()` method called by the embedder.
   void PostMessage(v8::Local<v8::Value> message);
 
-  std::unique_ptr<content::V8ValueConverter> v8_value_converter_;
+  base::WeakPtr<V8ValueConverter> v8_value_converter_;
 
   v8::Persistent<v8::FunctionTemplate> function_template_;
 

pdf/post_message_sender.cc

@@ -8,7 +8,7 @@
 
 #include "base/check_op.h"
 #include "base/values.h"
-#include "content/public/renderer/v8_value_converter.h"
+#include "pdf/v8_value_converter.h"
 #include "third_party/blink/public/web/blink.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_dom_message_event.h"
@@ -19,7 +19,9 @@
 
 namespace chrome_pdf {
 
-PostMessageSender::PostMessageSender() : isolate_(blink::MainThreadIsolate()) {}
+PostMessageSender::PostMessageSender(V8ValueConverter* v8_value_converter)
+    : v8_value_converter_(v8_value_converter),
+      isolate_(blink::MainThreadIsolate()) {}
 
 PostMessageSender::~PostMessageSender() = default;
 
@@ -40,11 +42,8 @@ void PostMessageSender::Post(base::Value message) {
   DCHECK_EQ(isolate_, context->GetIsolate());
   v8::Context::Scope context_scope(context);
 
-  if (!v8_value_converter_)
-    v8_value_converter_ = content::V8ValueConverter::Create();
-
   v8::Local<v8::Value> converted_message =
-      v8_value_converter_->ToV8Value(&message, context);
+      v8_value_converter_->ToV8Value(message, context);
 
   container_->EnqueueMessageEvent(
       blink::WebSerializedScriptValue::Serialize(isolate_, converted_message));

pdf/post_message_sender.h

@@ -5,9 +5,8 @@
 #ifndef PDF_POST_MESSAGE_SENDER_H_
 #define PDF_POST_MESSAGE_SENDER_H_
 
-#include <memory>
-
 #include "base/memory/raw_ptr.h"
+#include "v8/include/v8-forward.h"
 
 namespace base {
 class Value;
@@ -17,20 +16,14 @@ namespace blink {
 class WebPluginContainer;
 }  // namespace blink
 
-namespace content {
-class V8ValueConverter;
-}  // namespace content
-
-namespace v8 {
-class Isolate;
-}  // namespace v8
-
 namespace chrome_pdf {
 
+class V8ValueConverter;
+
 // Manages messages sent from the plugin to its embedder.
 class PostMessageSender final {
  public:
-  PostMessageSender();
+  explicit PostMessageSender(V8ValueConverter* v8_value_converter);
   PostMessageSender(const PostMessageSender&) = delete;
   PostMessageSender& operator=(const PostMessageSender&) = delete;
   ~PostMessageSender();
@@ -47,11 +40,11 @@ class PostMessageSender final {
   }
 
  private:
-  std::unique_ptr<content::V8ValueConverter> v8_value_converter_;
+  const raw_ptr<V8ValueConverter> v8_value_converter_;
 
-  raw_ptr<v8::Isolate> isolate_;
+  const raw_ptr<v8::Isolate> isolate_;
 
-  raw_ptr<blink::WebPluginContainer> container_ = nullptr;
+  raw_ptr<blink::WebPluginContainer> container_;
 };
 
 }  // namespace chrome_pdf

pdf/v8_value_converter.h

@@ -0,0 +1,37 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PDF_V8_VALUE_CONVERTER_H_
+#define PDF_V8_VALUE_CONVERTER_H_
+
+#include <memory>
+
+#include "v8/include/v8-forward.h"
+
+namespace base {
+class Value;
+}  // namespace base
+
+namespace chrome_pdf {
+
+// Abstraction layer for base::Value from/to V8::Value conversions.
+// Necessary because //pdf should not directly depend on //content. If the V8
+// V8 value conversion code ever moves into Blink, remove this and use the
+// conversion code from Blink directly.
+
+class V8ValueConverter {
+ public:
+  virtual std::unique_ptr<base::Value> FromV8Value(
+      v8::Local<v8::Value> value,
+      v8::Local<v8::Context> context) = 0;
+  virtual v8::Local<v8::Value> ToV8Value(const base::Value& value,
+                                         v8::Local<v8::Context> context) = 0;
+
+ protected:
+  ~V8ValueConverter() = default;
+};
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_V8_VALUE_CONVERTER_H_