Fix print preview workflow to reflect settings of selected printer.

chrome/renderer/print_web_view_helper.cc

 // Copyright (c) 2011 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/renderer/print_web_view_helper.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 552 matching lines @@
   // <object> with id="pdf-viewer" is created in
   // chrome/browser/resources/print_preview/print_preview.js
   WebElement pdf_element = document.getElementById("pdf-viewer");
   if (pdf_element.isNull()) {
     NOTREACHED();
     return;
   }
 
   WebFrame* pdf_frame = pdf_element.document().frame();
   scoped_ptr<PrepareFrameAndViewForPrint> prepare;
   if (!InitPrintSettingsAndPrepareFrame(pdf_frame, &pdf_element, &prepare)) {

[kmadhusu, 2011/09/13 20:15:34]

What will happen if the user has a invalid default printer and has chosen a
valid printer for the current print job? I think we will fail here. 

[arthurhsu, 2011/09/14 01:08:03]

Done.

     LOG(ERROR) << "Failed to initialize print page settings";
     return;
   }
 
   if (!UpdatePrintSettings(job_settings, false)) {
     LOG(ERROR) << "UpdatePrintSettings failed";
     DidFinishPrinting(FAIL_PRINT);
     return;
   }
 
@@ skipping 39 matching lines @@
     // copy when the print preview tab closes.
     WebNode duplicate_node(*node);
     Print(frame, &duplicate_node);
   }
 }
 
 void PrintWebViewHelper::OnPrintPreview(const DictionaryValue& settings) {
   DCHECK(is_preview_);
   print_preview_context_.OnPrintPreview();
 
-  if (!InitPrintSettings(print_preview_context_.frame(),

[Lei Zhang, 2011/09/13 20:09:53]

Since you are removing this, nobody will call InitPrintSettings() with
|print_preview| set to true. Can you cleanup InitPrintSettings()? I noticed
InitPrintSettings() also doesn't use it's |node| argument.

[arthurhsu, 2011/09/14 01:08:03]

Done.

-                         print_preview_context_.node(),
-                         true)) {
+  if (!UpdatePrintSettings(settings, true)) {
     Send(new PrintHostMsg_PrintPreviewInvalidPrinterSettings(
          routing_id(),
          print_pages_params_->params.document_cookie));
     return;

[kmadhusu, 2011/09/13 20:01:33]

Do you want to clear print_pages_params_ ??

[arthurhsu, 2011/09/14 01:08:03]

Done.

   }
 
-  if (!UpdatePrintSettings(settings, true)) {
-    LOG(ERROR) << "UpdatePrintSettings failed";
-    DidFinishPrinting(FAIL_PREVIEW);
-    return;
-  }
-
   if (!print_pages_params_->params.is_first_request &&
       old_print_pages_params_.get() &&
       PrintMsg_Print_Params_IsEqual(*old_print_pages_params_,
                                     *print_pages_params_)) {
     PrintHostMsg_DidPreviewDocument_Params preview_params;
     preview_params.reuse_existing_data = true;
     preview_params.data_size = 0;
     preview_params.document_cookie =
         print_pages_params_->params.document_cookie;
     preview_params.expected_pages_count =
@@ skipping 401 matching lines @@
                                            bool is_preview) {
   DCHECK(frame);
   PrintMsg_PrintPages_Params settings;
 
   Send(new PrintHostMsg_GetDefaultPrintSettings(routing_id(),
                                                 &settings.params));
   // Check if the printer returned any settings, if the settings is empty, we
   // can safely assume there are no printer drivers configured. So we safely
   // terminate.
   bool result = true;
-  if (PrintMsg_Print_Params_IsEmpty(settings.params)) {

[kmadhusu, 2011/09/13 20:01:33]

You still need this code. This function is used by native printing workflow. 

[Lei Zhang, 2011/09/13 20:09:53]

I don't think you can remove this for the native dialog print workflow, where we
never call UpdatePrintSettings(). Thus the code in your CL at line 1100 doesn't
make sense.

[arthurhsu, 2011/09/14 01:08:03]

Done.

[arthurhsu, 2011/09/14 01:08:03]

Done.

-    if (!is_preview) {
-      render_view()->runModalAlertDialog(
-          frame,
-          l10n_util::GetStringUTF16(
-              IDS_PRINT_PREVIEW_INVALID_PRINTER_SETTINGS));
-    }
-    result = false;
-  }
 
   if (result &&
       (settings.params.dpi < kMinDpi || settings.params.document_cookie == 0)) {
     // Invalid print page settings.
     NOTREACHED();
     result = false;
   }
 
   settings.pages.clear();
   print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
@@ skipping 13 matching lines @@
                                        &print_pages_params_->params);
   Send(new PrintHostMsg_DidGetDocumentCookie(
         routing_id(), print_pages_params_->params.document_cookie));
   return true;
 }
 
 bool PrintWebViewHelper::UpdatePrintSettings(
     const DictionaryValue& job_settings, bool is_preview) {
   PrintMsg_PrintPages_Params settings;
 
+  print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));

[Lei Zhang, 2011/09/13 20:09:53]

This basically resets |print_pages_params_| to all 0 values. Why do this?

[arthurhsu, 2011/09/14 01:08:03]

Done.

+  Send(new PrintHostMsg_DidGetDocumentCookie(

[kmadhusu, 2011/09/13 20:01:33]

You need to send this message only during the preview workflow.

During the native printing or preview printing, this message is already sent by
InitPrintSettingsAndPrepareFrame() function. 

[arthurhsu, 2011/09/14 01:08:03]

Done.

+      routing_id(), print_pages_params_->params.document_cookie));

[Lei Zhang, 2011/09/13 20:09:53]

This will always send a value of 0 for the cookie.

[arthurhsu, 2011/09/14 01:08:03]

Done.

   Send(new PrintHostMsg_UpdatePrintSettings(routing_id(),
-       print_pages_params_->params.document_cookie, job_settings, &settings));
+      print_pages_params_->params.document_cookie, job_settings, &settings));
+
+  if (PrintMsg_Print_Params_IsEmpty(settings.params)) {
+    if (!is_preview) {
+      render_view()->runModalAlertDialog(
+          print_preview_context_.frame(),
+          l10n_util::GetStringUTF16(
+              IDS_PRINT_PREVIEW_INVALID_PRINTER_SETTINGS));
+    }
+    return false;
+  }
+
   if (settings.params.dpi < kMinDpi || !settings.params.document_cookie)
     return false;
 
   if (is_preview) {
     // Validate expected print preview settings.
     if (!job_settings.GetString(printing::kPreviewUIAddr,
                                 &(settings.params.preview_ui_addr)) ||
         !job_settings.GetInteger(printing::kPreviewRequestID,
                                  &(settings.params.preview_request_id)) ||
         !job_settings.GetBoolean(printing::kIsFirstRequest,
@@ skipping 406 matching lines @@
 const gfx::Size&
 PrintWebViewHelper::PrintPreviewContext::GetPrintCanvasSize() const {
   return prep_frame_view_->GetPrintCanvasSize();
 }
 
 void PrintWebViewHelper::PrintPreviewContext::ClearContext() {
   prep_frame_view_.reset();
   metafile_.reset();
   pages_to_render_.clear();
 }