Share PDFiumEngine creation between PDF plugins

Consolidates code for creating (and destroying) the main PDFiumEngine in
a common base class, PdfViewPluginBase. This base class can be used for
any code which is not specific to the Pepper (OutOfProcessInstance) or
Pepper-free (PdfViewWebPlugin) plugin implementations.

Note that this approach involves multiple inheritance of implementation,
but I believe this is a reasonable trade-off during the migration.

Bug: 1085334
Change-Id: I5b17141972a371e70cf0e6801d7dad2cb0d20aae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343573
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796648}

pdf/BUILD.gn

@@ -98,6 +98,8 @@ if (enable_pdf) {
       "pdf_init.h",
       "pdf_transform.cc",
       "pdf_transform.h",
+      "pdf_view_plugin_base.cc",
+      "pdf_view_plugin_base.h",
       "pdfium/pdfium_api_string_buffer_adapter.cc",
       "pdfium/pdfium_api_string_buffer_adapter.h",
       "pdfium/pdfium_document.cc",

pdf/out_of_process_instance.h

@@ -18,7 +18,7 @@
 #include "base/containers/queue.h"
 #include "base/macros.h"
 #include "pdf/paint_manager.h"
-#include "pdf/pdf_engine.h"
+#include "pdf/pdf_view_plugin_base.h"
 #include "pdf/preview_mode_client.h"
 #include "ppapi/c/private/ppp_pdf.h"
 #include "ppapi/cpp/dev/printing_dev.h"
@@ -46,11 +46,11 @@ class Graphics;
 class PaintReadyRect;
 class PDFiumEngine;
 
-class OutOfProcessInstance : public pp::Instance,
+class OutOfProcessInstance : public PdfViewPluginBase,
+                             public pp::Instance,
                              public pp::Find_Private,
                              public pp::Printing_Dev,
                              public PaintManager::Client,
-                             public PDFEngine::Client,
                              public PreviewModeClient::Client {
  public:
   explicit OutOfProcessInstance(PP_Instance instance);
@@ -107,7 +107,7 @@ class OutOfProcessInstance : public pp::Instance,
   void DidOpen(int32_t result);
   void DidOpenPreview(int32_t result);
 
-  // PDFEngine::Client implementation.
+  // PdfViewPluginBase implementation.
   void ProposeDocumentLayout(const DocumentLayout& layout) override;
   void Invalidate(const pp::Rect& rect) override;
   void DidScroll(const gfx::Vector2d& offset) override;
@@ -408,8 +408,6 @@ class OutOfProcessInstance : public pp::Instance,
 
   PrintSettings print_settings_;
 
-  std::unique_ptr<PDFiumEngine> engine_;
-
   // The PreviewModeClient used for print preview. Will be passed to
   // |preview_engine_|.
   std::unique_ptr<PreviewModeClient> preview_client_;

pdf/pdf_view_plugin_base.cc

+// Copyright 2020 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.
+
+#include "pdf/pdf_view_plugin_base.h"
+
+#include <memory>
+
+#include "pdf/pdfium/pdfium_engine.h"
+
+namespace chrome_pdf {
+
+PdfViewPluginBase::PdfViewPluginBase() = default;
+
+PdfViewPluginBase::~PdfViewPluginBase() = default;
+
+void PdfViewPluginBase::InitializeEngine(bool enable_javascript) {
+  engine_ = std::make_unique<PDFiumEngine>(this, enable_javascript);
+}
+
+void PdfViewPluginBase::DestroyEngine() {
+  engine_.reset();
+}
+
+}  // namespace chrome_pdf

pdf/pdf_view_plugin_base.h

+// Copyright 2020 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_PDF_VIEW_PLUGIN_BASE_H_
+#define PDF_PDF_VIEW_PLUGIN_BASE_H_
+
+#include "pdf/pdf_engine.h"
+
+#include <memory>
+
+namespace chrome_pdf {
+
+class PDFiumEngine;
+
+// Common base to share code between the two plugin implementations,
+// `OutOfProcessInstance` (Pepper) and `PdfViewWebPlugin` (Blink).
+class PdfViewPluginBase : public PDFEngine::Client {
+ public:
+  PdfViewPluginBase(const PdfViewPluginBase& other) = delete;
+  PdfViewPluginBase& operator=(const PdfViewPluginBase& other) = delete;
+
+ protected:
+  PdfViewPluginBase();
+  ~PdfViewPluginBase() override;
+
+  // Initializes the main `PDFiumEngine`. If `enable_javascript` is true, the
+  // engine will support executing JavaScript.
+  //
+  // Any existing engine will be replaced.
+  void InitializeEngine(bool enable_javascript);
+
+  // Destroys the main `PDFiumEngine`. Subclasses should call this method in
+  // their destructor to ensure the engine is destroyed first.
+  void DestroyEngine();
+
+  PDFiumEngine* engine() { return engine_.get(); }
+
+ private:
+  std::unique_ptr<PDFiumEngine> engine_;
+};
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_PDF_VIEW_PLUGIN_BASE_H_

pdf/pdf_view_web_plugin.cc

@@ -28,11 +28,16 @@ namespace chrome_pdf {
 
 PdfViewWebPlugin::PdfViewWebPlugin(const blink::WebPluginParams& params) {}
 
-PdfViewWebPlugin::~PdfViewWebPlugin() = default;
+PdfViewWebPlugin::~PdfViewWebPlugin() {
+  // Explicitly destroy the PDFEngine during destruction as it may call back
+  // into this object.
+  DestroyEngine();
+}
 
 bool PdfViewWebPlugin::Initialize(blink::WebPluginContainer* container) {
   DCHECK_EQ(container->Plugin(), this);
   container_ = container;
+  InitializeEngine(/*enable_javascript=*/false);
   return true;
 }
 

pdf/pdf_view_web_plugin.h

@@ -5,7 +5,7 @@
 #ifndef PDF_PDF_VIEW_WEB_PLUGIN_H_
 #define PDF_PDF_VIEW_WEB_PLUGIN_H_
 
-#include "pdf/pdf_engine.h"
+#include "pdf/pdf_view_plugin_base.h"
 #include "third_party/blink/public/web/web_plugin.h"
 
 namespace blink {
@@ -16,8 +16,8 @@ struct WebPluginParams;
 namespace chrome_pdf {
 
 // Skeleton for a `blink::WebPlugin` to replace `OutOfProcessInstance`.
-class PdfViewWebPlugin final : public blink::WebPlugin,
-                               public PDFEngine::Client {
+class PdfViewWebPlugin final : public PdfViewPluginBase,
+                               public blink::WebPlugin {
  public:
   explicit PdfViewWebPlugin(const blink::WebPluginParams& params);
   PdfViewWebPlugin(const PdfViewWebPlugin& other) = delete;
@@ -43,7 +43,7 @@ class PdfViewWebPlugin final : public blink::WebPlugin,
   void DidFinishLoading() override;
   void DidFailLoading(const blink::WebURLError& error) override;
 
-  // PDFEngine::Client:
+  // PdfViewPluginBase:
   void ProposeDocumentLayout(const DocumentLayout& layout) override;
   void Invalidate(const pp::Rect& rect) override;
   void DidScroll(const gfx::Vector2d& offset) override;