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 27f716a1d608e47c77e1f7aa7715c2056a34a65e..f598d086ff9c6bf475283a6a5ddc6906b3c74a6f 100644
--- a/chrome/renderer/print_web_view_helper.cc
+++ b/chrome/renderer/print_web_view_helper.cc
@@ -568,12 +568,8 @@ void PrintWebViewHelper::OnPrintForPrintPreview(
     return;
   }
 
-  WebFrame* pdf_frame = pdf_element.document().frame();
-  scoped_ptr<PrepareFrameAndViewForPrint> prepare;
-  if (!InitPrintSettingsAndPrepareFrame(pdf_frame, &pdf_element, &prepare)) {
-    LOG(ERROR) << "Failed to initialize print page settings";
-    return;
-  }
+  if (!print_pages_params_.get())
+    print_pages_params_.reset(new PrintMsg_PrintPages_Params());

[kmadhusu, 2011/09/14 01:37:25]

lines 571 & 572 are not required. Any specific reason for adding this code here?

[arthurhsu, 2011/09/16 18:23:01]

Done.

 
   if (!UpdatePrintSettings(job_settings, false)) {
     LOG(ERROR) << "UpdatePrintSettings failed";
@@ -581,6 +577,13 @@ void PrintWebViewHelper::OnPrintForPrintPreview(
     return;
   }
 
+  WebFrame* pdf_frame = pdf_element.document().frame();
+  scoped_ptr<PrepareFrameAndViewForPrint> prepare;
+  if (!PrepareFrame(pdf_frame, &pdf_element, &prepare)) {
+    LOG(ERROR) << "Failed to initialize print page settings";

[kmadhusu, 2011/09/14 01:37:25]

You need to call DidFinishPrinting(FAIL_PRINT);

[arthurhsu, 2011/09/16 18:23:01]

Done.

+    return;
+  }
+
   // Render Pages for printing.
   if (!RenderPagesForPrint(pdf_frame, &pdf_element, prepare.get())) {
     LOG(ERROR) << "RenderPagesForPrint failed";
@@ -630,18 +633,11 @@ void PrintWebViewHelper::OnPrintPreview(const DictionaryValue& settings) {
   DCHECK(is_preview_);
   print_preview_context_.OnPrintPreview();
 
-  if (!InitPrintSettings(print_preview_context_.frame(),
-                         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);
+    print_pages_params_.reset(new PrintMsg_PrintPages_Params());

[kmadhusu, 2011/09/14 01:37:25]

Just release the previous settings.

print_pages_params_.reset(); 

[arthurhsu, 2011/09/16 18:23:01]

This line is actually wrong.  It shall issue failure.

     return;
   }
 
@@ -805,7 +801,8 @@ void PrintWebViewHelper::Print(WebKit::WebFrame* frame, WebKit::WebNode* node) {
 
   // Initialize print settings.
   scoped_ptr<PrepareFrameAndViewForPrint> prepare;
-  if (!InitPrintSettingsAndPrepareFrame(frame, node, &prepare))
+  if (!InitPrintSettings(frame) ||
+      !PrepareFrame(frame, node, &prepare))

[kmadhusu, 2011/09/14 01:37:25]

nit: {} required 

[kmadhusu, 2011/09/14 01:37:25]

When PrepareFrame() fails, you need to call DidFinishPrinting(FAIL_PRINT)

[arthurhsu, 2011/09/16 18:23:01]

Done.

[arthurhsu, 2011/09/16 18:23:01]

Done.

     return;  // Failed to init print page settings.
 
   int expected_page_count = 0;
@@ -1055,9 +1052,7 @@ void PrintWebViewHelper::UpdatePrintableSizeInPrintParameters(
   prepare->UpdatePrintParams(*params);
 }
 
-bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame,
-                                           WebKit::WebNode* node,
-                                           bool is_preview) {
+bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame) {
   DCHECK(frame);
   PrintMsg_PrintPages_Params settings;
 
@@ -1068,13 +1063,11 @@ bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame,
   // 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;
+    render_view()->runModalAlertDialog(
+        frame,
+        l10n_util::GetStringUTF16(
+            IDS_PRINT_PREVIEW_INVALID_PRINTER_SETTINGS));
+    result = false;
   }
 
   if (result &&
@@ -1089,12 +1082,9 @@ bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame,
   return result;
 }
 
-bool PrintWebViewHelper::InitPrintSettingsAndPrepareFrame(
+bool PrintWebViewHelper::PrepareFrame(
     WebKit::WebFrame* frame, WebKit::WebNode* node,
     scoped_ptr<PrepareFrameAndViewForPrint>* prepare) {
-  if (!InitPrintSettings(frame, node, false))
-    return false;
-
   DCHECK(!prepare->get());
   prepare->reset(new PrepareFrameAndViewForPrint(print_pages_params_->params,
                                                  frame, node));
@@ -1109,8 +1099,25 @@ bool PrintWebViewHelper::UpdatePrintSettings(
     const DictionaryValue& job_settings, bool is_preview) {
   PrintMsg_PrintPages_Params settings;
 
+  if (!is_preview) {
+    Send(new PrintHostMsg_DidGetDocumentCookie(
+        routing_id(), print_pages_params_->params.document_cookie));
+  } else if (!print_pages_params_.get()) {
+    print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
+  }
   Send(new PrintHostMsg_UpdatePrintSettings(routing_id(),
-       print_pages_params_->params.document_cookie, job_settings, &settings));
+      print_pages_params_->params.document_cookie, job_settings, &settings));

[kmadhusu, 2011/09/14 01:37:25]

As we discussed, you can remove print_pages_params_->params.document_cookie.

You can also remove lines 1102-1107.

[arthurhsu, 2011/09/16 18:23:01]

Done.

+
+  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) {
     print_preview_context_.set_error(PREVIEW_ERROR_UPDATING_PRINT_SETTINGS);
     return false;