Print Preview: Fixing behavior of event listeners.

Now that pending preview requests are being canceled when a new request is issued, the event listeners of the printing controls should not check for |hasPendingPreviewRequest| before issuing new requests, instead they should immediately request a new preview so that the previous one is canceled.

Also all functions that are associated with a specific print request should get the requestID as a parameter, in order to ignore obsolete responses.

I found out that when the print preview tab is reloaded, responses from requests before the reload are reaching the ui code. If reloading the preview tab repeatedly all these request have id=0 and are not being ignored as they should. Therefore I changed the id of the initial request to have a random variable.

BUG=91556, 91769
TEST=See bug description

Review URL: http://codereview.chromium.org/7550022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@96634 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 38 insertions, 16 deletions

diff --git a/chrome/renderer/mock_printer.cc b/chrome/renderer/mock_printer.cc
index 6fa65cc..ac2c779 100644
--- a/chrome/renderer/mock_printer.cc
+++ b/chrome/renderer/mock_printer.cc
@@ -35,7 +35,9 @@ MockPrinter::MockPrinter()
     current_document_cookie_(0),
     printer_status_(PRINTER_READY),
     number_pages_(0),
-    page_number_(0) {
+    page_number_(0),
+    is_first_request_(true),
+    preview_request_id_(0) {
   page_size_.SetSize(static_cast<int>(8.5 * dpi_),
                      static_cast<int>(11.0 * dpi_));
   printable_size_.SetSize(static_cast<int>((7.5 * dpi_)),
@@ -90,6 +92,8 @@ void MockPrinter::ScriptedPrint(int cookie,
   settings->params.document_cookie = document_cookie_;
   settings->params.page_size = page_size_;
   settings->params.printable_size = printable_size_;
+  settings->params.is_first_request = is_first_request_;
+  settings->params.preview_request_id = preview_request_id_;
   printer_status_ = PRINTER_PRINTING;
 }
 
@@ -230,4 +234,6 @@ void MockPrinter::SetPrintParams(PrintMsg_Print_Params* params) {
   params->printable_size = printable_size_;
   params->margin_left = margin_left_;
   params->margin_top = margin_top_;
+  params->is_first_request = is_first_request_;
+  params->preview_request_id = preview_request_id_;
 }
diff --git a/chrome/renderer/mock_printer.h b/chrome/renderer/mock_printer.h
index 1c2994c..0ae5687 100644
--- a/chrome/renderer/mock_printer.h
+++ b/chrome/renderer/mock_printer.h
@@ -129,6 +129,11 @@ class MockPrinter {
   // The output of a printing job.
   int number_pages_;
   int page_number_;
+
+  // Used only in the preview sequence.
+  bool is_first_request_;
+  int preview_request_id_;
+
   std::vector<scoped_refptr<MockPrinterPage> > pages_;
 
   DISALLOW_COPY_AND_ASSIGN(MockPrinter);
diff --git a/chrome/renderer/mock_render_thread.cc b/chrome/renderer/mock_render_thread.cc
index aac7156..b443559 100644
--- a/chrome/renderer/mock_render_thread.cc
+++ b/chrome/renderer/mock_render_thread.cc
@@ -211,10 +211,9 @@ void MockRenderThread::OnDidPrintPage(
     printer_->PrintPage(params);
 }
 
-void MockRenderThread::OnDidGetPreviewPageCount(int document_cookie,
-                                                int number_pages,
-                                                bool is_modifiable) {
-  print_preview_pages_remaining_ = number_pages;
+void MockRenderThread::OnDidGetPreviewPageCount(
+    const PrintHostMsg_DidGetPreviewPageCount_Params& params) {
+  print_preview_pages_remaining_ = params.page_count;
 }
 
 void MockRenderThread::OnDidPreviewPage(
@@ -235,9 +234,11 @@ void MockRenderThread::OnUpdatePrintSettings(
       !job_settings.GetBoolean(printing::kSettingCollate, NULL) ||
       !job_settings.GetBoolean(printing::kSettingColor, NULL) ||
       !job_settings.GetBoolean(printing::kSettingPrintToPDF, NULL) ||
+      !job_settings.GetBoolean(printing::kIsFirstRequest, NULL) ||
       !job_settings.GetString(printing::kSettingDeviceName, &dummy_string) ||
       !job_settings.GetInteger(printing::kSettingDuplexMode, NULL) ||
-      !job_settings.GetInteger(printing::kSettingCopies, NULL)) {
+      !job_settings.GetInteger(printing::kSettingCopies, NULL) ||
+      !job_settings.GetInteger(printing::kPreviewRequestID, NULL)) {
     return;
   }
 
diff --git a/chrome/renderer/mock_render_thread.h b/chrome/renderer/mock_render_thread.h
index 5eb7ad3..c510d91 100644
--- a/chrome/renderer/mock_render_thread.h
+++ b/chrome/renderer/mock_render_thread.h
@@ -18,10 +18,11 @@ namespace IPC {
 class MessageReplyDeserializer;
 }
 
-struct PrintMsg_Print_Params;
-struct PrintMsg_PrintPages_Params;
+struct PrintHostMsg_DidGetPreviewPageCount_Params;
 struct PrintHostMsg_DidPreviewPage_Params;
 struct PrintHostMsg_ScriptedPrint_Params;
+struct PrintMsg_PrintPages_Params;
+struct PrintMsg_Print_Params;
 
 // This class is very simple mock of RenderThread. It simulates an IPC channel
 // which supports only two messages:
@@ -125,8 +126,8 @@ class MockRenderThread : public RenderThreadBase {
 
   void OnDidGetPrintedPagesCount(int cookie, int number_pages);
   void OnDidPrintPage(const PrintHostMsg_DidPrintPage_Params& params);
-  void OnDidGetPreviewPageCount(int document_cookie, int number_pages,
-                                bool is_modifiable);
+  void OnDidGetPreviewPageCount(
+      const PrintHostMsg_DidGetPreviewPageCount_Params& params);
   void OnDidPreviewPage(const PrintHostMsg_DidPreviewPage_Params& params);
 
   // For print preview, PrintWebViewHelper will update settings.
diff --git a/chrome/renderer/print_web_view_helper.cc b/chrome/renderer/print_web_view_helper.cc
index b065f31..dbf1ca5 100644
--- a/chrome/renderer/print_web_view_helper.cc
+++ b/chrome/renderer/print_web_view_helper.cc
@@ -319,7 +319,7 @@ void PrintWebViewHelper::OnPrintPreview(const DictionaryValue& settings) {
     return;
   }
 
-  if (print_pages_params_->params.preview_request_id != 0 &&
+  if (!print_pages_params_->params.is_first_request &&
       old_print_pages_params_.get() &&
       PrintMsg_Print_Params_IsEqual(*old_print_pages_params_,
                                     *print_pages_params_)) {
@@ -351,11 +351,12 @@ bool PrintWebViewHelper::CreatePreviewDocument() {
   const std::vector<int>& pages = print_pages_params_->pages;
   if (!print_preview_context_.CreatePreviewDocument(&print_params, pages))
     return false;
-  int page_count = print_preview_context_.total_page_count();
-  bool is_modifiable = print_preview_context_.IsModifiable();
-  int document_cookie = print_pages_params_->params.document_cookie;
-  Send(new PrintHostMsg_DidGetPreviewPageCount(routing_id(), document_cookie,
-                                               page_count, is_modifiable));
+  PrintHostMsg_DidGetPreviewPageCount_Params params;
+  params.page_count = print_preview_context_.total_page_count();
+  params.is_modifiable = print_preview_context_.IsModifiable();
+  params.document_cookie = print_pages_params_->params.document_cookie;
+  params.preview_request_id = print_pages_params_->params.preview_request_id;
+  Send(new PrintHostMsg_DidGetPreviewPageCount(routing_id(), params));
   PreviewPageRendered(printing::INVALID_PAGE_INDEX, NULL);
   return true;
 }
@@ -773,6 +774,11 @@ bool PrintWebViewHelper::UpdatePrintSettings(
   if (!UpdatePrintSettingsRequestId(job_settings, &(settings.params)))
     return false;
 
+  if (!job_settings.GetBoolean(printing::kIsFirstRequest,
+                               &(settings.params.is_first_request))) {
+    NOTREACHED();
+  }
+
   print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
   Send(new PrintHostMsg_DidGetDocumentCookie(routing_id(),
                                              settings.params.document_cookie));
@@ -931,6 +937,8 @@ void PrintWebViewHelper::PreviewPageRendered(int page_number,
   }
   preview_page_params.data_size = buf_size;
   preview_page_params.page_number = page_number;
+  preview_page_params.preview_request_id =
+      print_pages_params_->params.preview_request_id;
   Send(new PrintHostMsg_DidPreviewPage(routing_id(), preview_page_params));
 }
 
diff --git a/chrome/renderer/print_web_view_helper_browsertest.cc b/chrome/renderer/print_web_view_helper_browsertest.cc
index d897736..a4e9a59 100644
--- a/chrome/renderer/print_web_view_helper_browsertest.cc
+++ b/chrome/renderer/print_web_view_helper_browsertest.cc
@@ -42,6 +42,7 @@ void CreatePrintSettingsDictionary(DictionaryValue* dict) {
   dict->SetInteger(printing::kSettingCopies, 1);
   dict->SetString(printing::kSettingDeviceName, "dummy");
   dict->SetInteger(printing::kPreviewRequestID, 12345);
+  dict->SetBoolean(printing::kIsFirstRequest, true);
 }
 
 }  // namespace