Refactor print_job.cc

chrome/browser/printing/print_job.cc

 // Copyright (c) 2012 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 "chrome/browser/printing/print_job.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 204 matching lines @@
 
 bool PrintJob::is_job_pending() const {
   return is_job_pending_;
 }
 
 PrintedDocument* PrintJob::document() const {
   return document_.get();
 }
 
 #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,
              bool print_text_with_gdi,
              const PdfConverter::StartCallback& start_callback) {
     converter_->Start(data, conversion_settings, print_text_with_gdi,
                       start_callback);
   }
 
   void GetMorePages(const PdfConverter::GetPageCallback& get_page_callback) {
     const int kMaxNumberOfTempFilesPerDocument = 3;
     while (pages_in_progress_ < kMaxNumberOfTempFilesPerDocument &&
            current_page_ < page_count_) {
       ++pages_in_progress_;
       converter_->GetPage(current_page_++, get_page_callback);
     }
   }
 
   void OnPageProcessed(const PdfConverter::GetPageCallback& get_page_callback) {
     --pages_in_progress_;
     GetMorePages(get_page_callback);
     // Release converter if we don't need this any more.
     if (!pages_in_progress_ && current_page_ >= page_count_)
       converter_.reset();
   }
 
   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_;
   gfx::Size page_size_;
   gfx::Rect content_area_;
   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);
 }
 
 void PrintJob::StartPdfToEmfConversion(
     const scoped_refptr<base::RefCountedMemory>& bytes,
     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)
     return;
 
   document_ = new_document;
 
   if (document_.get())
     settings_ = document_->settings();
 
@@ skipping 30 matching lines @@
       break;
     }
     case JobEventDetails::DOC_DONE: {
       // This will call Stop() and broadcast a JOB_DONE message.
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&PrintJob::OnDocumentDone, this));
       break;
     }
     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: {
       NOTREACHED();
       break;
     }
   }
 }
 
 void PrintJob::OnDocumentDone() {
@@ skipping 70 matching lines @@
 }
 
 JobEventDetails::~JobEventDetails() {
 }
 
 PrintedDocument* JobEventDetails::document() const { return document_.get(); }
 
 PrintedPage* JobEventDetails::page() const { return page_.get(); }
 
 }  // namespace printing