PrintPreview: Preview generation should not block print button.

chrome/browser/resources/print_preview.js

Index: chrome/browser/resources/print_preview.js
diff --git a/chrome/browser/resources/print_preview.js b/chrome/browser/resources/print_preview.js
index e0265ab1d0bf30f6f981c84b433e7c228288b2a5..8f90d36d8034150c82bbc25ecbeefd4b021278cb 100644
--- a/chrome/browser/resources/print_preview.js
+++ b/chrome/browser/resources/print_preview.js
@@ -30,34 +30,41 @@ const MANAGE_PRINTERS = 'Manage Printers';
 // State of the print preview settings.
 var printSettings = new PrintSettings();
 
+// Total number of print preview requests submitted.

[dpapad, 2011/06/06 20:47:43]

This variable is being decremented in updatePrintPreview(). Change the comment
to "Total number of in-flight print preview requests submitted." or something
else to this effect.

[kmadhusu, 2011/06/06 21:23:56]

Done.

+var previewRequestCount = 0;
+
+// True when a pending print file request exists.
+var hasPendingPrintFileRequest = false;

[dpapad, 2011/06/06 20:47:43]

This variable is calculated based on previewRequestCount. I would prefer
removing it and checking directly for previewRequestCount > 0 wherever needed.

[kmadhusu, 2011/06/06 21:23:56]

(just repeating our in-person conversation) I need this global variable. It is
modified in printFile() function. It is used to track whether a printFile
request is submitted earlier while a preview data is loading.

+
 /**
  * Window onload handler, sets up the page and starts print preview by getting
  * the printer list.
  */
 function onLoad() {
-  $('system-dialog-link').addEventListener('click', showSystemDialog);
   $('cancel-button').addEventListener('click', handleCancelButtonClick);
 
   if (!checkCompatiblePluginExists()) {
     displayErrorMessage(localStrings.getString('noPlugin'), false);
     $('mainview').parentElement.removeChild($('dummy-viewer'));
+    $('print-button').addEventListener('click', showSystemDialog);

[dpapad, 2011/06/06 20:47:43]

I think for now it is better to add this listener $('system-dialog-link') as
before (until we hear back from the UI people).

[kmadhusu, 2011/06/06 21:23:56]

Done.

     return;
   }
+  $('system-dialog-link').addEventListener('click', showSystemDialog);
   $('mainview').parentElement.removeChild($('dummy-viewer'));
 
   $('printer-list').disabled = true;
-  $('print-button').disabled = true;
+  $('print-button').onclick = printFile;
+
+  addEventListenersToPagesControls();
+  addEventListenersToCopiesControls();
   showLoadingAnimation();
   chrome.send('getDefaultPrinter');
 }
 
 /**
- * Adds event listeners to the settings controls.
+ * Adds event listeners to the pages section controls.
  */
