PrintPreview: Preview generation should not block print button.

Initial set of changes to enable the print button while the preview data is loading. This CL does not handle any print preview errors.

BUG=84127
TEST=Press Ctrl+p on a webpage. Observe the print button state in the preview tab.

Review URL: http://codereview.chromium.org/7056070

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88669 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/printing/background_printing_manager.cc

@@ -98,4 +98,8 @@ std::set<TabContentsWrapper*>::const_iterator
   return printing_contents_.end();
 }
 
+bool BackgroundPrintingManager::HasTabContents(TabContentsWrapper* entry) {
+  return printing_contents_.find(entry) != printing_contents_.end();
+}
+
 }  // namespace printing

chrome/browser/printing/background_printing_manager.h

@@ -36,6 +36,9 @@ class BackgroundPrintingManager
   std::set<TabContentsWrapper*>::const_iterator begin();
   std::set<TabContentsWrapper*>::const_iterator end();
 
+  // Returns true if |printing_contents_| contains |entry|.
+  bool HasTabContents(TabContentsWrapper* entry);
+
   // NotificationObserver overrides:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,

chrome/browser/printing/print_preview_message_handler.cc

@@ -6,6 +6,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/printing/background_printing_manager.h"
 #include "chrome/browser/printing/print_job_manager.h"
 #include "chrome/browser/printing/print_preview_tab_controller.h"
 #include "chrome/browser/printing/print_view_manager.h"
@@ -114,7 +115,19 @@ void PrintPreviewMessageHandler::OnPrintPreviewFailed(int document_cookie) {
   // User might have closed it already.
   if (!print_preview_tab)
     return;
-  print_preview_tab->web_ui()->CallJavascriptFunction("printPreviewFailed");
+
+  TabContentsWrapper* wrapper =
+      TabContentsWrapper::GetCurrentWrapperForContents(print_preview_tab);
+
+  if (g_browser_process->background_printing_manager()->
+          HasTabContents(wrapper)) {
+    // Preview tab was hidden to serve the print request.
+    delete wrapper;
+  } else {
+    PrintPreviewUI* print_preview_ui =
+      static_cast<PrintPreviewUI*>(print_preview_tab->web_ui());
+    print_preview_ui->CallJavascriptFunction("printPreviewFailed");
+  }
 }
 
 bool PrintPreviewMessageHandler::OnMessageReceived(

chrome/browser/printing/print_preview_tab_controller.cc

@@ -158,6 +158,16 @@ bool PrintPreviewTabController::IsPrintPreviewTab(TabContents* tab) {
           url.host() == chrome::kChromeUIPrintHost);
 }
 
