[Printing] Make PrintSettings non-copyable.

This is the "safe" version of the change where PrintSettings in
PrintingContext always exists. Unnecessary PrintSettings instances
would have to be removed in follow-up CL.

Bug: 964948
Change-Id: I88903473b7b2881cbf63543a86227ead2898eae0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1646772
Commit-Queue: Bo <boliu@chromium.org>
Auto-Submit: Vladislav Kuzkokov <vkuzkokov@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685983}

android_webview/browser/aw_pdf_exporter.cc

@@ -4,6 +4,10 @@
 
 #include "android_webview/browser/aw_pdf_exporter.h"
 
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include "android_webview/browser/aw_print_manager.h"
 #include "android_webview/native_jni/AwPdfExporter_jni.h"
 #include "base/android/jni_android.h"
@@ -60,12 +64,13 @@ void AwPdfExporter::ExportToPdf(JNIEnv* env,
                                 const JavaParamRef<jintArray>& pages,
                                 const JavaParamRef<jobject>& cancel_signal) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  printing::PrintSettings print_settings;
+  auto print_settings = std::make_unique<printing::PrintSettings>();
   printing::PageRanges page_ranges;
   JNI_AwPdfExporter_GetPageRanges(env, pages, &page_ranges);
-  InitPdfSettings(env, obj, page_ranges, print_settings);
+  // TODO(crbug.com/964948) make InitPdfSettings() return PrintSettings.
+  InitPdfSettings(env, obj, page_ranges, print_settings.get());
   AwPrintManager* print_manager = AwPrintManager::CreateForWebContents(
-      web_contents_, print_settings, fd,
+      web_contents_, std::move(print_settings), fd,
       base::Bind(&AwPdfExporter::DidExportPdf, base::Unretained(this)));
 
   if (!print_manager->PrintNow())
