789507 - Printing doesn't work and the tab hangs if mozPrintCallback doesn't call done()

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[JK]

Attachment: mozPrintCallback example from https://gist.github.com/2230187

If mozPrintCallback is used, there will just be an infinite progress bar when printing. If you click cancel and attempt to print again, you get a "Printer Error - Not available" messagebox. The print preview window cannot be closed with the close button. 

The tab also hangs: the url bar, back button, refresh button etc. stop working.

Error console says:
Error: TypeError: ctx.fillRect is not a function

Comment 1

[Brendan Dahl [:bdahl]]

That's a bad example, but yes we need to handle the case when the mozPrintCallback never calls done() to signal that it is done.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Need to get this fixed before next merge.

Comment 3

[Brendan Dahl [:bdahl]]

Attachment: mozPrintCallback watch dog v1

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 665609 [details] [diff] [review]
mozPrintCallback watch dog v1

># HG changeset patch
># User Brendan Dahl <[e-mail]>
># Date 1348771887 25200
># Node ID 0c3b8f7efa230e58b05d7573e9c9cf2704421ad2
># Parent  e327e66a027dde88be68f59a40a93d653f870acb
>Bug 789507 - Add watchdog for mozPrintCallbacks.
>
>diff --git a/layout/printing/nsPagePrintTimer.cpp b/layout/printing/nsPagePrintTimer.cpp
>--- a/layout/printing/nsPagePrintTimer.cpp
>+++ b/layout/printing/nsPagePrintTimer.cpp
>@@ -39,16 +39,44 @@ nsPagePrintTimer::StartTimer(bool aUseDe
>         delay = mDelay;
>       }
>     }
>     mTimer->InitWithCallback(this, delay, nsITimer::TYPE_ONE_SHOT);
>   }
>   return result;
> }
> 
>+nsresult
>+nsPagePrintTimer::StartWatchDogTimer()
>+{
>+  nsresult result;
>+  if (mWatchDogTimer) {
>+    mWatchDogTimer->Cancel();
>+  }
>+  mWatchDogTimer = do_CreateInstance("@mozilla.org/timer;1", &result);
>+  if (NS_FAILED(result)) {
>+    NS_WARNING("unable to start the timer");
>+  } else {
>+    // Instead of just doing one timer for a long period do multiple so we
>+    // can check if the user cancelled the printing.
>+    mWatchDogTimer->InitWithCallback(this, WATCH_DOG_INTERVAL,
>+                                     nsITimer::TYPE_ONE_SHOT);
>+  }
>+  return result;
>+}
>+
>+void
>+nsPagePrintTimer::StopWatchDogTimer()
>+{
>+  if (mWatchDogTimer) {
>+    mWatchDogTimer->Cancel();
>+    mWatchDogTimer = nullptr;
>+  }
>+}
>+
> //nsRunnable
> NS_IMETHODIMP
> nsPagePrintTimer::Run() 
> {
>   bool initNewTimer = true;
>   // Check to see if we are done
>   // inRange will be true if a page is actually printed
>   bool inRange;
>@@ -56,57 +84,93 @@ nsPagePrintTimer::Run()
> 
>   // donePrinting will be true if it completed successfully or
>   // if the printing was cancelled
>   donePrinting = mPrintEngine->PrintPage(mPrintObj, inRange);
>   if (donePrinting) {
>     // now clean up print or print the next webshell
>     if (mPrintEngine->DonePrintingPages(mPrintObj, NS_OK)) {
>       initNewTimer = false;
>+      mDone = true;
>     }
>   }
> 
>   // Note that the Stop() destroys this after the print job finishes
>   // (The PrintEngine stops holding a reference when DonePrintingPages
>   // returns true.)
>   Stop(); 
>   if (initNewTimer) {
>     ++mFiringCount;
>     nsresult result = StartTimer(inRange);
>     if (NS_FAILED(result)) {
>-      donePrinting = true;     // had a failure.. we are finished..
>+      mDone = true;     // had a failure.. we are finished..
>       mPrintEngine->SetIsPrinting(false);
>     }
>   }
>   return NS_OK;
> }
> 
> // nsITimerCallback
> NS_IMETHODIMP
> nsPagePrintTimer::Notify(nsITimer *timer)
> {
>+  // When finished there still may be pending notificaitons just ignore them.
>+  if (mDone) {
>+    return NS_OK;
>+  }
>+
>+  if (timer && timer == mWatchDogTimer) {
>+    mWatchDogCount++;
>+    if (mWatchDogCount > WATCH_DOG_MAX_COUNT) {
>+      Fail();
>+      return NS_OK;
>+    }
>+  } else {
>+    // Reset the counter for any nofications that aren't from the watch dog
>+    // since this signals something has happened.
>+    mWatchDogCount = 0;
>+  }
>+
>   if (mDocViewerPrint) {
>     bool donePrePrint = mPrintEngine->PrePrintPage();
> 
>     if (donePrePrint) {
>+      StopWatchDogTimer();
>       NS_DispatchToMainThread(this);
>+    } else {
>+      // Start the watch dog if we're waiting for preprint to ensure that if any
>+      // mozPrintCallbacks take to long we error out.
>+      StartWatchDogTimer();
>     }
> 
>   }
>   return NS_OK;
> }
> 
> nsresult 
> nsPagePrintTimer::Start(nsPrintObject* aPO)
> {
>   mPrintObj = aPO;
>+  mWatchDogCount = 0;
>+  mDone = false;
>   return StartTimer(false);
> }
> 
> 
> void  
> nsPagePrintTimer::Stop()
> {
>   if (mTimer) {
>     mTimer->Cancel();
>     mTimer = nullptr;
>   }
>+  StopWatchDogTimer();
> }
>+
>+void
>+nsPagePrintTimer::Fail()
>+{
>+  mDone = true;
>+  Stop();
>+  if (mPrintEngine) {
>+    mPrintEngine->CleanupOnFailure(NS_OK, false);
>+  }
>+}
>diff --git a/layout/printing/nsPagePrintTimer.h b/layout/printing/nsPagePrintTimer.h
>--- a/layout/printing/nsPagePrintTimer.h
>+++ b/layout/printing/nsPagePrintTimer.h
>@@ -28,37 +28,48 @@ public:
>   nsPagePrintTimer(nsPrintEngine* aPrintEngine,
>                    nsIDocumentViewerPrint* aDocViewerPrint,
>                    uint32_t aDelay)
>     : mPrintEngine(aPrintEngine)
>     , mDocViewerPrint(aDocViewerPrint)
>     , mDelay(aDelay)
>     , mFiringCount(0)
>     , mPrintObj(nullptr)
>+    , mWatchDogCount(0)
>+    , mDone(false)
>   {
>     mDocViewerPrint->IncrementDestroyRefCount();
>   }
>   ~nsPagePrintTimer();
> 
>   NS_DECL_NSITIMERCALLBACK
> 
>   nsresult Start(nsPrintObject* aPO);
> 
>   NS_IMETHOD Run();
> 
>   void Stop();
> 
> private:
>   nsresult StartTimer(bool aUseDelay);
>+  nsresult StartWatchDogTimer();
>+  void     StopWatchDogTimer();
>+  void     Fail();
> 
>   nsPrintEngine*             mPrintEngine;
>   nsCOMPtr<nsIDocumentViewerPrint> mDocViewerPrint;
>   nsCOMPtr<nsITimer>         mTimer;
>+  nsCOMPtr<nsITimer>         mWatchDogTimer;
>   uint32_t                   mDelay;
>   uint32_t                   mFiringCount;
>   nsPrintObject *            mPrintObj;
>+  uint32_t                   mWatchDogCount;
>+  bool                       mDone;
>+
>+  static const uint32_t WATCH_DOG_INTERVAL  = 1000;
>+  static const uint32_t WATCH_DOG_MAX_COUNT = 10;
> };
> 
> 
> nsresult
> NS_NewPagePrintTimer(nsPagePrintTimer **aResult);
> 
> #endif /* nsPagePrintTimer_h___ */
>diff --git a/layout/printing/nsPrintEngine.cpp b/layout/printing/nsPrintEngine.cpp
>--- a/layout/printing/nsPrintEngine.cpp
>+++ b/layout/printing/nsPrintEngine.cpp
>@@ -1512,16 +1512,20 @@ nsresult nsPrintEngine::CleanupOnFailure
>   
>   if (aIsPrinting) {
>     SetIsPrinting(false);
>   } else {
>     SetIsPrintPreview(false);
>     SetIsCreatingPrintPreview(false);
>   }
> 
>+  if (mPageSeqFrame) {
>+    mPageSeqFrame->ResetPrintCanvasList();
>+  }
>+
>   /* cleanup done, let's fire-up an error dialog to notify the user
>    * what went wrong... 
>    * 
>    * When rv == NS_ERROR_ABORT, it means we want out of the 
>    * print job without displaying any error messages
>    */
>   if (aResult != NS_ERROR_ABORT) {
>     ShowPrintErrorDialog(aResult, aIsPrinting);

Comment 5

[Brendan Dahl [:bdahl]]

Attachment: mozPrintCallback watch dog v2

Addresses comments from irc:
- comment on what calls notify
- reset watchdog counter only from mozPrintCallback
- cleanup on destroy instead of failure

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 666036 [details] [diff] [review]
mozPrintCallback watch dog v2


> nsPagePrintTimer::Notify(nsITimer *timer)
> {
>+  // When finished there still may be pending notifications just ignore them.
Something like
When finished there may be still pending notifications, which we can just ignore.


this is not pretty, but we can improve the situation in FF19.
And hopefully there will be some more ideas in whatwg mailing list.

Comment 7

[Brendan Dahl [:bdahl]]

Attachment: mozPrintCallback watch dog v3

Comment update. Carrying forward r+

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Pushed to Try.
https://tbpl.mozilla.org/?tree=Try&rev=70bffe879810

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe034183d766

Should this have a test?

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fe034183d766

Comment 11

[Brendan Dahl [:bdahl]]

(In reply to Ryan VanderMeulen from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/fe034183d766
> 
> Should this have a test?

It ideally would, but there currently isn't an easy way to test the print engine.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

There are ways to test print preview, and indeed this should have a test.

Comment 13

[Virgil Dicu [:virgil] [QA]]

While using the attachment I could reproduce the issue on 2012-09-07 Nightly
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20120907030554


Tested on Firefox 18.0 beta 2 whit the same attachment
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20121128060531
a message of "An unknown error occurred while printing" while the printing bar was in progress.

I clicked "Ok" and started a new print job. While printing bar was in progress, I clicked on "Cancel". A "Not available" message appears and after a few seconds, the other message of "An unknown error occurred while printing" appeared.

Same behavior in Latest Nightly
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121204 Firefox/20.0
Build ID: 20121204030754