Print Preview: Associating preview requests and responses using an identifier

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 1b2a509adabf5f29bf21ebda517216fefdc39316..3f79e2694441259b6b4945d6ca589b74fcdde46b 100644
--- a/chrome/renderer/print_web_view_helper.cc
+++ b/chrome/renderer/print_web_view_helper.cc
@@ -18,6 +18,7 @@
 #include "content/renderer/render_view.h"
 #include "grit/generated_resources.h"
 #include "printing/metafile.h"
+#include "printing/print_job_constants.h"
 #include "printing/units.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebConsoleMessage.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDataSource.h"
@@ -378,6 +379,8 @@ void PrintWebViewHelper::PrintPreview(WebKit::WebFrame* frame,
         print_pages_params_->params.document_cookie;
     preview_params.expected_pages_count = preview_page_count_;
     preview_params.modifiable = IsModifiable(frame, node);
+    settings.GetInteger(printing::kPreviewRequestID,

[Lei Zhang, 2011/07/12 23:19:30]

Actually, why do we have to do this again here? Didn't we fetch the request id
and stored it in |print_pages_params| on line 668?

[dpapad, 2011/07/12 23:42:29]

This handles the case when there is no rerendering. At this point
|print_pages_params_| contains the id of the previous request (if any), right?

In any case |preview_params| is the struct being sent to the js code that
results in a call to updatePrintPreview(). We need to inject the request ID in
that struct. That is why I had to add preview_request_id in 2 places in
chrome/common/print_messages.h.

[Lei Zhang, 2011/07/12 23:58:40]

We just called UpdatePrintSettings(), which should have the most recent request
id in print_pages_params_, right? What I'm suggesting is to just assign it
directly:

preview_params.preview_request_id =
print_pages_params_.params.preview_request_id;

[dpapad, 2011/07/13 00:15:45]

Done, you are right, I confused the order of execution.

+                        &preview_params.preview_request_id);
 
     Send(new PrintHostMsg_PagesReadyForPreview(routing_id(), preview_params));
     return;
@@ -648,12 +651,20 @@ bool PrintWebViewHelper::InitPrintSettings(WebKit::WebFrame* frame,
 bool PrintWebViewHelper::UpdatePrintSettings(
     const DictionaryValue& job_settings) {
   PrintMsg_PrintPages_Params settings;
+  if (!job_settings.GetInteger(printing::kPreviewRequestID,
+                               &settings.params.preview_request_id)) {
+    NOTREACHED();
+  }
+
   Send(new PrintHostMsg_UpdatePrintSettings(routing_id(),
       print_pages_params_->params.document_cookie, job_settings, &settings));
 
   if (!settings.params.dpi || !settings.params.document_cookie)
     return false;
 
+  job_settings.GetInteger(printing::kPreviewRequestID,
+                          &settings.params.preview_request_id);

[dpapad, 2011/07/12 23:02:11]

I still have to do this, because it seems that |settings| is getting overwritten
in line 660.

[Lei Zhang, 2011/07/12 23:19:30]

Oh, in that case just ignore my comment about doing the check early and put this
back the way it was.

[dpapad, 2011/07/12 23:42:29]

Done.

+
   print_pages_params_.reset(new PrintMsg_PrintPages_Params(settings));
   Send(new PrintHostMsg_DidGetDocumentCookie(routing_id(),
                                              settings.params.document_cookie));