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 612 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/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_,
                                     *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 386 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)) {
-    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;
 
+  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;
 
   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 397 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();
 }