Print preview page selection should not require a rerendering of draft pages.

chrome/browser/resources/print_preview/print_preview.js

Index: chrome/browser/resources/print_preview/print_preview.js
diff --git a/chrome/browser/resources/print_preview/print_preview.js b/chrome/browser/resources/print_preview/print_preview.js
index 35ec000338f2f92da3c38462ea19c040f2c210a9..df0fc1c965e7401d49b21bfd82c80df627937ad5 100644
--- a/chrome/browser/resources/print_preview/print_preview.js
+++ b/chrome/browser/resources/print_preview/print_preview.js
@@ -62,6 +62,11 @@ var showingSystemDialog = false;
 var firstCloudPrintOptionPos = 0;
 var lastCloudPrintOptionPos = firstCloudPrintOptionPos;
 
+// Store the current previewUid.
+var currentPreviewUid = '';
+
+// True if we need to generate draft preview data.
+var generateDraftData = true;
 
 // TODO(abodenha@chromium.org) A lot of cloud print specific logic has
 // made its way into this file.  Refactor to create a cleaner boundary
@@ -311,7 +316,8 @@ function getSettings() {
        'landscape': layoutSettings.isLandscape(),
        'color': colorSettings.isColor(),
        'printToPDF': printToPDF,
-       'requestID': 0};
+       'requestID': 0,
+       'generateDraftData': generateDraftData};
 
   var printerList = $('printer-list');
   var selectedPrinter = printerList.selectedIndex;
@@ -421,15 +427,39 @@ function sendPrintDocumentRequest() {
 }
 
 /**
+ * Loads the selected preview pages.
+ */
+function LoadSelectedPages() {

[dpapad, 2011/08/01 17:51:40]

Nit: Function names start with lowercase. Also maybe a more descriptive name
would be displaySelectedPages.

[kmadhusu, 2011/08/02 22:53:45]

Done
I felt loadSelectedPages is more apt for this function, since we are asking the
plugin to load the preview urls.

+  hasPendingPreviewRequest = false;
+  pageSettings.updatePageSelection();
+  var pageSet = pageSettings.previouslySelectedPages;
+  var pageCount = pageSet.length;
+  if (pageCount == 0 || currentPreviewUid == '')
+    return;
+
+  for (var i =0; i < pageCount; i++)
+    onDidPreviewPage(pageSet[i]-1, currentPreviewUid);
+  addEventListeners();
+}
+
+/**
  * Asks the browser to generate a preview PDF based on current print settings.
  */
 function requestPrintPreview() {
   hasPendingPreviewRequest = true;
   removeEventListeners();
   printSettings.save();
+  generateDraftData = true;
   if (!isTabHidden)
     showLoadingAnimation();
 
+  if (previewModifiable && isOnlyPageSettingsChanged()) {
+    LoadSelectedPages();
+    generateDraftData = false;
+  }
+  if (!previewModifiable && pageSettings.totalPageCount > 0)
+    generateDraftData = false;
+
   var settings = getSettings();
   settings.requestID = generatePreviewRequestID();
   chrome.send('getPreview', [JSON.stringify(settings)]);
@@ -833,8 +863,13 @@ function onDidPreviewPage(pageNumber, previewUid) {
 
   if (checkIfSettingsChangedAndRegeneratePreview())
     return;
+
+  if (pageIndex == -1)

[dpapad, 2011/08/01 17:51:40]

Is this possible?

[kmadhusu, 2011/08/02 22:53:45]

Yes it is possible now. When the user changes the layout and enter some page
ranges, we generate the draft pages for all preview pages. We only need to load
the requested selected pages. 

+    return;
+
+  currentPreviewUid = previewUid;
   if (pageIndex == 0)
-    createPDFPlugin(previewUid);
+    createPDFPlugin(pageNumber);
 
   $('pdf-viewer').loadPreviewPage(
       getPageSrcURL(previewUid, pageNumber), pageIndex);
@@ -856,6 +891,8 @@ function updatePrintPreview(jobTitle,
   if (!isExpectedPreviewResponse(previewResponseId))
     return;
   hasPendingPreviewRequest = false;
+  if (!previewModifiable)
+    pageSettings.updatePageSelection();
 
   if (checkIfSettingsChangedAndRegeneratePreview())
     return;
@@ -864,7 +901,8 @@ function updatePrintPreview(jobTitle,
 
   if (!previewModifiable) {
     // If the preview is not modifiable the plugin has not been created yet.
-    createPDFPlugin(previewUid);
+    currentPreviewUid = previewUid;
+    createPDFPlugin(-1);

[dpapad, 2011/08/01 17:51:40]

Create a constant
const ALL_PAGES = -1;
and use 
createPDFPlugin(ALL_PAGES);
for more readability.

[kmadhusu, 2011/08/02 22:53:45]

created const COMPLETE_METAFILE_DATA_INDEX

   }
 
   cr.dispatchSimpleEvent(document, 'updateSummary');
@@ -898,28 +936,41 @@ function checkIfSettingsChangedAndRegeneratePreview() {
 }
 
 /**
+ * Check if only page selection has been changed and is valid.

[dpapad, 2011/08/01 17:51:40]

Document since when the page selection is being checked (since last preview
request?).

[kmadhusu, 2011/08/02 22:53:45]

Done.

+ * @return {boolean} true if the nwe page selection is valid.

[dpapad, 2011/08/01 17:51:40]

s/nwe/new

[kmadhusu, 2011/08/02 22:53:45]

Done.

+ */
+function isOnlyPageSettingsChanged() {

[dpapad, 2011/08/01 17:51:40]

Nit (optional): Maybe haveOnlyPageSettingsChanged is more correct grammatically.

[kmadhusu, 2011/08/02 22:53:45]

changed to hasOnlyPageSettingsChanged

+  var tempPrintSettings = new PrintSettings();
+  tempPrintSettings.save();
+
+  return !!(printSettings.deviceName == tempPrintSettings.deviceName &&
+            printSettings.isLandscape == tempPrintSettings.isLandscape &&
+            pageSettings.hasPageSelectionChangedAndIsValid());
+}
+
+/**
  * Create the PDF plugin or reload the existing one.
- * @param {string} previewUid Preview unique identifier.
+ * @param {number} srcDataIndex Preview data source index.

[dpapad, 2011/08/01 17:51:40]

Document the meaning of -1.

[kmadhusu, 2011/08/02 22:53:45]

I think the const variable defined here is descriptive.

  */
-function createPDFPlugin(previewUid) {
+function createPDFPlugin(srcDataIndex) {
   var pdfViewer = $('pdf-viewer');
+  var srcURL = getPageSrcURL(currentPreviewUid, srcDataIndex);
   if (pdfViewer) {
     // Need to call this before the reload(), where the plugin resets its
     // internal page count.
     pdfViewer.goToPage('0');
+    pdfViewer.resetPrintPreviewUrl(srcURL);
     pdfViewer.reload();
     pdfViewer.grayscale(!colorSettings.isColor());
     return;
   }
 
-  // Get the complete preview document.
-  var dataIndex = previewModifiable ? '0' : '-1';
-
   pdfViewer = document.createElement('embed');
   pdfViewer.setAttribute('id', 'pdf-viewer');
+
   pdfViewer.setAttribute('type',
                          'application/x-google-chrome-print-preview-pdf');
-  pdfViewer.setAttribute('src', getPageSrcURL(previewUid, dataIndex));
+  pdfViewer.setAttribute('src', srcURL);
   pdfViewer.setAttribute('aria-live', 'polite');
   pdfViewer.setAttribute('aria-atomic', 'true');
   $('mainview').appendChild(pdfViewer);