-function addEventListeners() {
-  $('print-button').onclick = printFile;
-
-  // Controls that require preview rendering.
+function addEventListenersToPagesControls() {
   $('all-pages').onclick = onPageSelectionMayHaveChanged;
   $('print-pages').onclick = handleIndividualPagesCheckbox;
   var individualPages = $('individual-pages');
@@ -67,19 +74,17 @@ function addEventListeners() {
   };
   individualPages.onfocus = addTimerToPageRangeField;
   individualPages.oninput = resetPageRangeFieldTimer;
-  $('landscape').onclick = onLayoutModeToggle;
-  $('portrait').onclick = onLayoutModeToggle;
-  $('printer-list').onchange = updateControlsWithSelectedPrinterCapabilities;
+}
 
-  // Controls that dont require preview rendering.
+/**
+ * Adds event listeners to the copies controls.
+ */
+function addEventListenersToCopiesControls() {
   $('copies').oninput = function() {
     copiesFieldChanged();
     updatePrintButtonState();
     updatePrintSummary();
   };
-  $('two-sided').onclick = handleTwoSidedClick;
-  $('color').onclick = function() { setColor(true); };
-  $('bw').onclick = function() { setColor(false); };
   $('increment').onclick = function() {
     onCopiesButtonsClicked(1);
     updatePrintButtonState();
@@ -93,29 +98,33 @@ function addEventListeners() {
 }
 
 /**
+ * Adds event listeners to the settings controls.
+ */
+function addEventListeners() {
+  // Controls that require preview rendering.
+  $('landscape').onclick = onLayoutModeToggle;
+  $('portrait').onclick = onLayoutModeToggle;
+  $('printer-list').onchange = updateControlsWithSelectedPrinterCapabilities;
+
+  // Controls that dont require preview rendering.
+  $('two-sided').onclick = handleTwoSidedClick;
+  $('color').onclick = function() { setColor(true); };
+  $('bw').onclick = function() { setColor(false); };
+}
+
+/**
  * Removes event listeners from the settings controls.
  */
 function removeEventListeners() {
   // Controls that require preview rendering.
-  $('print-button').disabled = true;
-  $('all-pages').onclick = null;
-  $('print-pages').onclick = null;
-  var individualPages = $('individual-pages');
-  individualPages.onblur = null;
-  individualPages.onfocus = null;
-  individualPages.oninput = null;
-  clearTimeout(timerId);
   $('landscape').onclick = null;
   $('portrait').onclick = null;
   $('printer-list').onchange = null;
 
   // Controls that dont require preview rendering.
-  $('copies').oninput = copiesFieldChanged;
   $('two-sided').onclick = null;
   $('color').onclick = null;
   $('bw').onclick = null;
-  $('increment').onclick = function() { onCopiesButtonsClicked(1); };
-  $('decrement').onclick = function() { onCopiesButtonsClicked(-1); };
 }
 
 /**
@@ -142,6 +151,7 @@ function onInitiatorTabClosed(initiatorTabURL) {
       window.location = initiatorTabURL;
   });
   displayErrorMessage(localStrings.getString('initiatorTabClosed'), true);
+  $('print-button').disabled = true;
 }
 
 /**
@@ -326,6 +336,14 @@ function getSelectedPrinterName() {
  * Asks the browser to print the preview PDF based on current print settings.
  */
 function printFile() {
+  hasPendingPrintFileRequest = previewRequestCount > 0;
+
+  if (hasPendingPrintFileRequest) {
+    if (getSelectedPrinterName() != PRINT_TO_PDF)
+      chrome.send('hidePreview');
+    return;
+  }
+
   if (getSelectedPrinterName() != PRINT_TO_PDF) {
     $('print-button').classList.add('loading');
     $('cancel-button').classList.add('loading');
@@ -333,14 +351,16 @@ function printFile() {
     removeEventListeners();
     window.setTimeout(function() { chrome.send('print', [getSettingsJSON()]); },
                       1000);
-  } else
+  } else {
     chrome.send('print', [getSettingsJSON()]);
+  }
 }
 
 /**
  * Asks the browser to generate a preview PDF based on current print settings.
  */
 function requestPrintPreview() {
+  previewRequestCount++;
   removeEventListeners();
   printSettings.save();
   showLoadingAnimation();
@@ -497,6 +517,9 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
 
   previewModifiable = modifiable;
 

[dpapad, 2011/06/06 20:47:43]

Add "var hasPendingPrintFileRequest = previewRequestCount > 0;" here since
hasPendingPrintFileRequest is no longer global (in case you followed my earlier
comment).

[kmadhusu, 2011/06/06 21:23:56]

Please refer to my reply for your earlier comment.

+  if (previewRequestCount > 0)
+    previewRequestCount--;
+
   if (totalPageCount == -1)
     totalPageCount = pageCount;
 
@@ -526,6 +549,12 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
   document.title = localStrings.getStringF('printPreviewTitleFormat', jobTitle);
 
   createPDFPlugin(previewUid);
+
+  if (previewRequestCount == 0 && hasPendingPrintFileRequest) {
+    printFile();
+    return;
+  }
+
   updatePrintSummary();
   updatePrintButtonState();
   addEventListeners();