[Printing] Don't create unnecessary PrintSettings instances.

Make |PrintingContext::settings_| null before settings are created or
after ownership is passed to PrinterQuery.

Bug: 964948
Change-Id: I0cff0b5b460ecb3424ff0cb07e5849bed3ffe51e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1678182
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Vladislav Kuzkokov <vkuzkokov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688541}

chrome/browser/printing/print_job_unittest.cc

@@ -57,7 +57,7 @@ class TestQuery : public PrinterQuery {
     auto worker = std::make_unique<TestPrintJobWorker>();
     EXPECT_TRUE(worker->Start());
     worker->printing_context()->UseDefaultSettings();
-    SetSettingsForTest(worker->printing_context()->TakeAndResetSettings());
+    SetSettingsForTest(worker->printing_context()->ExtractSettings());
 
     return std::move(worker);
   }

chrome/browser/printing/print_job_worker.cc

@@ -159,6 +159,7 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings,
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   DCHECK_EQ(page_number_, PageNumber::npos());
 
+  printing_context_->ResetSettings();
   printing_context_->set_margin_type(margin_type);
   printing_context_->set_is_modifiable(is_modifiable);
 
@@ -221,7 +222,7 @@ void PrintJobWorker::UpdatePrintSettingsFromPOD(
 
 void PrintJobWorker::GetSettingsDone(SettingsCallback callback,
                                      PrintingContext::Result result) {
-  std::move(callback).Run(printing_context_->TakeAndResetSettings(), result);
+  std::move(callback).Run(printing_context_->ExtractSettings(), result);
 }
 
 void PrintJobWorker::GetSettingsWithUI(int document_page_count,

printing/print_settings.cc

@@ -149,46 +149,10 @@ bool IsColorModelSelected(int color_mode) {
 // Global SequenceNumber used for generating unique cookie values.
 static base::AtomicSequenceNumber cookie_seq;
 
-PrintSettings::PrintSettings() {
-  Clear();
-}
+PrintSettings::PrintSettings() = default;
 
 PrintSettings::~PrintSettings() = default;
 
-void PrintSettings::Clear() {
-  ranges_.clear();
-  selection_only_ = false;
-  margin_type_ = DEFAULT_MARGINS;
-  title_.clear();
-  url_.clear();
-  display_header_footer_ = false;
-  should_print_backgrounds_ = false;
-  collate_ = false;
-  color_ = UNKNOWN_COLOR_MODEL;
-  copies_ = 0;
-  duplex_mode_ = UNKNOWN_DUPLEX_MODE;
-  device_name_.clear();
-  requested_media_ = RequestedMedia();
-  page_setup_device_units_.Clear();
-  dpi_ = gfx::Size();
-  scale_factor_ = 1.0f;
-  rasterize_pdf_ = false;
-  landscape_ = false;
-  supports_alpha_blend_ = true;
-#if defined(OS_WIN)
-  print_text_with_gdi_ = false;
-  printer_type_ = PrintSettings::PrinterType::TYPE_NONE;
-#endif
-  is_modifiable_ = true;
-  pages_per_sheet_ = 1;
-#if defined(OS_CHROMEOS)
-  send_user_info_ = false;
-  username_.clear();
-  job_title_.clear();
-  pin_value_.clear();
-#endif  // defined(OS_CHROMEOS)
-}
-
 void PrintSettings::SetPrinterPrintableArea(
     const gfx::Size& physical_size_device_units,
     const gfx::Rect& printable_area_device_units,

printing/print_settings.h

@@ -62,9 +62,6 @@ class PRINTING_EXPORT PrintSettings {
   PrintSettings();
   ~PrintSettings();
 
-  // Reinitialize the settings to the default values.
-  void Clear();
-
   void SetCustomMargins(const PageMargins& requested_margins_in_points);
   const PageMargins& requested_custom_margins_in_points() const {
     return requested_custom_margins_in_points_;
@@ -214,32 +211,32 @@ class PRINTING_EXPORT PrintSettings {
   PageRanges ranges_;
 
   // Indicates if the user only wants to print the current selection.
-  bool selection_only_;
+  bool selection_only_ = false;
 
   // Indicates what kind of margins should be applied to the printable area.
-  MarginType margin_type_;
+  MarginType margin_type_ = DEFAULT_MARGINS;
 
   // Strings to be printed as headers and footers if requested by the user.
   base::string16 title_;
   base::string16 url_;
 
   // True if the user wants headers and footers to be displayed.
-  bool display_header_footer_;
+  bool display_header_footer_ = false;
 
   // True if the user wants to print CSS backgrounds.
-  bool should_print_backgrounds_;
+  bool should_print_backgrounds_ = false;
 
   // True if the user wants to print with collate.
-  bool collate_;
+  bool collate_ = false;
 
   // True if the user wants to print with collate.
-  ColorModel color_;
+  ColorModel color_ = UNKNOWN_COLOR_MODEL;
 
   // Number of copies user wants to print.
-  int copies_;
+  int copies_ = 0;
 
   // Duplex type user wants to use.
-  DuplexMode duplex_mode_;
+  DuplexMode duplex_mode_ = UNKNOWN_DUPLEX_MODE;
 
   // Printer device name as opened by the OS.
   base::string16 device_name_;
@@ -256,35 +253,35 @@ class PRINTING_EXPORT PrintSettings {
   gfx::Size dpi_;
 
   // Scale factor
-  double scale_factor_;
+  double scale_factor_ = 1.0;
 
   // True if PDF should be printed as a raster PDF
-  bool rasterize_pdf_;
+  bool rasterize_pdf_ = false;
 
   // Is the orientation landscape or portrait.
-  bool landscape_;
+  bool landscape_ = false;
 
   // True if this printer supports AlphaBlend.
-  bool supports_alpha_blend_;
+  bool supports_alpha_blend_ = true;
 
 #if defined(OS_WIN)
   // True to print text with GDI.
-  bool print_text_with_gdi_;
+  bool print_text_with_gdi_ = false;
 
-  PrinterType printer_type_;
+  PrinterType printer_type_ = PrinterType::TYPE_NONE;
 #endif
 
-  bool is_modifiable_;
+  bool is_modifiable_ = true;
 
   // If margin type is custom, this is what was requested.
   PageMargins requested_custom_margins_in_points_;
 
   // Number of pages per sheet.
-  int pages_per_sheet_;
+  int pages_per_sheet_ = 1;
 
 #if defined(OS_CHROMEOS)
   // Whether to send user info.
-  bool send_user_info_;
+  bool send_user_info_ = false;
 
   // Username if it's required by the printer.
   std::string username_;

printing/printing_context.cc

@@ -20,10 +20,7 @@ const float kCloudPrintMarginInch = 0.25;
 }
 
 PrintingContext::PrintingContext(Delegate* delegate)
-    : settings_(std::make_unique<PrintSettings>()),
-      delegate_(delegate),
-      in_print_job_(false),
-      abort_printing_(false) {
+    : delegate_(delegate), in_print_job_(false), abort_printing_(false) {
   DCHECK(delegate_);
 }
 
@@ -46,24 +43,30 @@ const PrintSettings& PrintingContext::settings() const {
   return *settings_;
 }
 
-void PrintingContext::ResetSettings() {
+void PrintingContext::ResetSettingsImpl(bool create_empty) {
   ReleaseContext();
 
-  settings_->Clear();
+  settings_ = create_empty ? std::make_unique<PrintSettings>() : nullptr;
 
   in_print_job_ = false;
   abort_printing_ = false;
 }
 
-std::unique_ptr<PrintSettings> PrintingContext::TakeAndResetSettings() {
-  std::unique_ptr<PrintSettings> result = std::move(settings_);
-  settings_ = std::make_unique<PrintSettings>();
-  return result;
+void PrintingContext::ResetSettings() {
+  ResetSettingsImpl(true);
+}
+
+void PrintingContext::DeleteSettings() {
+  ResetSettingsImpl(false);
+}
+
+std::unique_ptr<PrintSettings> PrintingContext::ExtractSettings() {
+  return std::move(settings_);
 }
 
 PrintingContext::Result PrintingContext::OnError() {
   Result result = abort_printing_ ? CANCEL : FAILED;
-  ResetSettings();
+  DeleteSettings();
   return result;
 }
 
@@ -158,7 +161,7 @@ PrintingContext::Result PrintingContext::UpdatePrintSettings(
 #if defined(OS_CHROMEOS)
 PrintingContext::Result PrintingContext::UpdatePrintSettingsFromPOD(
     std::unique_ptr<PrintSettings> job_settings) {
-  ResetSettings();
+  DeleteSettings();
   settings_ = std::move(job_settings);
 
   return UpdatePrinterSettings(false /* external_preview */,

printing/printing_context.h

@@ -127,16 +127,19 @@ class PRINTING_EXPORT PrintingContext {
 
   const PrintSettings& settings() const;
 
-  std::unique_ptr<PrintSettings> TakeAndResetSettings();
+  std::unique_ptr<PrintSettings> ExtractSettings();
 
   int job_id() const { return job_id_; }
 
- protected:
-  explicit PrintingContext(Delegate* delegate);
-
   // Reinitializes the settings for object reuse.
   void ResetSettings();
 
+  // Cleans up object settings.
+  void DeleteSettings();
+
+ protected:
+  explicit PrintingContext(Delegate* delegate);
+
   // Does bookkeeping when an error occurs.
   PrintingContext::Result OnError();
 
@@ -156,6 +159,8 @@ class PRINTING_EXPORT PrintingContext {
   int job_id_;
 
  private:
+  void ResetSettingsImpl(bool create_empty);
+
   DISALLOW_COPY_AND_ASSIGN(PrintingContext);
 };
 

printing/printing_context_android.cc

@@ -121,6 +121,7 @@ void PrintingContextAndroid::AskUserForSettingsReply(
     std::move(callback_).Run(FAILED);
     return;
   }
+  ResetSettings();
 
   // We use device name variable to store the file descriptor.  This is hacky
   // but necessary. Since device name is not necessary for the upstream
@@ -245,7 +246,7 @@ PrintingContext::Result PrintingContextAndroid::DocumentDone() {
     return CANCEL;
   DCHECK(in_print_job_);
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_chromeos.cc

@@ -365,7 +365,7 @@ PrintingContext::Result PrintingContextChromeos::DocumentDone() {
     return OnError();
   }
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_linux.cc

@@ -172,7 +172,7 @@ PrintingContext::Result PrintingContextLinux::DocumentDone() {
     return CANCEL;
   DCHECK(in_print_job_);
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_mac.mm

@@ -161,6 +161,7 @@ PMPaper MatchPaper(CFArrayRef paper_list,
 PrintingContext::Result PrintingContextMac::UseDefaultSettings() {
   DCHECK(!in_print_job_);
 
+  ResetSettings();
   print_info_.reset([[NSPrintInfo sharedPrintInfo] copy]);
   settings_->set_ranges(GetPageRangesFromPrintInfo());
   InitPrintSettingsFromPrintInfo();
@@ -485,7 +486,7 @@ PMPaper MatchPaper(CFArrayRef paper_list,
   if (status != noErr)
     OnError();
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_no_system_dialog.cc

@@ -123,7 +123,7 @@ PrintingContext::Result PrintingContextNoSystemDialog::DocumentDone() {
     return CANCEL;
   DCHECK(in_print_job_);
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_system_dialog_win.cc

@@ -67,7 +67,7 @@ void PrintingContextSystemDialogWin::AskUserForSettings(
   }
 
   if (ShowPrintDialog(&dialog_options) != S_OK) {
-    ResetSettings();
+    DeleteSettings();
     std::move(callback).Run(FAILED);
     return;
   }
@@ -113,7 +113,7 @@ bool PrintingContextSystemDialogWin::InitializeSettingsWithRanges(
   if (!(GetDeviceCaps(context(), RASTERCAPS) & RC_STRETCHDIB) ||
       !(GetDeviceCaps(context(), RASTERCAPS) & RC_BITMAP64)) {
     NOTREACHED();
-    ResetSettings();
+    DeleteSettings();
     return false;
   }
 

printing/printing_context_win.cc

@@ -71,6 +71,7 @@ PrintingContext::Result PrintingContextWin::UseDefaultSettings() {
   scoped_refptr<PrintBackend> backend = PrintBackend::CreateInstance(nullptr);
   base::string16 default_printer =
       base::UTF8ToWide(backend->GetDefaultPrinterName());
+  ResetSettings();
   if (!default_printer.empty()) {
     ScopedPrinterHandle printer;
     if (printer.OpenPrinterWithName(default_printer.c_str())) {
@@ -321,7 +322,7 @@ PrintingContext::Result PrintingContextWin::DocumentDone() {
   if (EndDoc(context_) <= 0)
     return OnError();
 
-  ResetSettings();
+  DeleteSettings();
   return OK;
 }
 

printing/printing_context_win_unittest.cc

@@ -149,6 +149,7 @@ TEST_F(PrintingContextTest, PrintAll) {
 
   base::test::TaskEnvironment task_environment;
   MockPrintingContextWin context(this);
+  context.ResetSettings();
   context.AskUserForSettings(
       123, false, false,
       base::BindOnce(&PrintingContextTest::PrintSettingsCallback,
@@ -164,6 +165,7 @@ TEST_F(PrintingContextTest, Color) {
 
   base::test::TaskEnvironment task_environment;
   MockPrintingContextWin context(this);
+  context.ResetSettings();
   context.AskUserForSettings(
       123, false, false,
       base::BindOnce(&PrintingContextTest::PrintSettingsCallback,