Closed Bug 789507 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Layout, defect, major)

x86
Windows 7
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox18 + fixed

People

(Reporter: spammaaja, Assigned: bdahl)

References

Details

(Whiteboard: [qa?])

Attachments

(2 files, 2 obsolete files)

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
Attachment #659284 - Attachment mime type: text/plain → text/html
Blocks: 745025
OS: All → Windows 7
Hardware: All → x86
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.
Attachment #659284 - Attachment description: mozPrintCallback example https://gist.github.com/2230187 → mozPrintCallback example from https://gist.github.com/2230187
Summary: Printing doesn't work and the tab hangs if mozPrintCallback is used → Printing doesn't work and the tab hangs if mozPrintCallback doesn't call done()
Need to get this fixed before next merge.
Attached patch mozPrintCallback watch dog v1 (obsolete) — Splinter Review
Attachment #665609 - Flags: review?(bugs)
Comment on attachment 665609 [details] [diff] [review]
mozPrintCallback watch dog v1

># HG changeset patch
># User Brendan Dahl <bdahl@mozilla.com>
># 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);
Attachment #665609 - Flags: review?(bugs)
Attached patch mozPrintCallback watch dog v2 (obsolete) — Splinter Review
Addresses comments from irc:
- comment on what calls notify
- reset watchdog counter only from mozPrintCallback
- cleanup on destroy instead of failure
Attachment #665609 - Attachment is obsolete: true
Attachment #666036 - Flags: review?(bugs)
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.
Attachment #666036 - Flags: review?(bugs) → review+
Comment update. Carrying forward r+
Attachment #666036 - Attachment is obsolete: true
Attachment #667057 - Flags: review+
Keywords: checkin-needed
Pushed to Try.
https://tbpl.mozilla.org/?tree=Try&rev=70bffe879810
Assignee: nobody → bdahl
https://hg.mozilla.org/mozilla-central/rev/fe034183d766
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(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.
There are ways to test print preview, and indeed this should have a test.
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
Whiteboard: [qa?]
Depends on: 844479
You need to log in before you can comment on or make changes to this bug.