Print Preview: Go from event driven print preview back to print preview with sync messages. The s...

chrome/renderer/print_web_view_helper.cc

Index: chrome/renderer/print_web_view_helper.cc
===================================================================
--- chrome/renderer/print_web_view_helper.cc	(revision 97484)
+++ chrome/renderer/print_web_view_helper.cc	(working copy)
@@ -6,10 +6,8 @@
 
 #include <string>
 
-#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
-#include "base/process_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/print_messages.h"
@@ -33,6 +31,7 @@
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_POSIX)
+#include "base/process_util.h"
 #include "content/common/view_messages.h"
 #endif
 
@@ -371,8 +370,6 @@
     IPC_MESSAGE_HANDLER(PrintMsg_PrintingDone, OnPrintingDone)
     IPC_MESSAGE_HANDLER(PrintMsg_ResetScriptedPrintCount,
                         ResetScriptedPrintCount)
-    IPC_MESSAGE_HANDLER(PrintMsg_ContinuePreview, OnContinuePreview)
-    IPC_MESSAGE_HANDLER(PrintMsg_AbortPreview, OnAbortPreview)
     IPC_MESSAGE_HANDLER(PrintMsg_PreviewPrintingRequestCancelled,
                         DisplayPrintJobError)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -495,8 +492,11 @@
 
   // PDF printer device supports alpha blending.
   print_pages_params_->params.supports_alpha_blend = true;
-  if (!CreatePreviewDocument())
+  if (CreatePreviewDocument()) {

[kmadhusu, 2011/08/19 19:51:03]

nit: Remove {}

[Lei Zhang, 2011/08/19 22:50:28]

Done.

+    DidFinishPrinting(OK);
+  } else {
     DidFinishPrinting(FAIL_PREVIEW);
+  }
 }
 
 bool PrintWebViewHelper::CreatePreviewDocument() {
@@ -510,44 +510,22 @@
   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;
-}
+  if (!PreviewPageRendered(printing::INVALID_PAGE_INDEX, NULL))
+    return false;
 
-void PrintWebViewHelper::OnContinuePreview(int requested_preview_page_index) {
-  // Spurious message. We already finished/cancelled/aborted the print preview.
-  if (!print_preview_context_.IsBusy())
-    return;
   int page_number;
-#if defined(USE_SKIA)
-  if (requested_preview_page_index >= printing::FIRST_PAGE_INDEX) {
-    page_number = requested_preview_page_index;
-  } else
-#endif
-  {
-    page_number = print_preview_context_.GetNextPageNumber();
-  }
+  while ((page_number = print_preview_context_.GetNextPageNumber()) >= 0) {
+    if (!RenderPreviewPage(page_number))
+      return false;
+  };
 
-  if (page_number >= printing::FIRST_PAGE_INDEX) {
-    // Continue generating the print preview.
-    RenderPreviewPage(page_number);
-    return;
-  }
-
   // Finished generating preview. Finalize the document.
-  if (FinalizePreviewDocument()) {
-    print_preview_context_.Finished();
-    DidFinishPrinting(OK);
-  } else {
-    DidFinishPrinting(FAIL_PREVIEW);
-  }
+  if (!FinalizePreviewDocument())
+    return false;
+  print_preview_context_.Finished();
+  return true;
 }
 
-void PrintWebViewHelper::OnAbortPreview() {
-  DidFinishPrinting(ABORT_PREVIEW);
-  return;
-}
-
 bool PrintWebViewHelper::FinalizePreviewDocument() {
   print_preview_context_.FinalizePreviewDocument();
 
@@ -660,12 +638,12 @@
     DCHECK(is_preview_);
     store_print_pages_params = false;
     int cookie = print_pages_params_->params.document_cookie;
-    Send(new PrintHostMsg_PrintPreviewFailed(routing_id(), cookie));
+    if (notify_browser_of_print_failure_) {

[kmadhusu, 2011/08/19 19:51:03]

nit: Remove {}

[Lei Zhang, 2011/08/19 22:50:28]

Done.

+      Send(new PrintHostMsg_PrintPreviewFailed(routing_id(), cookie));
+    } else {
+      Send(new PrintHostMsg_PrintPreviewCancelled(routing_id(), cookie));
+    }
     print_preview_context_.Failed();
-  } else if (result == ABORT_PREVIEW) {
-    DCHECK(is_preview_);
-    store_print_pages_params = false;
-    print_preview_context_.Abort();
   }
 
   if (print_web_view_) {
@@ -903,17 +881,6 @@
   return true;
 }
 
