Cleanup PDF plugin code.

- Consolidate callback factories in OutOfProcessInstance.
- Make PDFEngine::Create() return a unique_ptr.
- Unify LoadUrl methods.

Review-Url: https://codereview.chromium.org/2837663002
Cr-Commit-Position: refs/heads/master@{#469377}

pdf/out_of_process_instance.cc

@@ -299,11 +299,8 @@ OutOfProcessInstance::OutOfProcessInstance(PP_Instance instance)
       top_toolbar_height_(0),
       accessibility_state_(ACCESSIBILITY_STATE_OFF),
       is_print_preview_(false) {
-  loader_factory_.Initialize(this);
-  timer_factory_.Initialize(this);
-  form_factory_.Initialize(this);
-  print_callback_factory_.Initialize(this);
-  engine_.reset(PDFEngine::Create(this));
+  callback_factory_.Initialize(this);
+  engine_ = PDFEngine::Create(this);
   pp::Module::Get()->AddPluginInterface(kPPPPdfInterface, &ppp_private);
   AddPerInstanceObject(kPPPPdfInterface, this);
 
@@ -384,7 +381,7 @@ bool OutOfProcessInstance::Init(uint32_t argc,
   if (IsPrintPreview())
     return true;
 
-  LoadUrl(stream_url);
+  LoadUrl(stream_url, /*is_print_preview=*/false);
   url_ = original_url;
   pp::PDF::SetCrashData(GetPluginInstance(), original_url, top_level_url);
   return engine_->New(original_url, headers);
@@ -535,9 +532,9 @@ void OutOfProcessInstance::HandleMessage(const pp::Var& message) {
     preview_pages_info_ = std::queue<PreviewPageInfo>();
     preview_document_load_state_ = LOAD_STATE_COMPLETE;
     document_load_state_ = LOAD_STATE_LOADING;
-    LoadUrl(url_);
+    LoadUrl(url_, /*is_print_preview=*/false);
     preview_engine_.reset();
-    engine_.reset(PDFEngine::Create(this));
+    engine_ = PDFEngine::Create(this);
     engine_->SetGrayscale(dict.Get(pp::Var(kJSPrintPreviewGrayscale)).AsBool());
     engine_->New(url_.c_str(), nullptr /* empty header */);
 
@@ -718,7 +715,7 @@ void OutOfProcessInstance::LoadAccessibility() {
   SendAccessibilityViewportInfo();
 
   // Schedule loading the first page.
-  pp::CompletionCallback callback = timer_factory_.NewCallback(
+  pp::CompletionCallback callback = callback_factory_.NewCallback(
       &OutOfProcessInstance::SendNextAccessibilityPage);
   pp::Module::Get()->core()->CallOnMainThread(kAccessibilityPageDelayMs,
                                               callback, 0);
@@ -784,7 +781,7 @@ void OutOfProcessInstance::SendNextAccessibilityPage(int32_t page_index) {
                                     text_runs.data(), chars.data());
 
   // Schedule loading the next page.
-  pp::CompletionCallback callback = timer_factory_.NewCallback(
+  pp::CompletionCallback callback = callback_factory_.NewCallback(
       &OutOfProcessInstance::SendNextAccessibilityPage);
   pp::Module::Get()->core()->CallOnMainThread(kAccessibilityPageDelayMs,
                                               callback, page_index + 1);
@@ -958,7 +955,7 @@ void OutOfProcessInstance::DidOpen(int32_t result) {
 void OutOfProcessInstance::DidOpenPreview(int32_t result) {
   if (result == PP_OK) {
     preview_client_ = base::MakeUnique<PreviewModeClient>(this);
-    preview_engine_.reset(PDFEngine::Create(preview_client_.get()));
+    preview_engine_ = PDFEngine::Create(preview_client_.get());
     preview_engine_->HandleDocumentLoad(embed_preview_loader_);
   } else {
     NOTREACHED();
@@ -1126,7 +1123,7 @@ void OutOfProcessInstance::NotifyNumberOfFindResultsChanged(int total,
   NumberOfFindResultsChanged(total, final_result);
   SetTickmarks(tickmarks_);
   recently_sent_find_update_ = true;
-  pp::CompletionCallback callback = timer_factory_.NewCallback(
+  pp::CompletionCallback callback = callback_factory_.NewCallback(
       &OutOfProcessInstance::ResetRecentlySentFindUpdate);
   pp::Module::Get()->core()->CallOnMainThread(kFindResultCooldownMs, callback,
                                               0);
@@ -1198,7 +1195,7 @@ void OutOfProcessInstance::Print() {
   }
 
   pp::CompletionCallback callback =
-      print_callback_factory_.NewCallback(&OutOfProcessInstance::OnPrint);
+      callback_factory_.NewCallback(&OutOfProcessInstance::OnPrint);
   pp::Module::Get()->core()->CallOnMainThread(0, callback);
 }
 
@@ -1215,7 +1212,7 @@ void OutOfProcessInstance::SubmitForm(const std::string& url,
   request.AppendDataToBody(reinterpret_cast<const char*>(data), length);
 
   pp::CompletionCallback callback =
-      form_factory_.NewCallback(&OutOfProcessInstance::FormDidOpen);
+      callback_factory_.NewCallback(&OutOfProcessInstance::FormDidOpen);
   form_loader_ = CreateURLLoaderInternal();
   int rv = form_loader_.Open(request, callback);
   if (rv != PP_OK_COMPLETIONPENDING)
@@ -1255,7 +1252,7 @@ pp::URLLoader OutOfProcessInstance::CreateURLLoader() {
 
 void OutOfProcessInstance::ScheduleCallback(int id, int delay_in_ms) {
   pp::CompletionCallback callback =
-      timer_factory_.NewCallback(&OutOfProcessInstance::OnClientTimerFired);
+      callback_factory_.NewCallback(&OutOfProcessInstance::OnClientTimerFired);
   pp::Module::Get()->core()->CallOnMainThread(delay_in_ms, callback, id);
 }
 
@@ -1364,15 +1361,15 @@ void OutOfProcessInstance::PreviewDocumentLoadComplete() {
 
   preview_document_load_state_ = LOAD_STATE_COMPLETE;
 
+  const std::string& url = preview_pages_info_.front().first;
   int dest_page_index = preview_pages_info_.front().second;
-  int src_page_index =
-      ExtractPrintPreviewPageIndex(preview_pages_info_.front().first);
-  if (src_page_index > 0 && dest_page_index > -1 && preview_engine_.get())
+  int src_page_index = ExtractPrintPreviewPageIndex(url);
+  if (src_page_index > 0 && dest_page_index > -1 && preview_engine_)
     engine_->AppendPage(preview_engine_.get(), dest_page_index);
 
   preview_pages_info_.pop();
   // |print_preview_page_count_| is not updated yet. Do not load any
-  // other preview pages till we get this information.
+  // other preview pages until this information is available.
   if (print_preview_page_count_ == 0)
     return;
 
@@ -1472,7 +1469,7 @@ void OutOfProcessInstance::DocumentLoadProgress(uint32_t available,
 }
 
 void OutOfProcessInstance::FormTextFieldFocusChange(bool in_focus) {
-  if (!text_input_.get())
+  if (!text_input_)
     return;
 
   pp::VarDictionary message;
@@ -1516,26 +1513,19 @@ void OutOfProcessInstance::OnGeometryChanged(double old_zoom,
     SendAccessibilityViewportInfo();
 }
 
-void OutOfProcessInstance::LoadUrl(const std::string& url) {
-  LoadUrlInternal(url, &embed_loader_, &OutOfProcessInstance::DidOpen);
-}
-
-void OutOfProcessInstance::LoadPreviewUrl(const std::string& url) {
-  LoadUrlInternal(url, &embed_preview_loader_,
-                  &OutOfProcessInstance::DidOpenPreview);
-}
-
-void OutOfProcessInstance::LoadUrlInternal(
-    const std::string& url,
-    pp::URLLoader* loader,
-    void (OutOfProcessInstance::*method)(int32_t)) {
+void OutOfProcessInstance::LoadUrl(const std::string& url,
+                                   bool is_print_preview) {
   pp::URLRequestInfo request(this);
   request.SetURL(url);
   request.SetMethod("GET");
   request.SetFollowRedirects(false);
 
+  pp::URLLoader* loader =
+      is_print_preview ? &embed_preview_loader_ : &embed_loader_;
   *loader = CreateURLLoaderInternal();
-  pp::CompletionCallback callback = loader_factory_.NewCallback(method);
+  pp::CompletionCallback callback = callback_factory_.NewCallback(
+      is_print_preview ? &OutOfProcessInstance::DidOpenPreview
+                       : &OutOfProcessInstance::DidOpen);
   int rv = loader->Open(request, callback);
   if (rv != PP_OK_COMPLETIONPENDING)
     callback.Run(rv);
@@ -1600,7 +1590,7 @@ void OutOfProcessInstance::LoadAvailablePreviewPage() {
     return;
   }
 
-  std::string url = preview_pages_info_.front().first;
+  const std::string& url = preview_pages_info_.front().first;
   int dst_page_index = preview_pages_info_.front().second;
   int src_page_index = ExtractPrintPreviewPageIndex(url);
   if (src_page_index < 1 || dst_page_index >= print_preview_page_count_ ||
@@ -1609,7 +1599,7 @@ void OutOfProcessInstance::LoadAvailablePreviewPage() {
   }
 
   preview_document_load_state_ = LOAD_STATE_LOADING;
-  LoadPreviewUrl(url);
+  LoadUrl(url, /*is_print_preview=*/true);
 }
 
 void OutOfProcessInstance::UserMetricsRecordAction(const std::string& action) {

pdf/out_of_process_instance.h

@@ -163,11 +163,7 @@ class OutOfProcessInstance : public pp::Instance,
   // Draws a rectangle with the specified dimensions and color in our buffer.
   void FillRect(const pp::Rect& rect, uint32_t color);
 
-  void LoadUrl(const std::string& url);
-  void LoadPreviewUrl(const std::string& url);
-  void LoadUrlInternal(const std::string& url,
-                       pp::URLLoader* loader,
-                       void (OutOfProcessInstance::*method)(int32_t));
+  void LoadUrl(const std::string& url, bool is_print_preview);
 
   // Creates a URL loader and allows it to access all urls, i.e. not just the
   // frame's origin.
@@ -217,14 +213,11 @@ class OutOfProcessInstance : public pp::Instance,
   pp::ImageData image_data_;
   // Used when the plugin is embedded in a page and we have to create the loader
   // ourself.
-  pp::CompletionCallbackFactory<OutOfProcessInstance> loader_factory_;
   pp::URLLoader embed_loader_;
   pp::URLLoader embed_preview_loader_;
 
   PP_CursorType_Dev cursor_;  // The current cursor.
 
-  pp::CompletionCallbackFactory<OutOfProcessInstance> timer_factory_;
-
   // Size, in pixels, of plugin rectangle.
   pp::Size plugin_size_;
   // Size, in DIPs, of plugin rectangle.
@@ -301,11 +294,9 @@ class OutOfProcessInstance : public pp::Instance,
   std::string url_;
 
   // Used for submitting forms.
-  pp::CompletionCallbackFactory<OutOfProcessInstance> form_factory_;
   pp::URLLoader form_loader_;
 
-  // Used for printing without re-entrancy issues.
-  pp::CompletionCallbackFactory<OutOfProcessInstance> print_callback_factory_;
+  pp::CompletionCallbackFactory<OutOfProcessInstance> callback_factory_;
 
   // The callback for receiving the password from the page.
   std::unique_ptr<pp::CompletionCallbackWithOutput<pp::Var>> password_callback_;

pdf/pdf_engine.h

@@ -13,6 +13,7 @@
 #include <windows.h>
 #endif
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -189,7 +190,7 @@ class PDFEngine {
   };
 
   // Factory method to create an instance of the PDF Engine.
-  static PDFEngine* Create(Client* client);
+  static std::unique_ptr<PDFEngine> Create(Client* client);
 
   virtual ~PDFEngine() {}
 

pdf/pdfium/pdfium_engine.cc

@@ -659,8 +659,8 @@ void ShutdownSDK() {
   TearDownV8();
 }
 
-PDFEngine* PDFEngine::Create(PDFEngine::Client* client) {
-  return new PDFiumEngine(client);
+std::unique_ptr<PDFEngine> PDFEngine::Create(PDFEngine::Client* client) {
+  return base::MakeUnique<PDFiumEngine>(client);
 }
 
 PDFiumEngine::PDFiumEngine(PDFEngine::Client* client)