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..d84009e1407ce84ad84a528371095455d24c4751 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.
+var previewRequestCount = 0;
+
+// True when a pending print file request exists.
+var hasPendingPrintFileRequest = false;
+
 /**
  * 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);
     return;
   }
+  $('system-dialog-link').addEventListener('click', showSystemDialog);

[dpapad, 2011/06/06 18:07:35]

Why move this line? If the there is no compatible plugin this line will never
get executed and Chromium users will not be able to print at all. 

[kmadhusu, 2011/06/06 18:44:08]

Print button is now enabled. In line #49, I have associated the print button to
show the system native dialog for Chromium users.

   $('mainview').parentElement.removeChild($('dummy-viewer'));
 
   $('printer-list').disabled = true;
-  $('print-button').disabled = true;
+  $('print-button').onclick = printFile;
+
+  addEventListenersToPagesControls();
+  addEventListenersToCopiesControl();
   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 addEventListenersToCopiesControl() {

[dpapad, 2011/06/06 18:07:35]

rename to addEventListenersToCopiesControls for consistency with the previous
function.

[kmadhusu, 2011/06/06 18:44:08]

Done.

   $('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,21 +336,36 @@ function getSelectedPrinterName() {
  * Asks the browser to print the preview PDF based on current print settings.
  */
 function printFile() {
+  $('print-summary').innerHTML = localStrings.getString('printing');

[dpapad, 2011/06/06 18:07:35]

I suspect that if you select Print to PDF, the hit print and then hit cancel the
print summary will keep displaying "Printing...". If so, move this line within
the "if (getSelectedPrinterName() != PRINT_TO_PDF)" block as before.

[kmadhusu, 2011/06/06 18:44:08]

Done.

+
+  if (previewRequestCount > 0)
+    hasPendingPrintFileRequest = true;
+  else
+    hasPendingPrintFileRequest = false;

[dpapad, 2011/06/06 18:07:35]

Replace if/else with
hasPendingPrintFileRequest = previewRequestCount > 0 ? true : false;

[kmadhusu, 2011/06/06 18:44:08]

Done.

+
+  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');
-    $('print-summary').innerHTML = localStrings.getString('printing');
+
     removeEventListeners();
-    window.setTimeout(function() { chrome.send('print', [getSettingsJSON()]); },
+    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 +522,9 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
 
   previewModifiable = modifiable;
 
+  if (previewRequestCount > 0)
+    previewRequestCount--;
+
   if (totalPageCount == -1)
     totalPageCount = pageCount;
 
@@ -526,6 +554,12 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
   document.title = localStrings.getStringF('printPreviewTitleFormat', jobTitle);
 
   createPDFPlugin(previewUid);
+
+  if (previewRequestCount == 0 && hasPendingPrintFileRequest) {
+    printFile();
+    return;
+  }
+
   updatePrintSummary();
   updatePrintButtonState();
   addEventListeners();