-bool PrintWebViewHelper::UpdatePrintSettingsRequestId(
-    const DictionaryValue& job_settings,
-    PrintMsg_Print_Params* params) {
-  if (!job_settings.GetInteger(printing::kPreviewRequestID,
-                               &(params->preview_request_id))) {
-    NOTREACHED();
-    return false;
-  }
-  return true;
-}
-
 bool PrintWebViewHelper::UpdatePrintSettings(
     const DictionaryValue& job_settings) {
   PrintMsg_PrintPages_Params settings;
@@ -924,12 +891,22 @@
   if (settings.params.dpi < kMinDpi || !settings.params.document_cookie)
     return false;
 
-  if (!UpdatePrintSettingsRequestId(job_settings, &(settings.params)))
+  if (!job_settings.GetString(printing::kPreviewUIAddr,

[kmadhusu, 2011/08/19 19:51:03]

Can you merge these 3 if conditions (line #894, 900, 906)?

[Lei Zhang, 2011/08/19 22:50:28]

Done.

+                              &(settings.params.preview_ui_addr))) {
+    NOTREACHED();
     return false;
+  }
 
+  if (!job_settings.GetInteger(printing::kPreviewRequestID,
+                               &(settings.params.preview_request_id))) {
+    NOTREACHED();
+    return false;
+  }
+
   if (!job_settings.GetBoolean(printing::kIsFirstRequest,
                                &(settings.params.is_first_request))) {
     NOTREACHED();
+    return false;
   }
 
   print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
@@ -1039,9 +1016,6 @@
     }
   }
 
-  if (!too_frequent && print_preview_context_.IsBusy())
-    too_frequent = true;
-
   if (!too_frequent)
     return false;
 
@@ -1077,16 +1051,28 @@
   Send(new PrintHostMsg_RequestPrintPreview(routing_id()));
 }
 
-void PrintWebViewHelper::PreviewPageRendered(int page_number,
+bool PrintWebViewHelper::PreviewPageRendered(int page_number,

[kmadhusu, 2011/08/19 19:51:03]

PreviewPageRendered() not only sends the rendered page but also checks for
cancelling. I think its better to create a function just to check for cancelling
so that this function always send the rendered page. 

[Lei Zhang, 2011/08/19 22:50:28]

Done.

                                              printing::Metafile* metafile) {
   if ((page_number == printing::INVALID_PAGE_INDEX && metafile) ||
        (page_number >= printing::FIRST_PAGE_INDEX && !metafile &&
         print_preview_context_.IsModifiable())) {
     NOTREACHED();
-    DidFinishPrinting(FAIL_PREVIEW);
-    return;
+    return false;
   }
 
+  // First check whether to cancel or not.
+  bool cancel = false;
+  Send(new PrintHostMsg_CheckForCancel(
+      routing_id(),
+      print_pages_params_->params.preview_ui_addr,
+      print_pages_params_->params.preview_request_id,
+      &cancel));
+  if (cancel) {
+    notify_browser_of_print_failure_ = false;
+    return false;
+  }
+
+  // Not cancelling, send the page data.
   uint32 buf_size = 0;
   PrintHostMsg_DidPreviewPage_Params preview_page_params;
   // Get the size of the resulting metafile.
@@ -1095,22 +1081,23 @@
     DCHECK_GT(buf_size, 0u);
     if (!CopyMetafileDataToSharedMem(
             metafile, &(preview_page_params.metafile_data_handle))) {
-      DidFinishPrinting(FAIL_PREVIEW);
-      return;
+      return false;
     }
   }
   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));
+  return true;

[kmadhusu, 2011/08/19 19:51:03]

Previously, We were sending an empty draft metafile if the preview is not
modifiable to continue rendering. Now we can avoid that message completely. 

[Lei Zhang, 2011/08/19 22:50:28]

Done.

 }
 
 PrintWebViewHelper::PrintPreviewContext::PrintPreviewContext()
     : frame_(NULL),
       total_page_count_(0),
       actual_page_count_(0),
