Cleanup PDF plugin code.

pdf/out_of_process_instance.cc

Index: pdf/out_of_process_instance.cc
diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
index 2f80e2eb358aa55fb9277caf2796ffed3159fc9d..3431f6a811849a88e2cae3abc377973f1619761f 100644
--- a/pdf/out_of_process_instance.cc
+++ b/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);
 
@@ -385,7 +382,7 @@ bool OutOfProcessInstance::Init(uint32_t argc,
   if (IsPrintPreview())
     return true;
 
-  LoadUrl(stream_url);
+  LoadUrl(stream_url, false /* is_print_preview */);

[raymes, 2017/04/26 02:44:19]

I just had advice from thakis@ that /*is_print_preview=*/ is now preferred
because clang can check it.

[Lei Zhang, 2017/05/04 17:39:28]

Done.

   url_ = original_url;
   pp::PDF::SetCrashData(GetPluginInstance(), original_url, top_level_url);
   return engine_->New(original_url, headers);
@@ -536,9 +533,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_, false /* is_print_preview */);
     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 */);
 
@@ -719,7 +716,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);
@@ -785,7 +782,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);
@@ -959,7 +956,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();
@@ -1127,7 +1124,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);
@@ -1199,7 +1196,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);
 }
 
@@ -1216,7 +1213,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)
@@ -1256,7 +1253,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);
 }
 
@@ -1365,15 +1362,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;
 
@@ -1473,7 +1470,7 @@ void OutOfProcessInstance::DocumentLoadProgress(uint32_t available,
 }
 
 void OutOfProcessInstance::FormTextFieldFocusChange(bool in_focus) {
-  if (!text_input_.get())
+  if (!text_input_)
     return;
 
   pp::VarDictionary message;
@@ -1517,27 +1514,20 @@ 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);
 
-  *loader = CreateURLLoaderInternal();
-  pp::CompletionCallback callback = loader_factory_.NewCallback(method);
-  int rv = loader->Open(request, callback);
+  pp::URLLoader& loader =
+      is_print_preview ? embed_preview_loader_ : embed_loader_;

[raymes, 2017/04/26 02:44:19]

This is a bit subtle to me. Maybe it's just because we don't use non-const
references much in Chrome though. 

[Lei Zhang, 2017/05/04 17:39:28]

Back to pointers.

+  loader = CreateURLLoaderInternal();
+  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);
 }
@@ -1601,7 +1591,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_ ||
@@ -1610,7 +1600,7 @@ void OutOfProcessInstance::LoadAvailablePreviewPage() {
   }
 
   preview_document_load_state_ = LOAD_STATE_LOADING;
-  LoadPreviewUrl(url);
+  LoadUrl(url, true /* is_print_preview */);
 }
 
 void OutOfProcessInstance::UserMetricsRecordAction(const std::string& action) {