Fix print preview workflow to reflect settings of selected printer.

chrome/renderer/print_web_view_helper.cc

Index: chrome/renderer/print_web_view_helper.cc
diff --git a/chrome/renderer/print_web_view_helper.cc b/chrome/renderer/print_web_view_helper.cc
index 36726eee4667e71193e7de272e489219da218c46..7abe910afcd430b3d4ded6aee3e846acb5a107b9 100644
--- a/chrome/renderer/print_web_view_helper.cc
+++ b/chrome/renderer/print_web_view_helper.cc
@@ -630,21 +630,13 @@ void PrintWebViewHelper::OnPrintPreview(const DictionaryValue& settings) {
   DCHECK(is_preview_);
   print_preview_context_.OnPrintPreview();
 
-  if (!InitPrintSettings(print_preview_context_.frame(),

[Lei Zhang, 2011/09/02 18:44:19]

I don't understand why you are not calling InitPrintSettings().

[arthurhsu, 2011/09/06 22:02:27]

It's done so based on Kausalya's suggestion.  For print preview, we don't need
InitPrintSettings(), we just need to get the selected printer's settings.

[kmadhusu, 2011/09/06 22:47:11]

thestig@:

Two main reasons for this change:

(1) InitPrintSettings() is initializing the PrintSettings info with default
printer default settings. Whenever the user changes the printer, we want to
initialize the PrintSettings with selected printer default settings.

(2) InitPrintSettings() is not actually required for the print preview workflow.
We had that native printing workflow because we need to calculate the page count
before displaying the system dialog to the user. With respect to the preview
workflow, we always call InitPrintSettings() followed by UpdatePrintSettings(). 

Do you remember any reasons why we had InitPrintSettings() before
UpdatePrintSettings()?

-                         print_preview_context_.node(),
-                         true)) {
+  if (!UpdatePrintSettings(settings, true)) {
     Send(new PrintHostMsg_PrintPreviewInvalidPrinterSettings(
          routing_id(),
          print_pages_params_->params.document_cookie));
     return;
   }
 
-  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_,
@@ -1051,15 +1043,6 @@ bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame,
   // can safely assume there are no printer drivers configured. So we safely
   // terminate.
   bool result = true;
-  if (PrintMsg_Print_Params_IsEmpty(settings.params)) {
-    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)) {
@@ -1093,8 +1076,25 @@ bool PrintWebViewHelper::UpdatePrintSettings(
     const DictionaryValue& job_settings, bool is_preview) {
   PrintMsg_PrintPages_Params settings;
 
+  if (print_pages_params_ == NULL) {

[kmadhusu, 2011/09/12 17:20:33]

Can you please explain why do you need this check here? It looks like you will
be initializing print_pages_params_ only once. 

[arthurhsu, 2011/09/12 17:25:13]

The check is to avoid crashing in line 1085 when params is dereferenced.  I
think the print_pages_params_ shall only be created once.  No?

[kmadhusu, 2011/09/12 17:36:52]

(repeating our in-person conversation): We will call this function whenever a
setting change (layout, color, page range, printer change, etc.,). Therefore for
every preview request, we will reset this print_pages_params_ with new settings
info.

[arthurhsu, 2011/09/12 18:20:11]

Done.

+    print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
+    Send(new PrintHostMsg_DidGetDocumentCookie(
+        routing_id(), print_pages_params_->params.document_cookie));
+  }
+
   Send(new PrintHostMsg_UpdatePrintSettings(routing_id(),
        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;