Refactor print_job.cc

chrome/browser/printing/print_job.cc

Index: chrome/browser/printing/print_job.cc
diff --git a/chrome/browser/printing/print_job.cc b/chrome/browser/printing/print_job.cc
index fc9b9a9a9dbfa35b38408630d1a584916c24ee43..820e57579b917ce84723b22154df4d695ecc97f9 100644
--- a/chrome/browser/printing/print_job.cc
+++ b/chrome/browser/printing/print_job.cc
@@ -222,16 +222,14 @@ PrintedDocument* PrintJob::document() const {
 }
 
 #if defined(OS_WIN)
-
-class PrintJob::PdfToEmfState {
+class PrintJob::PdfConversionState {
  public:
-  PdfToEmfState(const gfx::Size& page_size, const gfx::Rect& content_area)
+  PdfConversionState()
       : page_count_(0),
         current_page_(0),
         pages_in_progress_(0),
-        page_size_(page_size),
-        content_area_(content_area),
-        converter_(PdfConverter::CreatePdfToEmfConverter()) {}
+        page_size_(gfx::Size()),
+        content_area_(gfx::Rect()) {}
 
   void Start(const scoped_refptr<base::RefCountedMemory>& data,
              const PdfRenderSettings& conversion_settings,
@@ -259,10 +257,10 @@ class PrintJob::PdfToEmfState {
   }
 
   void set_page_count(int page_count) { page_count_ = page_count; }
-  gfx::Size page_size() const { return page_size_; }
-  gfx::Rect content_area() const { return content_area_; }
+  const gfx::Size& page_size() const { return page_size_; }
+  const gfx::Rect& content_area() const { return content_area_; }
 
- private:
+ protected:

[Vitaly Buka (NO REVIEWS), 2017/01/20 08:36:47]

I don't reason for exception here
https://google.github.io/styleguide/cppguide.html#Access_Control

[rbpotter, 2017/01/20 16:51:32]

Done.

   int page_count_;
   int current_page_;
   int pages_in_progress_;
@@ -271,6 +269,15 @@ class PrintJob::PdfToEmfState {
   std::unique_ptr<PdfConverter> converter_;
 };
 
+class PrintJob::PdfToEmfState : public PrintJob::PdfConversionState {
+ public:
+  PdfToEmfState(const gfx::Size& page_size, const gfx::Rect& content_area) {
+    page_size_ = page_size;

[Vitaly Buka (NO REVIEWS), 2017/01/20 08:36:47]

Why not to just remove this class and 
use something like
  new PrintJob::PdfConversionState(size, area,
PdfConverter::CreatePdfToEmfConverter())

[rbpotter, 2017/01/20 16:51:32]

Done.

+    content_area_ = content_area;
+    converter_.reset(PdfConverter::CreatePdfToEmfConverter().release());
+  }
+};
+
 void PrintJob::AppendPrintedPage(int page_number) {
   pdf_page_mapping_.push_back(page_number);
 }
@@ -280,46 +287,47 @@ void PrintJob::StartPdfToEmfConversion(
     const gfx::Size& page_size,
     const gfx::Rect& content_area,
     bool print_text_with_gdi) {
-  DCHECK(!pdf_to_emf_state_);
-  pdf_to_emf_state_ = base::MakeUnique<PdfToEmfState>(page_size, content_area);
+  DCHECK(!pdf_conversion_state_);
+  pdf_conversion_state_ =
+      base::MakeUnique<PdfToEmfState>(page_size, content_area);
   const int kPrinterDpi = settings().dpi();
-  pdf_to_emf_state_->Start(
-      bytes, PdfRenderSettings(content_area, kPrinterDpi, true),
-      print_text_with_gdi, base::Bind(&PrintJob::OnPdfToEmfStarted, this));
+  PdfRenderSettings settings(content_area, kPrinterDpi, true /* autorotate? */);
+  pdf_conversion_state_->Start(
+      bytes, settings, print_text_with_gdi,
+      base::Bind(&PrintJob::OnPdfConversionStarted, this));
 }
 
-void PrintJob::OnPdfToEmfStarted(int page_count) {
+void PrintJob::OnPdfConversionStarted(int page_count) {
   if (page_count <= 0) {
-    pdf_to_emf_state_.reset();
+    pdf_conversion_state_.reset();
     Cancel();
     return;
   }
-  pdf_to_emf_state_->set_page_count(page_count);
-  pdf_to_emf_state_->GetMorePages(
-      base::Bind(&PrintJob::OnPdfToEmfPageConverted, this));
+  pdf_conversion_state_->set_page_count(page_count);
+  pdf_conversion_state_->GetMorePages(
+      base::Bind(&PrintJob::OnPdfPageConverted, this));
 }
 
-void PrintJob::OnPdfToEmfPageConverted(int page_number,
-                                       float scale_factor,
-                                       std::unique_ptr<MetafilePlayer> emf) {
-  DCHECK(pdf_to_emf_state_);
-  if (!document_.get() || !emf || page_number < 0 ||
+void PrintJob::OnPdfPageConverted(int page_number,
+                                  float scale_factor,
+                                  std::unique_ptr<MetafilePlayer> metafile) {
+  DCHECK(pdf_conversion_state_);
+  if (!document_.get() || !metafile || page_number < 0 ||
       static_cast<size_t>(page_number) >= pdf_page_mapping_.size()) {
-    pdf_to_emf_state_.reset();
+    pdf_conversion_state_.reset();
     Cancel();
     return;
   }
 
   // Update the rendered document. It will send notifications to the listener.
-  document_->SetPage(pdf_page_mapping_[page_number], std::move(emf),
-                     scale_factor, pdf_to_emf_state_->page_size(),
-                     pdf_to_emf_state_->content_area());
+  document_->SetPage(pdf_page_mapping_[page_number], std::move(metafile),
+                     scale_factor, pdf_conversion_state_->page_size(),
+                     pdf_conversion_state_->content_area());
 
-  pdf_to_emf_state_->GetMorePages(
-      base::Bind(&PrintJob::OnPdfToEmfPageConverted, this));
+  pdf_conversion_state_->GetMorePages(
+      base::Bind(&PrintJob::OnPdfPageConverted, this));
 }
-
-#endif  // OS_WIN
+#endif  // defined(OS_WIN)
 
 void PrintJob::UpdatePrintedDocument(PrintedDocument* new_document) {
   if (document_.get() == new_document)
@@ -370,8 +378,10 @@ void PrintJob::OnNotifyPrintJobEvent(const JobEventDetails& event_details) {
     }
     case JobEventDetails::PAGE_DONE:
 #if defined(OS_WIN)
-      pdf_to_emf_state_->OnPageProcessed(
-          base::Bind(&PrintJob::OnPdfToEmfPageConverted, this));
+      if (pdf_conversion_state_) {
+        pdf_conversion_state_->OnPageProcessed(
+            base::Bind(&PrintJob::OnPdfPageConverted, this));
+      }
 #endif  // defined(OS_WIN)
       break;
     default: {