168510 - [ActiveX] control enters infinite message loop when print command issued.

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: ActiveX Wrapper, defect)

Description

[Jim Jacobs]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020826

Added command handler for a print button to MozCtlDemo application:

void CMozCtlDemoDlg::OnPrint() 
{
  m_Browser.ExecWB (OLECMDID_PRINT, OLECMDEXECOPT_PROMPTUSER, NULL, & v);	
}

Callin ExecWB causes the UI thread to create a new message loop.  This allows
dialog to continue to function but the loop never terminates.  When dialog is
closed the process continues to execute becoming effectively a ghost process.  I
would guess the message loop in question is contained in "void
PrintListener::WaitForComplete()".  



Reproducible: Always

Steps to Reproduce:
1.  Add print button and handler (see details) to dialog class.
2.  Press the print button (you can press cancel on the print dialog).
3.  After a bit break process and notice call stack of active thread.

Actual Results:  
Normal message loop has a call to COleDispatchDriver::InvokeHelperV which calls
into Mozilla control code which appears to be implementing a message loop.

Expected Results:  
No MozCtl routines should be on the stack as is normal when the normal Windows
message loop is active.

Workaround:

Change contents of print command handler to:

  _printCount++; // dialog instance variable
  SetTimer (123, 200, NULL);
  m_Browser.ExecWB (OLECMDID_PRINT, OLECMDEXECOPT_PROMPTUSER, NULL, & v);	

Add timer handler:

afx_msg void CMozCtlDemoDlg::OnTimer( UINT id){

TRACE("Entering OnTimer; printCount=%d\n", _printCount);
  if (FindWindow (NULL, "Print") != NULL) return;
  if (_printCount <= 0){
    KillTimer (id);
    return;
  }
  _printCount --;
  PostQuitMessage (0);
}

Also add timer entry to CMozCtlDemoDlg message map:

  ON_WM_TIMER()

Also add method definition to CMozCtlDemoDlg class definition:

  afx_msg void OnTimer( UINT );

Comment 1

[Adam Lock]

*** Bug 171181 has been marked as a duplicate of this bug. ***

Comment 2

[Adam Lock]

The control asserts here:

http://lxr.mozilla.org/seamonkey/source/embedding/components/printingui/src/win/nsPrintingPromptService.cpp#182

It could be somehow related. The mfcEmbed app doesn't assert.

Comment 3

[Graham Freckleton]

Newbie here - be kind:  It appears that the call to Print() at:

http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/control/Mo
zillaBrowser.cpp#1484

is passing nsnull for the print listener callback interface, thus the 
WaitForComplete() call against the listener will never terminate since the 
callback is never called when printing is done.

Also, the print listener's OnStatusChange() at:

http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/control/Mo
zillaBrowser.cpp#3334

is testing the status for equality against STATE_STOP, while the calls to this 
interface I have seen in the code, such as:

http://lxr.mozilla.org/seamonkey/source/content/base/src/nsPrintData.cpp#173

is passing STATE_STOP | STATE_IS_DOCUMENT, therefore the test would fail 
anyway...

Comment 4

[Graham Freckleton]

Sorry, as an additional note, that workaround will work only if 
OLECMDEXECOPT_PROMPTUSER is used.  OLECMDEXECOPT_DONTPROMPTUSER will still loop.

Comment 5

[Graham Freckleton]

The changes I have detailed work, except that the code

http://lxr.mozilla.org/seamonkey/source/embedding/browser/activex/src/control/Mo
zillaBrowser.cpp#3334

needs to test aStateFlags for the actual state.

Comment 6

[Adam Lock]

Thanks Graham, you're analysis is right. I'll try and work out a patch that
corrects the listener so it is invoked and tests the proper stop flags

Comment 7

[Adam Lock]

Attachment: Patch

The problem is slightly more complicated than first thought. I first tried to
do a simple 2 line fix to register the listener properly but then I hit problem
number 2 - XUL print progress dialogs. The printing mechanism uses a printing
prompt service object to display the various dialogs during printing.

The control doesn't like XUL, so in order to display native dialogs I have
overridden the printing prompt service with my own implementation. So far it
doesn't display any dialog at all which means printing proceeds to the default
printer. I'll need to implement the print dialog and also a simple progress
dialog before this patch is ready for review.

Ideally I'd like to find a way to reuse the native print dialog code from the
default print prompt service. I will raise a new bug to do a bit of surgery to
the print code to make it more reusable.

Comment 8

[Adam Lock]

Raised bug 175890

Comment 9

[rods (gone)]

Other than the Windows version of the PrintingPromptService, the file that does
all the work is nsPrintDialogUtil.cpp, if need be, maybe we can put somewhere
were both impls can use it, instead of making a copy.

Comment 10

[Adam Lock]

Ideally I'd like to be able to split out this concept of a native dialog into a
new service which is what the other bug is about.

Alternatively if might be possible to add a pref or a new attribute to
nsIPrintingPromptService which disables the XUL fallback. This would negate the
need for me to override the service at all.

Comment 11

[rods (gone)]

The XUL fallback for what? 

There is a pref for indicating "no progress dialogs" then the XUL progress
dialogs will not show and you can register yourself as a listener and show your
own progress like MFCEmbed. Then all you are using the service for is the native
print dialog.

The main reason for MFCEmbed making a copy of the nsPrintUtils.cpp and
overriding the PrintingPromptService, was so that I would have a "working"
testcase that embedders "could" override the service and it would work. It was
more of an illustration.

I put in the pref "print.show_print_progress" for the very purpose of embedders
needing to turn them off.

http://lxr.mozilla.org/mozilla/source/embedding/qa/testembed/BrowserView.cpp#976

Comment 12

[Adam Lock]

I'm referring to the various XUL print dialogs. The control must only use native
print dialogs which means I either have to override the service with my own 
where the implementation is guaranteed not to use XUL, or find some way of
suppressing the XUL dialogs in the default printing prompt service.

If the latter is possible I'd take it instead of re-implementing the identical
service sans the XUL dialog. I'll try setting the print.show_print_progress pref
might fix this particular problem, but we need to consider a way to disable XUL
altogether if required.

Comment 13

[rods (gone)]

As far as Windows goes, the Progress dialog is the only XUL dialog. So if you
disable it via the pref, you should be all set.

Comment 14

[Adam Lock]

Attachment: Patch

This patch uses the pref to suppress the XUL progress dialog and is much
simpler.

Reviews?

Comment 15

[rods (gone)]

Comment on attachment 104811 [details] [diff] [review]
Patch

looks right r=rods

Comment 16

[rpotts (gone)]

hey adam,

don't we want to restore the value of the "print.show_print_progress" pref once
the print job has finished... so progress dialogs can show up in the future?

-- rick

Comment 17

[rods (gone)]

Good catch rick, yes I gree, especially if a different Moz browser would be run.

Comment 18

[Adam Lock]

The Mozilla control has its own profile called... "MozillaControl" so the answer
is no. 

Comment 19

[rpotts (gone)]

Comment on attachment 104811 [details] [diff] [review]
Patch

sr=[e-mail]

Comment 20

[Adam Lock]

Fixed in branch, nominate for mozilla1.2. Low risk, fix is significant for
ActiveX users.

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 104811 [details] [diff] [review]
Patch

a=asa for checkin to the 1.2 branch (on behalf of drivers)

Comment 22

[Adam Lock]

Checked into 1.2