-      current_page_number_(0),
+      current_page_index_(0),
       state_(UNINITIALIZED) {
 }
 
@@ -1120,8 +1107,6 @@
 void PrintWebViewHelper::PrintPreviewContext::InitWithFrame(
     WebKit::WebFrame* web_frame) {
   DCHECK(web_frame);
-  if (IsReadyToRender())
-    return;
   state_ = INITIALIZED;
   frame_ = web_frame;
   node_.reset();
@@ -1130,8 +1115,6 @@
 void PrintWebViewHelper::PrintPreviewContext::InitWithNode(
     const WebKit::WebNode& web_node) {
   DCHECK(!web_node.isNull());
-  if (IsReadyToRender())
-    return;
   state_ = INITIALIZED;
   frame_ = web_node.document().frame();
   node_.reset(new WebNode(web_node));
@@ -1164,23 +1147,21 @@
   if (total_page_count_ == 0)
     return false;
 
-  current_page_number_ = 0;
+  current_page_index_ = 0;
   if (pages.empty()) {
     actual_page_count_ = total_page_count_;
-    rendered_pages_ = std::vector<PreviewPageInfo>(total_page_count_,
-                                                   std::make_pair(false, -1));
+    for (int i = 0; i < actual_page_count_; ++i) {

[kmadhusu, 2011/08/19 19:51:03]

nit: Remove {}

[Lei Zhang, 2011/08/19 22:50:28]

Done.

+      pages_to_render_.push_back(i);
+    }
   } else {
     actual_page_count_ = pages.size();
-    rendered_pages_ = std::vector<PreviewPageInfo>(total_page_count_,
-                                                   std::make_pair(true, -1));
     for (int i = 0; i < actual_page_count_; ++i) {
       int page_number = pages[i];
       if (page_number < printing::FIRST_PAGE_INDEX ||
           page_number >= total_page_count_) {
         return false;
       }
-      rendered_pages_[page_number].first = false;
-      rendered_pages_[page_number].second = i;
+      pages_to_render_.push_back(page_number);
     }
   }
 
@@ -1227,39 +1208,22 @@
 }
 
 void PrintWebViewHelper::PrintPreviewContext::Failed() {
-  DCHECK(IsBusy());
+  DCHECK(state_ == INITIALIZED || state_ == RENDERING);
   state_ = INITIALIZED;
   ClearContext();
 }
 
-void PrintWebViewHelper::PrintPreviewContext::Abort() {
-  state_ = UNINITIALIZED;
-  ClearContext();
-  frame_ = NULL;
-  node_.reset();
-}
-
 int PrintWebViewHelper::PrintPreviewContext::GetNextPageNumber() {
   DCHECK_EQ(RENDERING, state_);
-  for (int i = 0; i < total_page_count_; i++) {
-    if (!rendered_pages_[current_page_number_].first)
-      break;
-    current_page_number_ = (current_page_number_ + 1) % total_page_count_;
-  }
-  if (rendered_pages_[current_page_number_].first)
-    return printing::INVALID_PAGE_INDEX;
-  rendered_pages_[current_page_number_].first = true;
-  return current_page_number_;
+  if (current_page_index_ >= actual_page_count_)
+    return -1;
+  return pages_to_render_[current_page_index_++];
 }
 
 bool PrintWebViewHelper::PrintPreviewContext::IsReadyToRender() const {
   return state_ != UNINITIALIZED;
 }
 
-bool PrintWebViewHelper::PrintPreviewContext::IsBusy() const {
-  return state_ == INITIALIZED || state_ == RENDERING;
-}
-
 bool PrintWebViewHelper::PrintPreviewContext::IsModifiable() const {
   // TODO(vandebo) I think this should only return false if the content is a
   // PDF, just because we are printing a particular node does not mean it's
@@ -1300,5 +1264,8 @@
 void PrintWebViewHelper::PrintPreviewContext::ClearContext() {
   prep_frame_view_.reset();
   metafile_.reset();
-  rendered_pages_.clear();
+  pages_to_render_.clear();
+  total_page_count_ = 0;
+  actual_page_count_ = 0;
+  current_page_index_ = 0;
 }