+void PrintPreviewTabController::EraseInitiatorTabInfo(
+    TabContents* preview_tab) {
+  PrintPreviewTabMap::iterator it = preview_tab_map_.find(preview_tab);
+  if (it == preview_tab_map_.end())
+    return;
+
+  RemoveObservers(it->second);
+  preview_tab_map_[preview_tab] = NULL;
+}
+
 TabContents* PrintPreviewTabController::GetInitiatorTab(
     TabContents* preview_tab) {
   PrintPreviewTabMap::iterator it = preview_tab_map_.find(preview_tab);

chrome/browser/printing/print_preview_tab_controller.h

@@ -60,6 +60,9 @@ class PrintPreviewTabController
   // Returns true if |tab| is a print preview tab.
   static bool IsPrintPreviewTab(TabContents* tab);
 
+  // Erase the initiator tab info associated with |preview_tab|.
+  void EraseInitiatorTabInfo(TabContents* preview_tab);
+
  private:
   friend class base::RefCounted<PrintPreviewTabController>;
 

chrome/browser/printing/print_preview_tab_controller_unittest.cc

@@ -111,3 +111,45 @@ TEST_F(PrintPreviewTabControllerTest, MultiplePreviewTabs) {
   tab_controller->GetOrCreatePreviewTab(tab_contents_1);
   EXPECT_EQ(preview_tab_1_index, browser()->active_index());
 }
+
+// Clear the initiator tab details associated with preview tab.
+TEST_F(PrintPreviewTabControllerTest, ClearInitiatorTabDetails) {
+  ASSERT_TRUE(browser());
+  BrowserList::SetLastActive(browser());
+  ASSERT_TRUE(BrowserList::GetLastActive());
+
+  // Lets start with one window with one tab.
+  EXPECT_EQ(1u, BrowserList::size());
+  EXPECT_EQ(0, browser()->tab_count());
+  browser()->NewTab();
+  EXPECT_EQ(1, browser()->tab_count());
+
+  // Create a reference to initiator tab contents.
+  TabContents* initiator_tab = browser()->GetSelectedTabContents();
+
+  scoped_refptr<printing::PrintPreviewTabController>
+      tab_controller(new printing::PrintPreviewTabController());
+  ASSERT_TRUE(tab_controller);
+
+  // Get the preview tab for initiator tab.
+  TabContents* preview_tab =
+      tab_controller->GetOrCreatePreviewTab(initiator_tab);
+
+  // New print preview tab is created. Current focus is on preview tab.
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_NE(initiator_tab, preview_tab);
+
+  // Clear the initiator tab details associated with the preview tab.
+  tab_controller->EraseInitiatorTabInfo(preview_tab);
+
+  // Activate initiator_tab.
+  initiator_tab->Activate();
+
+  // Get the print preview tab for initiator tab.
+  TabContents* new_preview_tab =
+      tab_controller->GetOrCreatePreviewTab(initiator_tab);
+
+  // New preview tab is created.
+  EXPECT_EQ(3, browser()->tab_count());
+  EXPECT_NE(new_preview_tab, preview_tab);
+}

chrome/browser/resources/print_preview.js

@@ -33,6 +33,15 @@ var printSettings = new PrintSettings();
 // The name of the default or last used printer.
 var defaultOrLastUsedPrinterName = '';
 
+// True when a pending print preview request exists.
+var hasPendingPreviewRequest = false;
+
+// True when a pending print file request exists.
+var hasPendingPrintFileRequest = false;
+
+// True when preview tab has some error.
+var hasError = false;
+
 /**
  * Window onload handler, sets up the page and starts print preview by getting
  * the printer list.
@@ -51,17 +60,59 @@ function onLoad() {
   $('mainview').parentElement.removeChild($('dummy-viewer'));
 
   $('printer-list').disabled = true;
-  $('print-button').disabled = true;
+  $('print-button').onclick = printFile;
+
+  setDefaultHandlersForPagesAndCopiesControls();
   showLoadingAnimation();
   chrome.send('getDefaultPrinter');
 }
 
+/**
+ * Handles the individual pages input event.
+ */
+function handleIndividualPagesInputEvent() {
+  $('print-pages').checked = true;
+  resetPageRangeFieldTimer();
+}
+
+/**
+ * Handles the individual pages blur event.
+ */
+function onPageRangesFieldBlur() {
+  $('print-pages').checked = true;
+  validatePageRangesField();
+  updatePrintButtonState();
+}
+
+/**
+ * Sets the default event handlers for pages and copies controls.
+ */
+function setDefaultHandlersForPagesAndCopiesControls() {
+  var allPages = $('all-pages');
+  var printPages = $('print-pages');
+  var individualPages = $('individual-pages');
+
+  allPages.onclick = null;
+  printPages.onclick = null;
+  individualPages.oninput = null;
+  individualPages.onfocus = null;
+  individualPages.onblur = null;
+
+  if (!hasError) {
+    allPages.onclick = updatePrintButtonState;
+    printPages.onclick = handleIndividualPagesCheckbox;
+    individualPages.onblur = onPageRangesFieldBlur;
+  }
+
+  $('copies').oninput = copiesFieldChanged;
+  $('increment').onclick = function() { onCopiesButtonsClicked(1); };
+  $('decrement').onclick = function() { onCopiesButtonsClicked(-1); };
+}
+
 /**
  * Adds event listeners to the settings controls.
  */
 function addEventListeners() {
-  $('print-button').onclick = printFile;
-
   // Controls that require preview rendering.
   $('all-pages').onclick = onPageSelectionMayHaveChanged;
   $('print-pages').onclick = handleIndividualPagesCheckbox;
@@ -71,7 +122,7 @@ function addEventListeners() {
       onPageSelectionMayHaveChanged();
   };
   individualPages.onfocus = addTimerToPageRangeField;
-  individualPages.oninput = resetPageRangeFieldTimer;
+  individualPages.oninput = handleIndividualPagesInputEvent;
   $('landscape').onclick = onLayoutModeToggle;
   $('portrait').onclick = onLayoutModeToggle;
   $('printer-list').onchange = updateControlsWithSelectedPrinterCapabilities;
@@ -101,26 +152,18 @@ function addEventListeners() {
  * 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);
+  setDefaultHandlersForPagesAndCopiesControls();
+
+  // Controls that require preview rendering
   $('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); };
 }
 
 /**
@@ -330,6 +373,14 @@ function getSelectedPrinterName() {
  * Asks the browser to print the preview PDF based on current print settings.
  */
 function printFile() {
+  hasPendingPrintFileRequest = hasPendingPreviewRequest;
+
+  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');
@@ -337,14 +388,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() {
+  hasPendingPreviewRequest = true;
   removeEventListeners();
   printSettings.save();
   showLoadingAnimation();
@@ -440,6 +493,8 @@ function setColor(color) {
  * @param {string} errorMessage The error message to be displayed.
  */
 function displayErrorMessage(errorMessage) {
+  hasError = true;
+  $('print-button').disabled = true;
   $('overlay-layer').classList.remove('invisible');
   $('dancing-dots-text').classList.add('hidden');
   $('error-text').innerHTML = errorMessage;
@@ -509,6 +564,8 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
 
   previewModifiable = modifiable;
 
+  hasPendingPreviewRequest = false;
+
   if (totalPageCount == -1)
     totalPageCount = pageCount;
 
@@ -541,6 +598,9 @@ function updatePrintPreview(pageCount, jobTitle, modifiable, previewUid) {
   updatePrintSummary();
   updatePrintButtonState();
   addEventListeners();
+
+  if (hasPendingPrintFileRequest)
+    printFile();
 }
 
 /**
@@ -612,13 +672,9 @@ function copiesFieldChanged() {
 }
 
 /**
- * Executes whenever a blur event occurs on the 'individual-pages'
- * field or when the timer expires. It takes care of
- * 1) showing/hiding warnings/suggestions
- * 2) updating print button/summary
+ * Validates the page ranges text and updates the hint accordingly.
  */
-function pageRangesFieldChanged() {
-  var currentlySelectedPages = getSelectedPagesSet();
+function validatePageRangesField() {
   var individualPagesField = $('individual-pages');
   var individualPagesHint = $('individual-pages-hint');
 
@@ -634,6 +690,16 @@ function pageRangesFieldChanged() {
                                     'examplePageRangeText'));
     fadeInElement(individualPagesHint);
   }
+}
+
+/**
+ * Executes whenever a blur event occurs on the 'individual-pages'
+ * field or when the timer expires. It takes care of
+ * 1) showing/hiding warnings/suggestions
+ * 2) updating print button/summary
+ */
+function pageRangesFieldChanged() {
+  validatePageRangesField();
 
   resetPageRangeFieldTimer();
   updatePrintButtonState();
@@ -812,13 +878,17 @@ function isSelectedPagesValid() {
 
     var match = part.match(/^([0-9]+)-([0-9]*)$/);
     if (match && isValidNonZeroPositiveInteger(match[1])) {
+      if (!match[2] && totalPageCount == -1) {
+        successfullyParsed += 1;
+        continue;
+      }
       var from = parseInt(match[1], 10);
       var to = match[2] ? parseInt(match[2], 10) : totalPageCount;
 
       if (!to || from > to)
         return false;
-    } else if (!isValidNonZeroPositiveInteger(part) ||
-               !(parseInt(part, 10) <= totalPageCount)) {
+    } else if (!isValidNonZeroPositiveInteger(part) || (totalPageCount != -1 &&
+               !(parseInt(part, 10) <= totalPageCount))) {
       return false;
     }
     successfullyParsed += 1;

chrome/browser/ui/webui/print_preview_handler.cc

@@ -152,6 +152,10 @@ void ReportPrintSettingsStats(const DictionaryValue& settings) {
     ReportPrintSettingHistogram(is_color ? COLOR : BLACK_AND_WHITE);
 }
 
+printing::BackgroundPrintingManager* GetBackgroundPrintingManager() {
+  return g_browser_process->background_printing_manager();
+}
+
 }  // namespace
 
 class PrintSystemTaskProxy
@@ -374,6 +378,8 @@ void PrintPreviewHandler::RegisterMessages() {
       NewCallback(this, &PrintPreviewHandler::HandleManagePrinters));
   web_ui_->RegisterMessageCallback("closePrintPreviewTab",
       NewCallback(this, &PrintPreviewHandler::HandleClosePreviewTab));
+  web_ui_->RegisterMessageCallback("hidePreview",
+      NewCallback(this, &PrintPreviewHandler::HandleHidePreview));
 }
 
 TabContents* PrintPreviewHandler::preview_tab() {
@@ -478,12 +484,13 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
 
     SelectFile(default_filename);
   } else {
+    ClearInitiatorTabDetails();
     ReportPrintSettingsStats(*settings);
     ReportUserActionHistogram(PRINT_TO_PRINTER);
     UMA_HISTOGRAM_COUNTS("PrintPreview.PageCount.PrintToPrinter",
                          GetPageCountFromSettingsDictionary(*settings));
-    g_browser_process->background_printing_manager()->OwnTabContents(
-        preview_tab_wrapper);
+
+    HidePreviewTab();
 
     // The PDF being printed contains only the pages that the user selected,
     // so ignore the page range and print all pages.
@@ -493,6 +500,10 @@ void PrintPreviewHandler::HandlePrint(const ListValue* args) {
   }
 }
 
+void PrintPreviewHandler::HandleHidePreview(const ListValue* args) {
+  HidePreviewTab();
+}
+
 void PrintPreviewHandler::HandleGetPrinterCapabilities(
     const ListValue* args) {
   std::string printer_name;
@@ -645,3 +656,24 @@ void PrintPreviewHandler::FileSelected(const FilePath& path,
 
   ActivateInitiatorTabAndClosePreviewTab();
 }
+
+void PrintPreviewHandler::HidePreviewTab() {
+  TabContentsWrapper* preview_tab_wrapper =
+      TabContentsWrapper::GetCurrentWrapperForContents(preview_tab());
+  if (GetBackgroundPrintingManager()->HasTabContents(preview_tab_wrapper))
+    return;
+  GetBackgroundPrintingManager()->OwnTabContents(preview_tab_wrapper);
+}
+
+void PrintPreviewHandler::ClearInitiatorTabDetails() {
+  TabContents* initiator_tab = GetInitiatorTab();
+  if (initiator_tab) {
+    // We no longer require the intiator tab details. Remove those details
+    // associated with the preview tab to allow the initiator tab to create
+    // another preview tab.
+    printing::PrintPreviewTabController* tab_controller =
+       printing::PrintPreviewTabController::GetInstance();
+    if (tab_controller)
+      tab_controller->EraseInitiatorTabInfo(preview_tab());
+  }
+}

chrome/browser/ui/webui/print_preview_handler.h

@@ -56,6 +56,10 @@ class PrintPreviewHandler : public WebUIMessageHandler,
   // First element of |args| is a job settings JSON string.
   void HandlePrint(const ListValue* args);
 
+  // Handles the request to hide the preview tab for printing.
+  // |args| is unused.
+  void HandleHidePreview(const ListValue* args);
+
   // Get the printer capabilities.
   // First element of |args| is the printer name.
   void HandleGetPrinterCapabilities(const ListValue* args);
@@ -94,6 +98,12 @@ class PrintPreviewHandler : public WebUIMessageHandler,
   // Adds all the recorded stats taken so far to histogram counts.
   void ReportStats();
 
+  // Helper function to hide the preview tab for printing.
+  void HidePreviewTab();
+
+  // Helper function to clear initiator tab details for this preview tab.
+  void ClearInitiatorTabDetails();
+
   // Pointer to current print system.
   scoped_refptr<printing::PrintBackend> print_backend_;