Sam Reporter
	Description • 22 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1b) Gecko/20020823 MultiZilla/v1.1.20
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.1b) Gecko/20020823 MultiZilla/v1.1.20

After using Print Preview to print pages, it will not close when pressing the 'Close' button. Each time I must then clos out the browser and re-start it.

Reproducible: Always

Steps to Reproduce:
1.Select Print Preview
2.Print a page
3.Press the Close button

Actual Results:  
The Close button is unresponsive. The browser must be closed and re-started.

Expected Results:  
Closed the Print Preview and returned to normal browsing.

This only started with some of the most recent nightly builds and did not happen to me earlier this week (week of 8-18-02).

	Tracy Walker [:tracy]
	Comment 1 • 22 years ago

confirmed with windows98 trunk build 2002-08-26-04-trunk

	sujay
	Comment 2 • 22 years ago

--> samir

	Jessica Blanco
	Comment 3 • 22 years ago

This also happens on OS/2 and Win NT. The fix for bug 139905 checked in on
08/21/2002 by rods caused this problem.

	Bill Mason
	Comment 4 • 22 years ago

Nominating for 1.2.

	Brendan Eich [:brendan]
	Comment 5 • 22 years ago

Why is this assigned to sgehani and not rods?  Drivers will want this fixed for
1.2alpha.

/be

	Pete Zha
	Comment 6 • 22 years ago

Maybe we shouldn't release mPrintEngine when we in printpreview since we need
it to return to GalleyPresentation by press close button.

	rods (gone) Assignee
	Comment 7 • 22 years ago

taking

	rods (gone) Assignee
	Comment 8 • 22 years ago

really taking

	rods (gone) Assignee
	Comment 9 • 22 years ago

Comment on attachment 98111 [details] [diff] [review]
purpose patch

r=rods for pete's patch (thank you)

	Asa Dotzler [:asa]
	Comment 10 • 22 years ago

rods, who can super-review this. Time is short for 1.2alpha.

	Simon Fraser [no longer active]
	Comment 11 • 22 years ago

Do we really understand the refcounting behaviour here?

     mPrintEngine->Destroy();
-    NS_RELEASE(mPrintEngine);
+    if (!GetIsPrintPreview()) {
+      NS_RELEASE(mPrintEngine);
+    }

Why would we not want to release something that has just been destroyed? And why
does the failure to do a release prevent the window from being closed?

	rods (gone) Assignee
	Comment 12 • 22 years ago

Kin and I talked, this needs more work.

	rods (gone) Assignee
	Comment 13 • 22 years ago

Actually we do understand the ref count behavior.

The DV holds a strong ref to the PrintEngine (PE) and the PE holds a weak ref
to DV. Other services and objects in the system may hold a strong pointer to
either the PE or an object that it has a strong ref to.

In the DV it is necessary to call "Destroy" before the release of the PE
because it needs a chance to cleanup its internal objects (like the PrintData
object). By cleaning up its internal data object release the PE. Then the DV
can release the PE. It is never assumed that when the DV releases the PE that
it will get destroyed at that moment. Typically it gets detroyed when the
PrintProgress finishes and goes away.

I have tested this with the debugger and verified that the PE is getting
detroyed when printing, when PP, and when printing from PP.

In summary, Pete's patch was correct except that the Printing data was being
cleaned up when in PP, this patch makes it so the printing data gets cleaned
up.

	Asa Dotzler [:asa]
	Comment 14 • 22 years ago

kin, pete, can one of you review? who can sr? time is short and we need
something asap.

	Pete Zha
	Comment 15 • 22 years ago

Comment on attachment 98210 [details] [diff] [review]
patch v2

>+    if (!mPrintEngine->GetIsPrintPreview()) {
>+      mPrintEngine->Destroy();
>+      NS_RELEASE(mPrintEngine);
>+    } else {
>+      mPrintEngine->DestroyPrintingData();
>+    }
Maybe we can avoid using "!" op by exchange code in if(){} and else{}.
r=pete

	rods (gone) Assignee
	Comment 17 • 22 years ago

Comment on attachment 98443 [details] [diff] [review]
patch with pete's suggestion

bringing pete's r= forward r=pete

	kinmoz
	Comment 18 • 22 years ago

Comment on attachment 98443 [details] [diff] [review]
patch with pete's suggestion

sr=kin@netscape.com

	Asa Dotzler [:asa]
	Comment 19 • 22 years ago

Comment on attachment 98443 [details] [diff] [review]
patch with pete's suggestion

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

	rods (gone) Assignee
	Comment 20 • 22 years ago

fixed

	Tracy Walker [:tracy]
	Comment 21 • 22 years ago

verified fixed