@@ -82,7 +87,7 @@ int MilsToDots(int val, int dpi) {
 void AwPdfExporter::InitPdfSettings(JNIEnv* env,
                                     const JavaRef<jobject>& obj,
                                     const printing::PageRanges& page_ranges,
-                                    printing::PrintSettings& settings) {
+                                    printing::PrintSettings* settings) {
   int dpi = Java_AwPdfExporter_getDpi(env, obj);
   int width = Java_AwPdfExporter_getPageWidth(env, obj);
   int height = Java_AwPdfExporter_getPageHeight(env, obj);
@@ -96,22 +101,22 @@ void AwPdfExporter::InitPdfSettings(JNIEnv* env,
   printable_area_device_units.SetRect(0, 0, width_in_dots, height_in_dots);
 
   if (!page_ranges.empty())
-    settings.set_ranges(page_ranges);
+    settings->set_ranges(page_ranges);
 
-  settings.set_dpi(dpi);
+  settings->set_dpi(dpi);
   // TODO(sgurun) verify that the value for newly added parameter for
   // (i.e. landscape_needs_flip) is correct.
-  settings.SetPrinterPrintableArea(physical_size_device_units,
-                                   printable_area_device_units, true);
+  settings->SetPrinterPrintableArea(physical_size_device_units,
+                                    printable_area_device_units, true);
 
   printing::PageMargins margins;
   margins.left = MilsToDots(Java_AwPdfExporter_getLeftMargin(env, obj), dpi);
   margins.right = MilsToDots(Java_AwPdfExporter_getRightMargin(env, obj), dpi);
   margins.top = MilsToDots(Java_AwPdfExporter_getTopMargin(env, obj), dpi);
   margins.bottom =
       MilsToDots(Java_AwPdfExporter_getBottomMargin(env, obj), dpi);
-  settings.SetCustomMargins(margins);
-  settings.set_should_print_backgrounds(true);
+  settings->SetCustomMargins(margins);
+  settings->set_should_print_backgrounds(true);
 }
 
 void AwPdfExporter::DidExportPdf(int page_count) {

android_webview/browser/aw_pdf_exporter.h

@@ -39,7 +39,7 @@ class AwPdfExporter {
   void InitPdfSettings(JNIEnv* env,
                        const base::android::JavaRef<jobject>& obj,
                        const printing::PageRanges& page_ranges,
-                       printing::PrintSettings& settings);
+                       printing::PrintSettings* settings);
   void DidExportPdf(int page_count);
 
   JavaObjectWeakGlobalRef java_ref_;

android_webview/browser/aw_print_manager.cc

@@ -4,6 +4,8 @@
 
 #include "android_webview/browser/aw_print_manager.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
@@ -36,20 +38,24 @@ int SaveDataToFd(int fd,
 // static
 AwPrintManager* AwPrintManager::CreateForWebContents(
     content::WebContents* contents,
-    const printing::PrintSettings& settings,
+    std::unique_ptr<printing::PrintSettings> settings,
     int file_descriptor,
     PrintManager::PdfWritingDoneCallback callback) {
   AwPrintManager* print_manager = new AwPrintManager(
-      contents, settings, file_descriptor, std::move(callback));
+      contents, std::move(settings), file_descriptor, std::move(callback));
   contents->SetUserData(UserDataKey(), base::WrapUnique(print_manager));
   return print_manager;
 }
 
-AwPrintManager::AwPrintManager(content::WebContents* contents,
-                               const printing::PrintSettings& settings,
-                               int file_descriptor,
-                               PdfWritingDoneCallback callback)
-    : PrintManager(contents), settings_(settings), fd_(file_descriptor) {
+AwPrintManager::AwPrintManager(
+    content::WebContents* contents,
+    std::unique_ptr<printing::PrintSettings> settings,
+    int file_descriptor,
+    PdfWritingDoneCallback callback)
+    : PrintManager(contents),
+      settings_(std::move(settings)),
+      fd_(file_descriptor) {
+  DCHECK(settings_);
   pdf_writing_done_callback_ = std::move(callback);
   cookie_ = 1;  // Set a valid dummy cookie value.
 }
@@ -75,7 +81,7 @@ void AwPrintManager::OnGetDefaultPrintSettings(
   // Unlike the printing_message_filter, we do process this in UI thread.
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   PrintMsg_Print_Params params;
-  printing::RenderParamsFromPrintSettings(settings_, &params);
+  printing::RenderParamsFromPrintSettings(*settings_, &params);
   params.document_cookie = cookie_;
   PrintHostMsg_GetDefaultPrintSettings::WriteReplyParams(reply_msg, params);
   render_frame_host->Send(reply_msg);
@@ -87,9 +93,9 @@ void AwPrintManager::OnScriptedPrint(
     IPC::Message* reply_msg) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   PrintMsg_PrintPages_Params params;
-  printing::RenderParamsFromPrintSettings(settings_, &params.params);
+  printing::RenderParamsFromPrintSettings(*settings_, &params.params);
   params.params.document_cookie = scripted_params.cookie;
-  params.pages = printing::PageRange::GetPages(settings_.ranges());
+  params.pages = printing::PageRange::GetPages(settings_->ranges());
   PrintHostMsg_ScriptedPrint::WriteReplyParams(reply_msg, params);
   render_frame_host->Send(reply_msg);
 }

android_webview/browser/aw_print_manager.h

@@ -5,6 +5,8 @@
 #ifndef ANDROID_WEBVIEW_BROWSER_AW_PRINT_MANAGER_H_
 #define ANDROID_WEBVIEW_BROWSER_AW_PRINT_MANAGER_H_
 
+#include <memory>
+
 #include "base/macros.h"
 #include "components/printing/browser/print_manager.h"
 #include "components/printing/common/print_messages.h"
@@ -21,7 +23,7 @@ class AwPrintManager : public printing::PrintManager,
   // The returned pointer is owned by |contents|.
   static AwPrintManager* CreateForWebContents(
       content::WebContents* contents,
-      const printing::PrintSettings& settings,
+      std::unique_ptr<printing::PrintSettings> settings,
       int file_descriptor,
       PdfWritingDoneCallback callback);
 
@@ -36,7 +38,7 @@ class AwPrintManager : public printing::PrintManager,
   friend class content::WebContentsUserData<AwPrintManager>;
 
   AwPrintManager(content::WebContents* contents,
-                 const printing::PrintSettings& settings,
+                 std::unique_ptr<printing::PrintSettings> settings,
                  int file_descriptor,
                  PdfWritingDoneCallback callback);
 
@@ -50,7 +52,7 @@ class AwPrintManager : public printing::PrintManager,
                        const PrintHostMsg_ScriptedPrint_Params& params,
                        IPC::Message* reply_msg) override;
 
-  printing::PrintSettings settings_;
+  const std::unique_ptr<printing::PrintSettings> settings_;
 
   // The file descriptor into which the PDF of the document will be written.
   int fd_;

chrome/browser/printing/print_job.cc

@@ -61,21 +61,21 @@ void PrintJob::Initialize(std::unique_ptr<PrinterQuery> query,
   DCHECK(!document_);
   worker_ = query->DetachWorker();
   worker_->SetPrintJob(this);
-  const PrintSettings& settings = query->settings();
-
-  auto new_doc =
-      base::MakeRefCounted<PrintedDocument>(settings, name, query->cookie());
-  new_doc->set_page_count(page_count);
-  UpdatePrintedDocument(new_doc);
+  std::unique_ptr<PrintSettings> settings = query->ExtractSettings();
 
 #if defined(OS_WIN)
-  pdf_page_mapping_ = PageRange::GetPages(settings.ranges());
+  pdf_page_mapping_ = PageRange::GetPages(settings->ranges());
   if (pdf_page_mapping_.empty()) {
     for (int i = 0; i < page_count; i++)
       pdf_page_mapping_.push_back(i);
   }
 #endif
 
+  auto new_doc = base::MakeRefCounted<PrintedDocument>(std::move(settings),
+                                                       name, query->cookie());
+  new_doc->set_page_count(page_count);
+  UpdatePrintedDocument(new_doc);
+
   // Don't forget to register to our own messages.
   registrar_.Add(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
                  content::Source<PrintJob>(this));

chrome/browser/printing/print_job_unittest.cc

@@ -39,7 +39,7 @@ class TestQuery : public PrinterQuery {
                      content::ChildProcessHost::kInvalidUniqueID) {}
 
   void GetSettingsDone(base::OnceClosure callback,
-                       const PrintSettings& new_settings,
+                       std::unique_ptr<PrintSettings> new_settings,
                        PrintingContext::Result result) override {
     FAIL();
   }
@@ -57,16 +57,12 @@ class TestQuery : public PrinterQuery {
     auto worker = std::make_unique<TestPrintJobWorker>();
     EXPECT_TRUE(worker->Start());
     worker->printing_context()->UseDefaultSettings();
-    settings_ = worker->printing_context()->settings();
+    SetSettingsForTest(worker->printing_context()->TakeAndResetSettings());
 
     return std::move(worker);
   }
 
-  const PrintSettings& settings() const override { return settings_; }
-
  private:
-  PrintSettings settings_;
-
   DISALLOW_COPY_AND_ASSIGN(TestQuery);
 };
 

chrome/browser/printing/print_job_worker.cc

@@ -221,7 +221,7 @@ void PrintJobWorker::UpdatePrintSettingsFromPOD(
 
 void PrintJobWorker::GetSettingsDone(SettingsCallback callback,
                                      PrintingContext::Result result) {
-  std::move(callback).Run(printing_context_->settings(), result);
+  std::move(callback).Run(printing_context_->TakeAndResetSettings(), result);
 }
 
 void PrintJobWorker::GetSettingsWithUI(int document_page_count,

chrome/browser/printing/print_job_worker.h

@@ -32,7 +32,8 @@ class PrintedPage;
 class PrintJobWorker {
  public:
   using SettingsCallback =
-      base::OnceCallback<void(const PrintSettings&, PrintingContext::Result)>;
+      base::OnceCallback<void(std::unique_ptr<PrintSettings>,
+                              PrintingContext::Result)>;
 
   PrintJobWorker(int render_process_id, int render_frame_id);
   virtual ~PrintJobWorker();

chrome/browser/printing/printer_query.cc

@@ -37,13 +37,13 @@ PrinterQuery::~PrinterQuery() {
 }
 
 void PrinterQuery::GetSettingsDone(base::OnceClosure callback,
-                                   const PrintSettings& new_settings,
+                                   std::unique_ptr<PrintSettings> new_settings,
                                    PrintingContext::Result result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   is_print_dialog_box_shown_ = false;
   last_status_ = result;
   if (result != PrintingContext::FAILED) {
-    settings_ = new_settings;
+    settings_ = std::move(new_settings);
     cookie_ = PrintSettings::NewCookie();
   } else {
     // Failure.
@@ -53,14 +53,15 @@ void PrinterQuery::GetSettingsDone(base::OnceClosure callback,
   std::move(callback).Run();
 }
 
-void PrinterQuery::PostSettingsDoneToIO(base::OnceClosure callback,
-                                        const PrintSettings& new_settings,
-                                        PrintingContext::Result result) {
+void PrinterQuery::PostSettingsDoneToIO(
+    base::OnceClosure callback,
+    std::unique_ptr<PrintSettings> new_settings,
+    PrintingContext::Result result) {
   // |this| is owned by |callback|, so |base::Unretained()| is safe.
   base::PostTask(
       FROM_HERE, {content::BrowserThread::IO},
       base::BindOnce(&PrinterQuery::GetSettingsDone, base::Unretained(this),
-                     std::move(callback), new_settings, result));
+                     std::move(callback), std::move(new_settings), result));
 }
 
 std::unique_ptr<PrintJobWorker> PrinterQuery::DetachWorker() {
@@ -71,7 +72,15 @@ std::unique_ptr<PrintJobWorker> PrinterQuery::DetachWorker() {
 }
 
 const PrintSettings& PrinterQuery::settings() const {
-  return settings_;
+  return *settings_;
+}
+
+std::unique_ptr<PrintSettings> PrinterQuery::ExtractSettings() {
+  return std::move(settings_);
+}
+
+void PrinterQuery::SetSettingsForTest(std::unique_ptr<PrintSettings> settings) {
+  settings_ = std::move(settings);
 }
 
 int PrinterQuery::cookie() const {

chrome/browser/printing/printer_query.h

@@ -41,8 +41,9 @@ class PrinterQuery {
   // TODO(thestig): Do |worker_| and |callback_| need locks?
   virtual std::unique_ptr<PrintJobWorker> DetachWorker();
 
-  // Virtual so that tests can override.
-  virtual const PrintSettings& settings() const;
+  const PrintSettings& settings() const;
+
+  std::unique_ptr<PrintSettings> ExtractSettings();
 
   // Initializes the printing context. It is fine to call this function multiple
   // times to reinitialize the settings. |web_contents_observer| can be queried
@@ -84,19 +85,21 @@ class PrinterQuery {
  protected:
   // Virtual so that tests can override.
   virtual void GetSettingsDone(base::OnceClosure callback,
-                               const PrintSettings& new_settings,
+                               std::unique_ptr<PrintSettings> new_settings,
                                PrintingContext::Result result);
 
   void PostSettingsDoneToIO(base::OnceClosure callback,
-                            const PrintSettings& new_settings,
+                            std::unique_ptr<PrintSettings> new_settings,
                             PrintingContext::Result result);
 
+  void SetSettingsForTest(std::unique_ptr<PrintSettings> settings);
+
  private:
   // Lazy create the worker thread. There is one worker thread per print job.
   void StartWorker();
 
   // Cache of the print context settings for access in the UI thread.
-  PrintSettings settings_;
+  std::unique_ptr<PrintSettings> settings_;
 
   // Is the Print... dialog box currently shown.
   bool is_print_dialog_box_shown_ = false;

chrome/browser/printing/test_print_job.cc

@@ -24,7 +24,7 @@ void TestPrintJob::Initialize(std::unique_ptr<PrinterQuery> query,
   std::unique_ptr<PrintJobWorker> worker = query->DetachWorker();
 
   scoped_refptr<PrintedDocument> new_doc =
-      base::MakeRefCounted<PrintedDocument>(query->settings(), name,
+      base::MakeRefCounted<PrintedDocument>(query->ExtractSettings(), name,
                                             query->cookie());
 
   new_doc->set_page_count(page_count);

chrome/browser/printing/test_printer_query.cc

@@ -52,27 +52,27 @@ void TestPrinterQuery::SetSettings(base::Value new_settings,
 #if defined(OS_WIN)
   DCHECK(printer_type_);
 #endif
-  PrintSettings settings;
+  auto settings = std::make_unique<PrintSettings>();
   PrintingContext::Result result =
-      PrintSettingsFromJobSettings(new_settings, &settings)
+      PrintSettingsFromJobSettings(new_settings, settings.get())
           ? PrintingContext::OK
           : PrintingContext::FAILED;
 
   float device_microns_per_device_unit =
-      static_cast<float>(kMicronsPerInch) / settings.device_units_per_inch();
+      static_cast<float>(kMicronsPerInch) / settings->device_units_per_inch();
   gfx::Size paper_size =
-      gfx::Size(settings.requested_media().size_microns.width() /
+      gfx::Size(settings->requested_media().size_microns.width() /
                     device_microns_per_device_unit,
-                settings.requested_media().size_microns.height() /
+                settings->requested_media().size_microns.height() /
                     device_microns_per_device_unit);
   gfx::Rect paper_rect(0, 0, paper_size.width(), paper_size.height());
   paper_rect.Inset(offsets_->x(), offsets_->y());
-  settings.SetPrinterPrintableArea(paper_size, paper_rect, true);
+  settings->SetPrinterPrintableArea(paper_size, paper_rect, true);
 #if defined(OS_WIN)
-  settings.set_printer_type(*printer_type_);
+  settings->set_printer_type(*printer_type_);
 #endif
 
-  GetSettingsDone(std::move(callback), settings, result);
+  GetSettingsDone(std::move(callback), std::move(settings), result);
 }
 
 #if defined(OS_WIN)